nmbd.c open_sockets() have wonder initialization.

Kenichi Okuyama okuyamak at dd.iij4u.or.jp
Wed Nov 1 01:26:42 GMT 2000


Dear all,

I found
./source/nmbd/nmbd.c:open_sockets() 
contains bug.

Original code does something like this:
1) ClientNMB will be open-ed, without any check.
2) ClientDGRAM will be open-ed, without any check.
3) if ClientNMB was invalid, return False.
   -> This will cause main() to return ( i.e. exit with code 1 )
4) SIGPIPE will be blocked.
5) call set_socket_options() against ClientNMB.
6) call set_socket_options() against ClientDGRAM.
7) return True.


What so bad about this code, is that, there's no check against
validness of 'ClientDGRAM'. Nobody check about validness of
ClientDGRAM, but will start setting socket options, and continue
using.

Also, even set_socket_options() returns error, nobody is checking,
and will continue nmbd running, with ClientDGRAM kept invalid.

This problem will not appear frequently, for people usually start
nmbd at beginning of system start, when we have enouth resource in
system. I think that's reason why this bug did not appear.


To fix this problem, and to avoid this kind of mistake, we should
check file descriptor one by one, and if you found any invalid
result, close all the file descriptor and return with "False".
Opening socket should be fit within this function.


Here's patch. This patch will work not only against newest version,
but to 2.0.7 or SAMBA_2_2 branch as well, for this part is not being
changed.

best regards,
---- 
Kenichi Okuyama at Tokyo Research Lab. IBM-Japan Co.


Index: source/nmbd/nmbd.c
===================================================================
RCS file: /cvsroot/samba/source/nmbd/nmbd.c,v
retrieving revision 1.106
diff -b -B -u -r1.106 nmbd.c
--- source/nmbd/nmbd.c	2000/10/12 21:19:48	1.106
+++ source/nmbd/nmbd.c	2000/10/31 14:15:58
@@ -518,16 +518,21 @@
      now deprecated.
    */
 
-  if ( isdaemon )
+    if ( isdaemon ) {
     ClientNMB = open_socket_in(SOCK_DGRAM, port,0,0,True);
-  else
+        if ( ClientNMB == -1 ) {
+            return False;
+        }
+    } else {
     ClientNMB = 0;
+    }
   
   ClientDGRAM = open_socket_in(SOCK_DGRAM,DGRAM_PORT,3,0,True);
+    if ( ClientDGRAM == -1 ) {
+        close( ClientNMB );
+        return False;
+    }
 
-  if ( ClientNMB == -1 )
-    return( False );
-
   /* we are never interested in SIGPIPE */
   BlockSignals(True,SIGPIPE);




More information about the samba-technical mailing list