On Thu, May 29, 2008 at 05:56:07PM -0600, Andreas Dilger wrote:
> On May 28, 2008 11:02 +0200, Nick Piggin wrote:
> > fs: check for statfs overflow
> >
> > Adds a check for an overflow in the filesystem size so if someone is
> > checking with statfs() on a 16G hugetlbfs in a 32bit binary that it
> > will report back EOVERFLOW instead of a size of 0.
> >
> > Are other places that need a similar check? I had tried a similar
> > check in put_compat_statfs64 too but it didn't seem to generate an
> > EOVERFLOW in my test case.
> >
> > Signed-off-by: Jon Tollefson <kniht@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Nick Piggin <npiggin@xxxxxxx>
> > ---
> >
> > fs/compat.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >
> > Index: linux-2.6/fs/compat.c
> > ===================================================================
> > --- linux-2.6.orig/fs/compat.c
> > +++ linux-2.6/fs/compat.c
> > @@ -197,8 +197,8 @@ static int put_compat_statfs(struct comp
> > {
> >
> > if (sizeof ubuf->f_blocks == 4) {
> > - if ((kbuf->f_blocks | kbuf->f_bfree | kbuf->f_bavail) &
> > - 0xffffffff00000000ULL)
> > + if ((kbuf->f_blocks | kbuf->f_bfree | kbuf->f_bavail |
> > + kbuf->f_bsize | kbuf->f_frsize) & 0xffffffff00000000ULL)
> > return -EOVERFLOW;
>
> Hmm, doesn't this check break every filesystem > 16TB on 4kB PAGE_SIZE
> nodes? It would be better, IMHO, to scale down f_blocks, f_bfree, and
> f_bavail and correspondingly scale up f_bsize to fit into the 32-bit
> statfs structure.
Oh? Hmm, from my reading, such filesystems will already overflow f_blocks
check which is already there. Jon's patch only adds checks for f_bsize
and f_frsize.
One thing I'm a little worried about is the _exact_ semantics required
of the syscall wrt overflow, and type sizes. In the man page here for
example, ubuf->f_blocks is a differnt type to f_bsize and f_frsize...
Thanks,
Nick
> We did this for several years with Lustre, as the first installation was
> already larger than 16TB on 32-bit clients at the time. There was never
> a problem with statfs returning a larger f_bsize, since applications
> generally use the fstat() st_blocksize to determine IO size and not the
> statfs() data.
>
> Returning statfs data accurate to within a few kB is better than failing
> the request outright, IMHO.
>
> Cheers, Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
