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
dholl...@netbsd.org
