samba - Feb 1998 - Bug found: Truncation of files during multiple simultaneous logons ....

Hi Samba Team,

There seems to be a bug that seems to have survived from 1.9.16p11 (that is 
as far as I went). BTW, 1.9.15p8 does not have this bit of code.

Symptom:
	- This applies to situations where there are multiple logons (sessions) to 
the same SAMBA server by the *same* user (UNIX UID). The sessions may be 
from multiple logons to the same NT client or from different clients.
	- When the user logs of from one of these sessions, files open for 
writing/modification in the other sessions are truncated.

Cause:
	- The problem has been traced to code in the 'reply_ulogoff' routine in
reply.c. Code
	snippet between the two *** BUG *** markers

-----
int reply_ulogoffX(char *inbuf,char *outbuf,int length,int bufsize)
{
  uint16 vuid = SVAL(inbuf,smb_uid);
  user_struct *vuser = get_valid_user_struct(vuid);

  if(vuser == 0) {
    DEBUG(3,("ulogoff, vuser id %d does not map to user.\n", vuid));
  }

*** BUG ***
  /* in user level security we are supposed to close any files
     open by this user */
  if ((vuser != 0) && (lp_security() != SEC_SHARE)) {
    int i;
    for (i=0;i<MAX_OPEN_FILES;i++)
      if (Files[i].uid == vuser->uid && Files[i].open) {         
<- BUG IS
HERE !
        close_file(i,False);
      }
  }
*** BUG ***

  invalidate_vuid(vuid);

-------------------------------

	- When the user logs off from one of the logons, the NT client sends a 
SMBulogoffX to the
	SAMBA server, which then promptly, because of the offending line above, 
closes *all* files
	that this user has open on this SAMBA server.

	- Commenting this section of code, works for the most part, since the 
client does send an
	SMBclose when a program exits with open file handles (as part of the 
process termination
 	cleanup).

	- I think that this can be fixed by modifying the 'if' comparison to be
done on 'vuid' and not 'uid'.
	That would mean an modification of the 'files_struct' structure. I am 
working on this fix (not my
	highest priority though since commenting out works for now). Can someone 
from the SAMBA
	team fix	this ? It this is the fix then it should be fairly easy for 
someone familiar with
	the code to make the changes and evaluate the ramifications elsewhere, if 
any.

Finally, a question:

	- What was the motivation for putting in this code in the first place ? A 
possible scenario that
	comes to mind is when the client dies abnormally but ... does the client 
sends a ulogoff at this
	time ? Well I would like to hear from the SAMBA gurus !!

Please let me know if you need more information.

Thanks !

Ravi



--
======================================================Ravi Subramaniam
Senior Software Systems Engineer,
Computing Technology, Design Technology,
Mailstop: RN4-36,                               Ph : (408)-765-3566
Intel Corp., Santa Clara,                       Email: 
rsubram@scdt.intel.com
California, 95052			        Ravi_Subramaniam@ccm.sc.intel.com
=======================================================

Ravi Subramaniam wrote:
> 
> Hi Samba Team,
> 
> There seems to be a bug that seems to have survived from 1.9.16p11 (that is
> as far as I went). BTW, 1.9.15p8 does not have this bit of code.
>
> ...... explaination deleted...
>
>         - I think that this can be fixed by modifying the 'if'
comparison to be
> done on 'vuid' and not 'uid'.
> 
> Finally, a question:
> 
> What was the motivation for putting in this code in the first place ? A
> possible scenario that comes to mind is when the client dies abnormally 
> but ... does the client sends a ulogoff at this
> time ?
Ravi,

	*Wonderful* piece of debugging - congratulations
and thanks !

The fix of comparing with the vuid is correct, and
the problem in ulogoff was missed when I invented 
the and coded up the vuid construct (when I was fixing 
an earlier bug where Samba used to send the users uid 
directly as the vuid).

The code is there to comply with the SMB spec that
states when a ulogoff is done that all files opened
by that vuid must be closed.

Thanks once again for your help, I'll fix this
asap.

Cheers,

	Jeremy Allison,
	Samba Team.

-- 
--------------------------------------------------------
Buying an operating system without source is like buying
a self-assembly Space Shuttle with no instructions.
--------------------------------------------------------