From: Kees Cook <keescook@chromium.org>
To: Jan Kara <jack@suse.cz>
Cc: 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 10:43:09 -0800 [thread overview]
Message-ID: <201911141042.5B8B2BC4AB@keescook> (raw)
In-Reply-To: <20191114085812.GB28486@quack2.suse.cz>
On Thu, Nov 14, 2019 at 09:58:12AM +0100, Jan Kara wrote:
> 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...
Okay, cool. I'll mark it as such. Thanks!
--
Kees Cook
prev parent reply other threads:[~2019-11-15 5:29 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
2019-11-14 18:43 ` Kees Cook [this message]
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=201911141042.5B8B2BC4AB@keescook \
--to=keescook@chromium.org \
--cc=gustavo@embeddedor.com \
--cc=jack@suse.cz \
--cc=linux-next@vger.kernel.org \
--cc=mbobrowski@mbobrowski.org \
--cc=riteshh@linux.ibm.com \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).