linux-kernel - Re: [PATCH V2] f2fs: fix to adjust appropriate length for fiemap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHJ8P3J+8xEwNL3S6JoR=E0_RczgwvHAGp8jZs5GZESza6n9LQ@mail.gmail.com>
Date: Tue, 5 Nov 2024 12:02:45 +0800
From: Zhiguo Niu <niuzhiguo84@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: Zhiguo Niu <zhiguo.niu@...soc.com>, jaegeuk@...nel.org, 
	linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org, 
	ke.wang@...soc.com, Hao_hao.Wang@...soc.com
Subject: Re: [PATCH V2] f2fs: fix to adjust appropriate length for fiemap

Chao Yu <chao@...nel.org> 于2024年11月5日周二 11:15写道：
>
> On 2024/11/4 9:56, Zhiguo Niu wrote:
> > If user give a file size as "length" parameter for fiemap
> > operations, but if this size is non-block size aligned,
> > it will show 2 segments fiemap results even this whole file
> > is contiguous on disk, such as the following results:
> >
> >   ./f2fs_io fiemap 0 19034 ylog/analyzer.py
> > Fiemap: offset = 0 len = 19034
> >          logical addr.    physical addr.   length           flags
> > 0       0000000000000000 0000000020baa000 0000000000004000 00001000
> > 1       0000000000004000 0000000020bae000 0000000000001000 00001001
> >
> > after this patch:
> > ./f2fs_io fiemap 0 19034 ylog/analyzer.py
> > Fiemap: offset = 0 len = 19034
> >      logical addr.    physical addr.   length           flags
> > 0    0000000000000000 00000000315f3000 0000000000005000 00001001
>
> Hi Zhiguo,
>
> Any testcase to reproduce this bug? w/o this patch, it looks output
> from fiemap looks fine?
>
> f2fs_io fiemap 0 19034 file
> Fiemap: offset = 0 len = 19034
>         logical addr.    physical addr.   length           flags
> 0       0000000000000000 0000000004401000 0000000000005000 00001001
>
Hi Chao,
Sorry I didn't write clearly enough about the test case, and i put the
note below the ""Singed-off" tag.
let me describe it again， f2fs_io fiemap has been modified by me for
testing in my local, and  the length parameter
is the real file size of the file, not the block numer. because user
also pass the real file size to fiemap ioctl.
so with the new f2fs_io fiemap, a contiguous file on disk may be shown
2 segments if the length is not block size alinged.
such as:

ums9632_1h10:/data # ls -l ylog/ap/analyzer.py
-rw-rw-rw- 1 root system 19006 2008-01-01 00:00 ylog/ap/analyzer.py
ums9632_1h10:/data # ./f2fs_io fiemap 0 19006 ylog/ap/analyzer.py
Fiemap: offset = 0 len = 19006
        logical addr.    physical addr.   length           flags
0       0000000000000000 0000000020baa000 0000000000004000 00001000
1       0000000000004000 0000000020bae000 0000000000001000 00001001

but if we pass a length that is block size alinged, it will show one
whole segment in fiemap log.
ums9632_1h10:/data # ./f2fs_io fiemap 0 20480 ylog/ap/analyzer.py
Fiemap: offset = 0 len = 20480
        logical addr.    physical addr.   length           flags
0       0000000000000000 0000000020baa000 0000000000005000 00001001
Thanks.
> Thanks,
>
> >
> > Signed-off-by: Zhiguo Niu <zhiguo.niu@...soc.com>
> > ---
> > V2: correct commit msg according to Chao's questions
> > f2fs_io has been modified for testing, the length for fiemap is
> > real file size, not block number
> > ---
> >   fs/f2fs/data.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 306b86b0..9fc229d 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >                       goto out;
> >       }
> >
> > -     if (bytes_to_blks(inode, len) == 0)
> > -             len = blks_to_bytes(inode, 1);
> > +     if (len & (blks_to_bytes(inode, 1) - 1))
> > +             len = round_up(len, blks_to_bytes(inode, 1));
> >
> >       start_blk = bytes_to_blks(inode, start);
> >       last_blk = bytes_to_blks(inode, start + len - 1);
>