On Tue, 6 Aug 2024 05:59:05 +0100, Al Viro wrote:
> > > 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);
> > > }
> > This always return 0, why are you asking this?
>
> Because that implies the equivalence between
>
> symlink_size = le32_to_cpu(something);
> if (symlink_size > PAGE_SIZE)
> return -EINVAL;
> inode->i_size = symlink_size;
>
> and
>
> inode->i_size = le32_to_cpu(something);
> if (inode->i_size > PAGE_SIZE)
> return -EINVAL;
>
> However, you seem to find some problem in the latter form, and
> your explanations of the reasons have been hard to understand.
Here are the uninit-value related calltrace reports from syzbot:
page_get_link()->
read_mapping_page()->
read_cache_page()->
do_read_cache_page()->
do_read_cache_folio()->
filemap_read_folio()->
squashfs_symlink_read_folio()
fs/squashfs/symlink.c
8 static int squashfs_symlink_read_folio(struct file *file, struct folio *folio)
7 {
6 struct inode *inode = folio->mapping->host;
5 struct super_block *sb = inode->i_sb;
4 struct squashfs_sb_info *msblk = sb->s_fs_info;
3 int index = folio_pos(folio);
2 u64 block = squashfs_i(inode)->start;
1 int offset = squashfs_i(inode)->offset;
41 int length = min_t(int, i_size_read(inode) - index, PAGE_SIZE);
Please see line 41, because the value of i_size is too large, causing integer overflow
in the variable length. Which can result in folio not being initialized (as reported by
Syzbot: "KMSAN: uninit-value in pick_link").
My solution is to check if the value of symlink_size is too large before
initializing i_size with symlink_size. If it is, return -EINVAL.
>
> > > 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.
>
> > The type of loff_t is long long, so its values range is not 0..4G-1.
>
> 6.3.1.3[1] When a value with integer type is converted to another integer type
> other than _Bool, if the value can be represented by the new type, it is unchanged.
>
> Possible values of u32 are all in range 0..4G-1. All numbers in that range
> (and many others as well, but that is irrelevant here) can be represented by
> loff_t. In other words, nothing overflow-related is happening.
--
Lizhi
