Re: CVS commit: src/sys/ufs/ufs

[David Holland <dholland-sourcechanges%netbsd.org@localhost>]

On Sun, Feb 24, 2019 at 07:06:40PM +0000, Michael van Elst wrote:
 > Modified Files:
 > 	src/sys/ufs/ufs: ufs_vnops.c
 > 
 > Log Message:
 > Reading a directory may trigger a panic when the buffer is too small.
 > Adjust necessary checks.
       :
 > +       if (physstart >= physend) {
 > +               /* Need at least one block */
 > +               return EINVAL;
 > +       }

I do not think this will work in all cases. The libc code only
guarantees DIRBLKSIZ of space at once, and DIRBLKSIZ is 1024. On a
volume with 4K sectors the union mount code path in _initdir will
probably fail.

 > While here, also check for arithmetic overflow.

...which is impossible, because skipstart has to be less than
ump->um_dirblksiz.

Furthermore, this:

 > +       rawbuf -= dropend;

is entirely wrong (it needs to be "rawbufmax") and without that bound
on rawbufmax the code is unsafe...

Here's the fix I got bogged down trying to build and test, which also
adds a missing upper bound on callerbytes:


Index: ufs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.239
diff -u -p -r1.239 ufs_vnops.c
--- ufs_vnops.c	28 Oct 2017 00:37:13 -0000	1.239
+++ ufs_vnops.c	25 Feb 2019 05:49:21 -0000
@@ -1266,9 +1266,20 @@ ufs_readdir(void *v)
 		return EINVAL;
 	}
 
+#define UFS_READDIR_ARBITRARY_MAX (1024*1024)
+	if (callerbytes > UFS_READDIR_ARBITRARY_MAX) {
+		callerbytes = UFS_READDIR_ARBITRARY_MAX;
+	}
+
 	/* round start and end down to block boundaries */
 	physstart = startoffset & ~(off_t)(ump->um_dirblksiz - 1);
 	physend = endoffset & ~(off_t)(ump->um_dirblksiz - 1);
+	/* but if the region is within a single block, read at least one */
+	if (physstart == physend) {
+		physend += ump->um_dirblksiz;
+	}
+
+	/* compute the amount to ignore at each end */
 	skipstart = startoffset - physstart;
 	dropend = endoffset - physend;
 



-- 
David A. Holland
dholland%netbsd.org@localhost