On Tuesday, November 5, 2024 1:13:14 PM CET Greg Kurz wrote:
> On Tue, 5 Nov 2024 11:25:26 +0100
> Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
>
> > A bad (broken or malicious) 9p client (guest) could cause QEMU host to
> > crash by sending a 9p 'Treaddir' request with a numeric file ID (FID) that
> > was previously opened for a file instead of an expected directory:
> >
> > #0 0x0000762aff8f4919 in __GI___rewinddir (dirp=0xf) at
> > ../sysdeps/unix/sysv/linux/rewinddir.c:29
> > #1 0x0000557b7625fb40 in do_readdir_many (pdu=0x557bb67d2eb0,
> > fidp=0x557bb67955b0, entries=0x762afe9fff58, offset=0, maxsize=131072,
> > dostat=<optimized out>) at ../hw/9pfs/codir.c:101
> > #2 v9fs_co_readdir_many (pdu=pdu@entry=0x557bb67d2eb0,
> > fidp=fidp@entry=0x557bb67955b0, entries=entries@entry=0x762afe9fff58,
> > offset=0, maxsize=131072, dostat=false) at ../hw/9pfs/codir.c:226
> > #3 0x0000557b7625c1f9 in v9fs_do_readdir (pdu=0x557bb67d2eb0,
> > fidp=0x557bb67955b0, offset=<optimized out>,
> > max_count=<optimized out>) at ../hw/9pfs/9p.c:2488
> > #4 v9fs_readdir (opaque=0x557bb67d2eb0) at ../hw/9pfs/9p.c:2602
> >
> > That's because V9fsFidOpenState was declared as union type. So the
> > same memory region is used for either an open POSIX file handle (int),
> > or a POSIX DIR* pointer, etc., so 9p server incorrectly used the
> > previously opened (valid) POSIX file handle (0xf) as DIR* pointer,
> > eventually causing a crash in glibc's rewinddir() function.
> >
> > Root cause was therefore a missing check in 9p server's 'Treaddir'
> > request handler, which must ensure that the client supplied FID was
> > really opened as directory stream before trying to access the
> > aforementioned union and its DIR* member.
> >
> > Cc: qemu-sta...@nongnu.org
> > Fixes: d62dbb51f7 ("virtio-9p: Add fidtype so that we can do type ...")
> > Reported-by: Akihiro Suda <suda.ky...@gmail.com>
> > Tested-by: Akihiro Suda <suda.ky...@gmail.com>
> > Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com>
> > ---
> > hw/9pfs/9p.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
>
> Wow ! And this is there since pretty much always... I'm amazed how
> it went unnoticed for so long :-)
Yeah, a true hero, lasted for amazing 14 years. :)
Probably a good wake-up call for looking into that fuzzing environment
somebody made for 9pfs. I never tried it myself.
> Reviewed-by: Greg Kurz <gr...@kaod.org>
Thanks!
/Christian
