Hi Tom,
On Mon, May 26, 2025 at 3:25 PM Tom Rini <tr...@konsulko.com> wrote:
>
> On Thu, May 22, 2025 at 10:02:01PM -0700, Tony Dinh wrote:
>
> > Use lbaint_t for blknr to avoid overflow in ext4fs_read_file().
> >
> > Background:
> >
> > blknr (block number) used in ext4fs_read_file() could be increased to a
> > very large value and causes a wrap around at 32 bit signed integer max,
> > thus becomes negative. This results in an out-of-normal range for sector
> > number (during the assignment delayed_start = blknr) where delayed_start
> > sector is typed uint64 lbaint_t. This causes the "Read outside partition"
> > error.
> >
> > This patch was tested on the Synology DS116 (Armada 385) board, and a
> > 4TB Seagate HDD.
> >
> > Signed-off-by: Tony Dinh <mibo...@gmail.com>
> > ---
> >
> > fs/ext4/ext4fs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
> > index 1727da2dc6d..f10c622e62c 100644
> > --- a/fs/ext4/ext4fs.c
> > +++ b/fs/ext4/ext4fs.c
> > @@ -101,7 +101,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t
> > pos,
> > blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
> >
> > for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
> > - long int blknr;
> > + lbaint_t blknr;
> > int blockoff = pos - (blocksize * i);
> > int blockend = blocksize;
> > int skipfirst = 0;
>
> Sorry, I should have reviewed this deeper before accepting it. This
> patch isn't right because it introduces a new problem, we ignore all
> failures from read_allocated_block() now because we can never return a
> negative value. This was reported by Coverity as CID 131183 and I need
> to revert this for now.
Thanks! Indeed that was bad. I'll take a look again.
All the best,
Tony
>
> --
> Tom
