On Wed, 14 Oct 2015 13:45:04 +0200 Petr Hracek <phra...@redhat.com> wrote:
> Hi Amadeusz, > > based on our last discussion, I tried to use it > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=644788;msg=265;att=0 > > And I guess, that If I already updated screen with different message > structure. > then now I am not able to reconnect as you mentioned. > Althought WriteOldMessage(&m) is used. Hey, Yes struct old_msg may need adjusting along with WriteOldMessage function, so passing new values to old_msg works. Although now that I look at this patch again, it seems very hacky. It copies values from attaching (new) client, into old_msg struct of server and attaches to it with new client. It's wonder that it doesn't crash, as even with copied values, struct between client and server are different. Amadeusz > > Greetings > Petr > > On 10/12/2015 07:12 PM, Amadeusz Sławiński wrote: > > On Mon, 12 Oct 2015 12:24:56 +0200 > > Petr Hracek <phra...@redhat.com> wrote: > > > >> On 09/08/2015 06:26 PM, Amadeusz Sławiński wrote: > >>> On Tue, 8 Sep 2015 12:08:54 +0200 > >>> Petr Hracek <phra...@redhat.com> 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/screen-devel@gnu.org/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. > >> > >> > >