Re: [PATCH V7] squashfs: Add symlink size check in squash_read_inode

[Al Viro]

On Mon, Aug 05, 2024 at 09:02:31AM +0800, Lizhi Xu wrote:
> On Sun, 4 Aug 2024 22:20:34 +0100, Al Viro wrote:
> > Alternatively, just check ->i_size after assignment.  loff_t is
> > always a 64bit signed; le32_to_cpu() returns 32bit unsigned.
> > Conversion from u32 to s64 is always going to yield a non-negative
> > result; comparison with PAGE_SIZE is all you need there.

> It is int overflow, not others. 

Excuse me, what?

> Please see my V7 patch,
> Link: https://lore.kernel.org/all/20240803074349.3599957-1-lizhi.xu@xxxxxxxxxxxxx/

I have seen your patch.  Integer overflow has nothing to do with
the problem here.

Please, show me an unsigned int value N such that

_Bool mismatch(unsigned int N)
{
	u32 v32 = N;
	loff_t v64 = N;

	return (v32 > PAGE_SIZE) != (v64 > PAGE_SIZE);
}

would yield true if passed that value as an argument.

Note that assignment of le32_to_cpu() result to inode->i_size
does conversion from unsigned 32bit integer type to a signed 64bit
integer type.  Since the range of the former fits into the range of the
latter, conversion preserves value.  In other words, possible values
of inode->i_size after such assignment are from 0 to (loff_t)0xfffffff
and (inode->i_size > PAGE_SIZE) is true in exactly the same cases when
(symlink_size > PAGE_SIZE) is true with your patch.

Again, on all architectures inode->i_size is capable of representing
all values in range 0..4G-1 (for rather obvious reasons - we want the
kernel to be able to work with files larger than 4Gb).  There is
no wraparound of any kind on that assignment.

See https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf,
in particular sections 6.5.16.1[2] and 6.3.1.3[1]