Subject: [PATCH] misc: Use unsigned long long for file block count in filefrag

This patch fixes incorrect reporting of file block count on 32-bit platforms.

---
misc/filefrag.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/misc/filefrag.c b/misc/filefrag.c
index a050a22..756fad8 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -337,7 +337,7 @@ static void frag_report(const char *filename)
ext2fs_struct_stat st;
int blk_shift;
long fd;
- unsigned long numblocks;
+ unsigned long long numblocks;
int data_blocks_per_cyl = 1;
int num_extents = 1, expected = ~0;
int is_ext2 = 0;
@@ -409,7 +409,7 @@ static void frag_report(const char *filename)
if (width > logical_width)
logical_width = width;
if (verbose)
- printf("File size of %s is %llu (%lu block%s of %d bytes)\n",
+ printf("File size of %s is %llu (%llu block%s of %d bytes)\n",
filename, (unsigned long long)st.st_size,
numblocks * fsinfo.f_bsize >> blk_shift,
numblocks == 1 ? "" : "s", 1 << blk_shift);
--
1.9.0.rc3



2014-02-23 13:58:39

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] misc: Use unsigned long long for file block count in filefrag

On Sun, Feb 23, 2014 at 01:08:47PM +0400, Vitaliy Filippov wrote:
> This patch fixes incorrect reporting of file block count on 32-bit platforms.

Hi Vitaliy,

Thanks for the patch! Your patch is missing a Signed-Off-By footer.
Also, you note that numblocks gets passed to filefrag_fibmap(), so if
you're going to do this, and we fall back to using fibmap, you should
probably check to see if numblocks would overflow an unsigned long,
and if so, to give an error because fibmap ioctl only supports the use
of a ulong.

Cheers,

- Ted

Subject: Re: [PATCH] misc: Use unsigned long long for file block count in filefrag

> Hi Vitaliy,
>
> Thanks for the patch! Your patch is missing a Signed-Off-By footer.
> Also, you note that numblocks gets passed to filefrag_fibmap(), so if
> you're going to do this, and we fall back to using fibmap, you should
> probably check to see if numblocks would overflow an unsigned long,
> and if so, to give an error because fibmap ioctl only supports the use
> of a ulong.

OK, I've just resubmitted it :)

In fact, the block count was reported incorrectly for files bigger than
0xffffffff bytes, not blocks, because filefrag does (numblocks *
fsinfo.f_bsize >> blk_shift), and this overflows when numblocks is
32-bit...

--
With best regards,
Vitaliy Filippov