Hannes Schmidt wrote: > > As you suggested, I checked how other NSS libraries deal with this problem. > The libnss_files module in glibc doesn't leave files open like > libnss_winbind. > > E.g., getpwent() (I should say its implementation in libnss_files) and > setpwent() > open /etc/passwd and endpwent() closes it. getpwnam() and getpwuid() close > it before returning. > > I didn't completely understand libnss_nis, but it seems to do likewise.
nss_ldap would be worth a look. It does keep it's fd open, so should be more like winbind. > I noticed another interesting thing: all the above NSS libraries set the > close-on-exec flag on the files they open. This is a good idea for two > reasons: > > (A) security - see http://www.pgp.com/research/covert/advisories/028.asp. > > (B) saving resources - an exec() overwrites all data, even the > shared libraries data. Consequently, the variable which holds the pipe/file > handle gets reset too and there is no way to close the file. It will get > opened > a second time. For each exec() one file handle will be wasted. Very interesting. This would certainly 'fix' your problem, and seems like a good idea. > My patch fixes both problems - it sets the close-on-exec flag for the > winbindd > pipe handle and it makes sure that the handle isn't 0, 1, 2. I tested it on > my three > production servers. No problems. > > diff -u -x *.o -x *.so -x *.po samba-2.2.5/source/nsswitch/wb_common.c > samba-2.2.5-modified/source/nsswitch/wb_common.c > --- samba-2.2.5/source/nsswitch/wb_common.c Wed Jun 19 03:13:44 2002 > +++ samba-2.2.5-modified/source/nsswitch/wb_common.c Wed Jul 31 15:54:10 > 2002 > @@ -102,6 +102,7 @@ > static pid_t our_pid; > struct stat st; > pstring path; > + int old_fd, result, flags; > > if (our_pid != getpid()) { > close_sock(); > @@ -159,6 +160,30 @@ > return -1; > } > > + /* Make sure socket handle isn't stdin, stdout or stderr */ > + > + if (winbindd_fd < 3) { > + old_fd = winbindd_fd; > + if ((winbindd_fd = fcntl(winbindd_fd, F_DUPFD, 3)) == -1) { The only thing that worries me is portability. How portable is this? > + winbindd_fd = old_fd; > + close_sock(); > + return -1; > + } > + close( old_fd ); > + } > + > + /* Socket should be closed on exec() */ > + > + result = flags = fcntl(winbindd_fd, F_GETFD, 0); > + if (flags >= 0) { > + flags != FD_CLOEXEC; > + result = fcntl( winbindd_fd, F_SETFD, flags ); And in particular, how portable is these... > + } > + if (result < 0) { > + close_sock(); > + return -1; > + } > + > if (connect(winbindd_fd, (struct sockaddr *)&sunaddr, > sizeof(sunaddr)) == -1) { > close_sock(); > diff -u -x *.o -x *.so -x *.po > samba-2.2.5/source/nsswitch/winbind_nss_config.h > samba-2.2.5-modified/source/nsswitch/winbind_nss_config.h > --- samba-2.2.5/source/nsswitch/winbind_nss_config.h Wed Jun 19 03:13:44 > 2002 > +++ samba-2.2.5-modified/source/nsswitch/winbind_nss_config.h Wed Jul 31 > 15:21:25 2002 > @@ -62,6 +62,10 @@ > #include <string.h> > #endif > > +#ifdef HAVE_FCNTL_H > +#include <fcntl.h> > +#endif > + > #include <sys/types.h> > #include <sys/stat.h> > #include <errno.h> > Thankyou very much for this. The portability needs to be to quite a few systems, as this code is used for all systems, not just systems with nsswitch etc. If you can get back to me on those (and implement configure tests/alternate behaviours if required) I think I'll be able to apply it. Nice work! Andrew Bartlett -- Andrew Bartlett [EMAIL PROTECTED] Manager, Authentication Subsystems, Samba Team [EMAIL PROTECTED] Student Network Administrator, Hawker College [EMAIL PROTECTED] http://samba.org http://build.samba.org http://hawkerc.net