[Samba] Please evaluate my patch for Winbind

Hannes Schmidt mail at schmidt-net.via.t-online.de
Thu Jul 25 09:45:03 GMT 2002


Yesterday I posted a problem report with some diagnostics. I seem to have found a solution on my own 
and provide a patch here. [I didn't post this as a follow-up because nobody responded to my original post 
and I wanted to make sure someone reads at least this one.]

The patch makes the winbind client (libnss_winbind.so) code more robust.

Consider the following fragment of broken application code.

if( fork() == 0 ) {
    /* child */
    close( 0 );             /* next open() will return 0 (stdin) */
    getpwent(...);        /* pid changed, so wb_common.c will open the pipe to winbindd, the handle will be 0 */
    fd = open( ... );     /* returned fd isn't 0 although the code assumes this */
    execve( ... );         /* executed program reads from stdin which - unintentionally - is the winbind pipe */
} else {
    /* parent */
    ...
}

Obviously, this code is broken and could be easily fixed by doing the getpwent() before the close(). Unfortunately 
the version of cron installed on my system is broken in a more complex but similar way. There is probably more 
broken code like this in other standard unix software, especially since it's such a subtle mistake.

The provided patch to wb_common.c makes sure that the handle to the winbindd pipe is not 0, 1 or 2. 
Even if this is complete crap, I would appreciate any feedback.

*** /root/samba-2.2.5/source/nsswitch/wb_common.c Wed Jun 19 03:13:44 2002
--- wb_common.c Thu Jul 25 16:36:39 2002
***************
*** 94,99 ****
--- 94,120 ----
   }
  }
  
+ /* Returns a duplicate of the given file descriptor that is guranteed 
+    not to be 0, 1 or 2 (stdin, stdout or stderr respectively), unless 
+    an error occurs. If the given fd is invalid, it will be returned
+    unchanged. If a different error occurs, the returned file handle 
+    will still be valid but it may be 0, 1 or 2. If a valid file 
+    handle is returned and the returned handle is different to the
+    original one, the original one will be closed. If dup() is broken, 
+    this function might never return. */
+ 
+ int make_nonstd_fd(int fd) 
+ {
+  int saved_fd;
+  if (0 >= fd && fd <= 2) {
+      fd = dup(saved_fd = fd);
+      if (fd == -1) return saved_fd;
+      fd = make_nonstd_fd(fd);
+      close(saved_fd);
+  }
+  return fd;
+ }
+ 
  /* Connect to winbindd socket */
  
  int winbind_open_pipe_sock(void)
***************
*** 158,164 ****
   if ((winbindd_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
    return -1;
   }
!  
   if (connect(winbindd_fd, (struct sockaddr *)&sunaddr, 
        sizeof(sunaddr)) == -1) {
    close_sock();
--- 179,188 ----
   if ((winbindd_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
    return -1;
   }
! 
!  winbindd_fd = make_nonstd_fd( winbindd_fd );
! 
! 
   if (connect(winbindd_fd, (struct sockaddr *)&sunaddr, 
        sizeof(sunaddr)) == -1) {
    close_sock();
***************
*** 167,172 ****
--- 191,197 ----
          
   return winbindd_fd;
  }
+ 
  
  /* Write data to winbindd socket */
  






More information about the samba mailing list