Subject: kern/4396: msdosfs lookup bug
To: None <netbsd-bugs@NetBSD.ORG>
From: Rick Byers <rickb@iaw.on.ca>
List: netbsd-bugs
Date: 11/14/1997 20:55:38
>Number:         4396
>Category:       kern
>Synopsis:       msdosfs_lookup() doesn't allways function properly
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Oct 31 10:20:08 1997
>Originator:     Rick Byers
>Organization:
=========================================================================
Rick Byers                                      Internet Access Worldwide
rickb@iaw.on.ca                                              System Admin
University of Waterloo, Computer Science                    (905)714-1400
http://www.iaw.on.ca/rickb/                         http://www.iaw.on.ca/


>Release:        Oct 25, 1997
>Environment:

        
System: NetBSD rickb 1.3_ALPHA NetBSD 1.3_ALPHA (RICKB) #26: Thu Oct 30 02:46:1
8 EST 1997 root@rickb:/usr/src/sys/arch/i386/compile/RICKB i386




>Description:
        In msdosfs file systems, there exists a problem when looking up
        long file names with similar names.  If the file name being looked
        up matches the beginning 13 (or n*13) characters of an existing file,
        it will return a match.  This occurs because the filename comparison
        is done a slot at a time, and there is no check to ensure that when
        two filenames are compared, the name components are both in the same
        relative postion (same slot) of the whole filename.


>How-To-Repeat:
        cd /msdos
        touch 'NetBSD File 1'          #Note: 13 characters exactly
        ls
        NetBSD File 1


        touch 'NetBSD File 10'
        ls
        NetBSD File 1


        rm 'NetBSD File 1 some other file that doesnt exist'
        ls


>Fix:
        In msdosfs_lookup, some sort of check should be done to ensure that
        all of the filename has been compared, and not just the beginning
        portion.


        In my fix, I keep track of a current slot number (winslot).  I never
        compare two parts of a file name unless they are both the same
        relative position in the filename (same slot #).  If something
        doesn't compare, I reset the current slot number to one less than
        the number of slots required for the given filename.  This still
        leaves open the possibility of problems with filenames on disk
        who are longer than the file being looked up, but winChkName
        ensures that this will not be a problem.


        This patch appears to fix the problem, but someone more experienced
        than I might be able to come up with something more elegant.  This
        patch is created against a the source which includes a previous
        patch I sent in regarding the MSDOSFS_DEBUG option.

Index: msdosfs_lookup.c
===================================================================
RCS file: /usr/cvsroot/netbsd/src/sys/msdosfs/msdosfs_lookup.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -c -r1.2 -r1.3
*** msdosfs_lookup.c	1997/10/30 06:18:07	1.2
--- msdosfs_lookup.c	1997/10/30 07:51:13	1.3
***************
*** 111,119 ****
  	int flags = cnp->cn_flags;
  	int nameiop = cnp->cn_nameiop;
  	int wincnt = 1;
  	int chksum = -1;
  	int olddos = 1;
! 	
  #ifdef MSDOSFS_DEBUG
  	printf("msdosfs_lookup(): looking for %s\n", cnp->cn_nameptr);
  #endif
--- 111,120 ----
  	int flags = cnp->cn_flags;
  	int nameiop = cnp->cn_nameiop;
  	int wincnt = 1;
+ 	int winslot;
  	int chksum = -1;
  	int olddos = 1;
! 
  #ifdef MSDOSFS_DEBUG
  	printf("msdosfs_lookup(): looking for %s\n", cnp->cn_nameptr);
  #endif
***************
*** 237,243 ****
  	if ((nameiop == CREATE || nameiop == RENAME) &&
  	    (flags & ISLASTCN))
  		slotcount = 0;
! 	
  #ifdef MSDOSFS_DEBUG
  	printf("msdosfs_lookup(): dos version of filename %s, length %ld\n",
  	    dosfilename, cnp->cn_namelen);
--- 238,246 ----
  	if ((nameiop == CREATE || nameiop == RENAME) &&
  	    (flags & ISLASTCN))
  		slotcount = 0;
! 
! 	/* initialize the current winslot to one less than the wincnt */	
! 	winslot = wincnt -1;
  #ifdef MSDOSFS_DEBUG
  	printf("msdosfs_lookup(): dos version of filename %s, length %ld\n",
  	    dosfilename, cnp->cn_namelen);
***************
*** 285,290 ****
--- 288,294 ----
  				 * Drop memory of previous long matches
  				 */
  				chksum = -1;
+ 				winslot = wincnt -1;
  				
  				if (slotcount < wincnt) {
  					slotcount++;
***************
*** 309,318 ****
  					if (pmp->pm_flags & MSDOSFSMNT_SHORTNAME)
  						continue;
  
! 					chksum = winChkName((const u_char *)cnp->cn_nameptr,
! 							    cnp->cn_namelen,
! 							    (struct winentry *)dep,
! 							    chksum);
  					continue;
  				}
  				
--- 313,344 ----
  					if (pmp->pm_flags & MSDOSFSMNT_SHORTNAME)
  						continue;
  
! 					/*
! 					 * Only check the name if it is in the
! 					 * proper slot.  If the slot on disk
! 				 	 * doesn't agree with the slot we're
! 					 * looking for, then start over.
! 					 */
! 					if( winslot == (((struct winentry*)dep)->weCnt&WIN_CNT) )
! 					{
! 						chksum = winChkName((const u_char *)cnp->cn_nameptr,
! 								    cnp->cn_namelen,
! 								    (struct winentry *)dep,
! 								    chksum);
! 						if( chksum == -1 )
! 							winslot = wincnt -1;  /* go back to looking at the head slot */
! 						else
! 							winslot--;  /* now we're looking at the next slot */	
! 					}
! 					else
! 					{
! 						/*
! 						 * Slots don't match
! 						 * Start over.  
! 						 */	
! 						winslot = wincnt - 1;
! 						chksum = -1;
! 					}
  					continue;
  				}
  				
***************
*** 321,326 ****
--- 347,353 ----
  				 * the root directory).
  				 */
  				if (dep->deAttributes & ATTR_VOLUME) {
+ 					winslot = wincnt - 1;
  					chksum = -1;
  					continue;
  				}
***************
*** 330,335 ****
--- 357,363 ----
  				 */
  				if (chksum != winChksum(dep->deName)
  				    && (!olddos || bcmp(dosfilename, dep->deName, 11))) {
+ 					winslot = wincnt - 1;
  					chksum = -1;
  					continue;
  				}


>Audit-Trail:
>Unformatted:


>Last-Modified:


=========================================================================
Rick Byers                                      Internet Access Worldwide
rickb@iaw.on.ca                                		     System Admin
University of Waterloo, Computer Science                    (905)714-1400
http://www.iaw.on.ca/rickb/                         http://www.iaw.on.ca/