Bug 529

Summary: sshd doesn't work correctly after SIGHUP
Product: Portable OpenSSH Reporter: Petr Ostadal <postadal>
Component: sshdAssignee: OpenSSH Bugzilla mailing list <openssh-bugs>
Status: CLOSED FIXED    
Severity: normal    
Priority: P2    
Version: -current   
Hardware: All   
OS: Linux   
Attachments:
Description Flags
Patch for commanline options
none
New patch for re-apeared problem with save_argv none

Description Petr Ostadal 2003-04-02 21:52:16 AEST
In portopenssh-3.6p1-vs-openbsd.diff.gz is bug in patch of sshd.c

diff -ruN --exclude CVS ssh-openbsd-2003032600/sshd.c openssh-3.6p1/sshd.c
--- ssh-openbsd-2003032600/sshd.c       2003-03-26 16:04:08.000000000 +1100
+++ openssh-3.6p1/sshd.c        2003-03-10 11:38:10.000000000 +1100
@@ -804,8 +821,23 @@
        Key *key;
        int ret, key_used = 0;
 
-       /* Save argv. */
+#ifdef HAVE_SECUREWARE
+       (void)set_auth_parameters(ac, av);
+#endif
+       __progname = get_progname(av[0]);
+       init_rng();
+
+       /* Save argv. Duplicate so setproctitle emulation doesn't clobber it */
+       saved_argc = ac;
        saved_argv = av;
+       saved_argv = xmalloc(sizeof(*saved_argv) * ac);
+       for (i = 0; i < ac; i++)
+               saved_argv[i] = xstrdup(av[i]);
+
+#ifndef HAVE_SETPROCTITLE

-------------------
If sshd uses to reload service after receive SIGHUP, it use execve to start sshd
with same parameters, which are save in saved_argv (note: it is missing in older
release, which caused problems, if some agrumends was passed to sshd throught
command line)., 

therefore saved_argv must be terminated by a NULL pointer!

Fixed version:

       saved_argv = xmalloc(sizeof(*saved_argv) * (ac + 1));
                                                   ^^^^^^^
       for (i = 0; i < ac; i++)
               saved_argv[i] = xstrdup(av[i]);
       saved_argv[ac] = NULL;
       ^^^^^^^^^^^^^^^^^^^^^^
Comment 1 Damien Miller 2003-05-15 19:00:23 AEST
Please attach the patch to the bug, rather than pasting it in the comments
field. Patches in comments are a PITA to extract.
Comment 2 Petr Ostadal 2003-05-15 21:19:04 AEST
Created attachment 301 [details]
Patch for commanline options
Comment 3 Damien Miller 2003-05-15 21:30:10 AEST
(That patch was a diff of a diff)

Applied anyway.
Comment 4 Petr Ostadal 2004-02-24 21:28:16 AEDT
This problem is re-appeared in new 3.7.1p2 version with added followed lines on
position 832:

#ifndef HAVE_SETPROCTITLE                                                       
        /* Prepare for later setproctitle emulation */                          
        compat_init_setproctitle(ac, av);                                       
        av = saved_argv;
        ^^^^^^^^^^^^^^^^                                                        
#endif 

The new patch follows in attachment.
Comment 5 Petr Ostadal 2004-02-24 21:31:45 AEDT
Created attachment 552 [details]
New patch for re-apeared problem with save_argv
Comment 6 Darren Tucker 2004-02-24 21:45:51 AEDT
This is fixed in -current and will be fixed in 3.8p1 (which will be released in
the next day or so).  Check a snapshot to confirm.
Comment 7 Damien Miller 2004-04-14 12:24:18 AEST
Mass change of RESOLVED bugs to CLOSED