Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Jun 2008 13:29:00 +0300
From:      Jaakko Heinonen <jh@saunalahti.fi>
To:        freebsd-fs@freebsd.org
Subject:   =?utf-8?b?W3BhdGNoXcKgYnVn?= in cd9660 readdir code
Message-ID:  <20080605102900.GA1971@a91-153-120-204.elisa-laajakaista.fi>

next in thread | raw e-mail | index | archive | help

Hi,

There's a bug in cd9660_readdir() (src/sys/fs/cd9660/cd9660_vnops.c) which
may change the directory position (offset) to an invalid value.

The problem is that if all directory entries has been read and
idp->curroff >= endsearch the code doesn't enter to

	while (idp->curroff < endsearch) {

loop which initializes idp->uio_off. Later in code uio->uio_offset is
changed to idp->uio_off (which may be uninitialized).

The PR 122925 (http://www.freebsd.org/cgi/query-pr.cgi?pr=122925) has a
real life example of a problem caused by this bug. There's also a
stripped down test program attached to the PR. Problems include
readdir(3) restarting from random position and geom errors caused by
read attempts from bogus offsets.

Does following patch look good? Few people have tested the patch and
it has fixed problems for them. If the patch looks good could someone
consider committing it?

Index: cd9660_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/cd9660/cd9660_vnops.c,v
retrieving revision 1.113
diff -p -u -r1.113 cd9660_vnops.c
--- cd9660_vnops.c	15 Feb 2007 22:08:34 -0000	1.113
+++ cd9660_vnops.c	20 May 2008 06:45:20 -0000
@@ -495,6 +495,7 @@ cd9660_readdir(ap)
 	}
 	idp->eofflag = 1;
 	idp->curroff = uio->uio_offset;
+	idp->uio_off = uio->uio_offset;
 
 	if ((entryoffsetinblock = idp->curroff & bmask) &&
 	    (error = cd9660_blkatoff(vdp, (off_t)idp->curroff, NULL, &bp))) {

-- 
Jaakko



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080605102900.GA1971>