Subject: Re: Coverity: ext4_iomap_alloc(): Integer handling issues

From: Kees Cook <[email-masked]>
Date: Wed, 13 Nov 2019 10:38:43 -0800

On Wed, Nov 13, 2019 at 10:37:54AM +0100, Jan Kara wrote:
> Well, I don't think we want to clutter various places in the code with
> checks that inode->i_blkbits (which is what blkbits actually is) is what we
> expect. inode->i_blkbits is initialized in fs/inode.c:inode_init_always()
> from sb->s_blocksize_bits and never changed. sb->s_blocksize_bits gets set
> through sb_set_blocksize().  Now it would make sense to assert in
> sb_set_blocksize() that block size is in the range we expect it (currently
> there's just a comment there). But then I suspect that Coverity won't be
> able to carry over the limits as far as into ext4_iomap_alloc() code...
> Kees?

Yeah, I'm not sure it's capabilities in this regard. It's still a bit of a
black box. :) I just tend to lean toward adding asserts to code-document
value range expectations. Perhaps add the check in sb_set_blocksize()
just because it's a decent thing to test, and if Coverity doesn't notice,
that's okay -- my goal is to improve the kernel which may not always
reduce the static checker noise. :)

-- 
Kees Cook