[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] 9pfs: Fix divide by zero bug
From: |
Greg Kurz |
Subject: |
Re: [PATCH] 9pfs: Fix divide by zero bug |
Date: |
Sat, 23 Nov 2019 16:29:40 +0100 |
On Fri, 22 Nov 2019 22:53:09 +0100
Christian Schoenebeck <address@hidden> wrote:
> On Freitag, 22. November 2019 21:00:34 CET Dan Schatzberg wrote:
> > Some filesystems may return 0s in statfs (trivially, a FUSE filesystem
> > can do so). QEMU should handle this gracefully and just behave the
> > same as if statfs failed.
>
> Is that actually legal in non-error cases? Shouldn't a driver without a block
> size concept return 512 according to POSIX?
>
The first problem I see is that statfs() isn't POSIX. It is a linux-specific
implementation inspired by 4.4BSD. The equivalent system call in POSIX would
be statvfs():
https://pubs.opengroup.org/onlinepubs/9699919799/functions/statvfs.html
And even there, no details are provided about what f_bsize should contain,
apart from:
"It is unspecified whether all members of the statvfs structure have
meaningful values on all file systems."
A filesystem isn't necessarily backed by a block device, so I guess it
is legal to get a zero block size and we should definitely cope with it.
> > Signed-off-by: Dan Schatzberg <address@hidden>
> > ---
> > hw/9pfs/9p.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 37abcdb71e..520177f40c 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1834,8 +1834,10 @@ static int32_t coroutine_fn get_iounit(V9fsPDU *pdu,
> > V9fsPath *path) * and as well as less than (client msize - P9_IOHDRSZ))
> > */
> > if (!v9fs_co_statfs(pdu, path, &stbuf)) {
> > - iounit = stbuf.f_bsize;
> > - iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
> > + if (stbuf.f_bsize) {
> > + iounit = stbuf.f_bsize;
> > + iounit *= (s->msize - P9_IOHDRSZ) / stbuf.f_bsize;
> > + }
> > }
> > if (!iounit) {
> > iounit = s->msize - P9_IOHDRSZ;
>
> Nevertheless, since that will leave iounit initialized with zero and since
> there is already an !ionunit case handling there ...
>
This fix looks like the only sensible thing to do.
> Acked-by: Christian Schoenebeck <address@hidden>
>
Thanks for the Cc otherwise I would have certainly missed this one :)
I've pushed it to my 9p-fix branch and I'll send a pull request ASAP
in order to have this fixed in rc3.
> Best regards,
> Christian Schoenebeck
>
>