Re: [screen-devel] screen cannot reattach to previous session

[Amadeusz Sławiński]

On Mon, 12 Oct 2015 12:24:56 +0200
Petr Hracek <address@hidden> wrote:

> On 09/08/2015 06:26 PM, Amadeusz Sławiński wrote:
> > On Tue, 8 Sep 2015 12:08:54 +0200
> > Petr Hracek <address@hidden> wrote:
> >
> >> On 08/31/2015 07:41 PM, Amadeusz Sławiński wrote:
> >>>> I have received a bug
> >>>> https://bugzilla.redhat.com/show_bug.cgi?id=1253697
> >>>> which causes that screen indefinite hangs.
> >>>>
> >>>> Strace is a part of the bug.
> >>>> Did you ever discover this problem?
> >>>>
> >>>> This bug was discovered in screen-4.1.0.
> >>> Hi,
> >>>
> >>> I recall that someone on irc had similar problem, it seemed like
> >>> his /dev/pts was being mounted with wrong options
> >>> it needed something like those mount options to work:
> >>> "gid=5,mode=620" (group 5 being tty on my system).
> >>>
> >>> Amadeusz
> >>>
> >> Hi,
> >>
> >> they provided a options:
> >>
> >> /dev/pts mount:
> >>
> >> $ mount | grep pts
> >> devpts on /dev/pts type devpts
> >> (rw,nosuid,noexec,relatime,seclabel,gid=5,mode=620,ptmxmode=000)
> >>
> >> But it seems that it is not working still.
> >>
> >> What about ptmxmode?
> >> Could it be relevant?
> > Hi,
> >
> > I don't think so, mine is also 000.
> > As far as I can see from bugreport it happens between updates.
> > What was version before and after update?
> > Also do you have somewhere diff between those versions?
> >
> > It can be that there is some difference in "struct msg" between
> > versions which causes this problem.
> >
> > Does it only fails to reattach after update?
> > By which I mean, does new sessions after update attach properly?
> >
> > Amadeusz
> >
> Finally I have got the reason why screen hangs.
> It is caused by patch "LoginName too long".
> 
> See related report https://bugzilla.redhat.com/show_bug.cgi?id=1119794
> Patch for this case was mentioned there.
> 

Hi,
" You are not authorized to access bug #1119794. To see this bug, you
must first log in to an account with the appropriate permissions. "

> Screen related bug is https://savannah.gnu.org/bugs/?21653
> Patch which we used is 
> http://www.mail-archive.com/address@hidden/msg00829.html
> and another patch. 
> https://lists.gnu.org/archive/html/screen-devel/2011-05/msg00000.html'
> What is the correct one for this bug?

Yeah, those were my first steps in official tree, so I got to a bit
messy start, there were some discussions which resulted in:
http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=785f5992d84bb72a79ec6cd15389c79b9a434e17
http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=b62e4ef0971fc9bbf5298d810819f406abb23862
http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=fe8103cccd58c2e5de40634e694e2d99afce67ca
http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=ae76b78836aec6018d6b008c312c4d8ecf70326e

> 
> And unfortunately I have applied the wrong patch like:

The patch itself doesn't seem wrong, however...

> diff --git a/acls.c b/acls.c
> index b8a5af4..b8ef048 100644
> --- a/acls.c
> +++ b/acls.c
> @@ -182,7 +182,7 @@ struct acluser **up;
>   #endif
>     (*up)->u_Esc = DefaultEsc;
>     (*up)->u_MetaEsc = DefaultMetaEsc;
> -  strncpy((*up)->u_name, name, 20);
> +  strncpy((*up)->u_name, name, MAXLOGINLEN);
>     (*up)->u_password = NULL;
>     if (pass)
>       (*up)->u_password = SaveStr(pass);
> @@ -318,8 +318,8 @@ struct acluser **up;
>       return UserAdd(name, pass, up);
>     if (!strcmp(name, "nobody"))        /* he remains without
> password */ return -1;
> -  strncpy((*up)->u_password, pass ? pass : "", 20);
> -  (*up)->u_password[20] = '\0';
> +  strncpy((*up)->u_password, pass ? pass : "", MAXLOGINLEN);
> +  (*up)->u_password[MAXLOGINLEN] = '\0';
>     return 0;
>   }
>   #endif
> diff --git a/acls.h b/acls.h
> index 907e953..42c7c18 100644
> --- a/acls.h
> +++ b/acls.h
> @@ -78,7 +78,7 @@ struct plop
>   typedef struct acluser
>   {
>     struct acluser *u_next;        /* continue the main user list */
> -  char u_name[20+1];        /* login name how he showed up */
> +  char u_name[MAXLOGINLEN + 1];        /* login name how he showed
> up */ char *u_password;        /* his password (may be NullStr). */
>     int  u_checkpassword;        /* nonzero if this u_password is
> valid */ int  u_detachwin;        /* the window where he last
> detached */ diff --git a/comm.c b/comm.c
> index 5f4af8a..7705fcb 100644
> --- a/comm.c
> +++ b/comm.c
> @@ -36,6 +36,7 @@
>    */
> 
>   #include "config.h"
> +#include "os.h"
>   #include "acls.h"
>   #include "comm.h"
> 
> diff --git a/display.h b/display.h
> index b1ab748..a433e6d 100644
> --- a/display.h
> +++ b/display.h
> @@ -73,7 +73,7 @@ struct display
>     struct win *d_other;        /* pointer to other window */
>     int   d_nonblock;        /* -1 don't block if obufmax reached */
>                   /* >0: block after nonblock secs */
> -  char  d_termname[40 + 1];    /* $TERM */
> +  char  d_termname[MAXTERMLEN + 1];    /* $TERM */
>     char    *d_tentry;        /* buffer for tgetstr */
>     char    d_tcinited;        /* termcap inited flag */
>     int    d_width, d_height;    /* width/height of the screen */
> diff --git a/os.h b/os.h
> index 5c17c83..92846ad 100644
> --- a/os.h
> +++ b/os.h
> @@ -521,3 +521,8 @@ typedef struct fd_set { int fds_bits[1]; } fd_set;
>    */
>   #define IOSIZE        4096
> 
> +/* Changing those you won't be able to attach to your old sessions
> + * when changing those values in official tree don't forget to bump
> + * MSG_VERSION */
> +#define MAXTERMLEN  50
> +#define MAXLOGINLEN 256

here is your problem, MSG_VERSION is not revbumped in patch
(if you check git links above, I've revbumped it separately before
release). Basically it changes struct which screen uses to maintain
it's state, so attaching to previous versions doesn't work.
If you've used some other patches to fix this problem before, you can
change those values to be identical to those used previously, and it
should probably work fine then.

Amadeusz


> diff --git a/process.c b/process.c
> index 0769cbe..623bf87 100644
> --- a/process.c
> +++ b/process.c
> @@ -2664,9 +2664,9 @@ int key;
>         s = NULL;
>         if (ParseSaveStr(act, &s))
>       break;
> -      if (strlen(s) >= 20)
> +      if (strlen(s) >= MAXTERMLEN)
>       {
> -      OutputMsg(0, "%s: term: argument too long ( < 20)", rc_name);
> +     OutputMsg(0, "%s: term: argument too long ( < %d)", rc_name, 
> MAXTERMLEN);
>         free(s);
>         break;
>       }
> diff --git a/screen.c b/screen.c
> index 1c88157..eda16a3 100644
> --- a/screen.c
> +++ b/screen.c
> @@ -995,10 +995,10 @@ char **av;
> 
>     if (home == 0 || *home == '\0')
>       home = ppp->pw_dir;
> -  if (strlen(LoginName) > 20)
>   #ifdef MULTIUSER
> -  if (multi && strlen(multi) > 20)
> +  if (multi && strlen(multi) > MAXLOGINLEN)
>       Panic(0, "Screen owner name too long - sorry.");
>   #endif
>     if (strlen(home) > MAXPATHLEN - 25)
> diff --git a/screen.h b/screen.h
> index 1a388e3..b95f8a2 100644
> --- a/screen.h
> +++ b/screen.h
> @@ -202,32 +202,32 @@ struct msg
>         int nargs;
>         char line[MAXPATHLEN];
>         char dir[MAXPATHLEN];
> -      char screenterm[20];    /* is screen really "screen" ? */
> +     char screenterm[MAXTERMLEN];    /* is screen really "screen" ?
> */ }
>         create;
>         struct
>       {
> -      char auser[20 + 1];    /* username */
> +     char auser[MAXLOGINLEN + 1];    /* username */
>         int apid;        /* pid of frontend */
>         int adaptflag;    /* adapt window size? */
>         int lines, columns;    /* display size */
>         char preselect[20];
>         int esc;        /* his new escape character unless -1 */
>         int meta_esc;        /* his new meta esc character unless -1
> */
> -      char envterm[40 + 1];    /* terminal type */
> +     char envterm[MAXTERMLEN + 1];    /* terminal type */
>         int encoding;        /* encoding of display */
>         int detachfirst;      /* whether to detach remote sessions
> first */ }
>         attach;
>         struct
>       {
> -      char duser[20 + 1];    /* username */
> +     char duser[MAXLOGINLEN + 1];    /* username */
>         int dpid;        /* pid of frontend */
>       }
>         detach;
>         struct
>       {
> -      char auser[20 + 1];    /* username */
> +     char auser[MAXLOGINLEN + 1];    /* username */
>         int nargs;
>         char cmd[MAXPATHLEN];    /* command */
>         int apid;        /* pid of frontend */
> diff --git a/socket.c b/socket.c
> index 45887fb..b4d2400 100644
> --- a/socket.c
> +++ b/socket.c
> @@ -1528,7 +1528,7 @@ static void PasswordProcessInput __P((char *,
> int));
> 
>   struct pwdata {
>     int l;
> -  char buf[20 + 1];
> +  char buf[MAXLOGINLEN + 1];
>     struct msg m;
>   };
> 
> 
> Greetings.
> 
>