From: Jan Kara <jack@suse•cz>
To: Kees Cook <keescook@chromium•org>
Cc: Jan Kara <jack@suse•cz>,
Matthew Bobrowski <mbobrowski@mbobrowski•org>,
Theodore Ts'o <tytso@mit•edu>,
Ritesh Harjani <riteshh@linux•ibm.com>,
"Gustavo A. R. Silva" <gustavo@embeddedor•com>,
linux-next@vger•kernel.org
Subject: Re: Coverity: ext4_iomap_alloc(): Integer handling issues
Date: Thu, 14 Nov 2019 09:58:12 +0100 [thread overview]
Message-ID: <20191114085812.GB28486@quack2.suse.cz> (raw)
In-Reply-To: <201911131036.2E3F280B9@keescook>
On Wed 13-11-19 10:38:43, Kees Cook wrote:
> 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. :)
Now I've noticed that set_blocksize() called from sb_set_blocksize()
already has these checks. So there's nothing to add. Just Coverity is not
able to carry over those limits that far...
Honza
--
Jan Kara <jack@suse•com>
SUSE Labs, CR
next prev parent reply other threads:[~2019-11-14 8:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-12 1:35 Coverity: ext4_iomap_alloc(): Integer handling issues coverity-bot
2019-11-12 7:22 ` Matthew Bobrowski
2019-11-12 11:00 ` Jan Kara
2019-11-12 20:56 ` Kees Cook
2019-11-12 21:28 ` Matthew Bobrowski
2019-11-12 22:17 ` Kees Cook
2019-11-13 4:38 ` Matthew Bobrowski
2019-11-13 9:37 ` Jan Kara
2019-11-13 18:38 ` Kees Cook
2019-11-14 8:58 ` Jan Kara [this message]
2019-11-14 18:43 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191114085812.GB28486@quack2.suse.cz \
--to=jack@suse$(echo .)cz \
--cc=gustavo@embeddedor$(echo .)com \
--cc=keescook@chromium$(echo .)org \
--cc=linux-next@vger$(echo .)kernel.org \
--cc=mbobrowski@mbobrowski$(echo .)org \
--cc=riteshh@linux$(echo .)ibm.com \
--cc=tytso@mit$(echo .)edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox