From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Nikolay Borisov <nborisov@suse.com>, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 1/5] btrfs: extent_io: Do extra check for extent buffer read write functions
Date: Wed, 31 Jul 2019 15:47:25 +0200 [thread overview]
Message-ID: <20190731134725.GL28208@twin.jikos.cz> (raw)
In-Reply-To: <403a5d56-6d70-d983-25cc-e8481922f56d@gmx.com>
On Thu, Jul 25, 2019 at 02:58:08PM +0800, Qu Wenruo wrote:
> [snip]
> >> - if (dst_offset + len > dst->len) {
> >> - btrfs_err(fs_info,
> >> - "memmove bogus dst_offset %lu move len %lu dst len %lu",
> >> - dst_offset, len, dst->len);
> >> - BUG();
> >> - }
> >> + if (check_eb_range(dst, dst_offset, len) ||
> >> + check_eb_range(dst, src_offset, len))
> >> + return;
> >
> > I'm not sure about this. If the code expects memcpy_extent_buffer to
> > never fail then it will make more sense to do the range check outside of
> > this function. Otherwise it might silently fail and cause mayhem up the
> > call chain. Or just leave the BUG (I'd rather not).
>
> Yes, that's also what I'm concerned.
>
> But at least, for that case we're not making things worse.
> Furthermore, btrfs tree checker should have already rejected most
> invalid trees already.
> So I'm not so concerned.
I think this is the valid use of BUG, the check is supposed something
that's verified in advance and it must not happen inisdie the extent
buffer functions. Stress on the 'must not', so if it happens something
is seriously wrong, like a memory bitflip or accidental overwrite by
some other code and the BUG is there to stop propagating the error.
next prev parent reply other threads:[~2019-07-31 13:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 6:12 [PATCH v2 0/5] btrfs: Enhanced runtime defence against fuzzed images Qu Wenruo
2019-07-25 6:12 ` [PATCH v2 1/5] btrfs: extent_io: Do extra check for extent buffer read write functions Qu Wenruo
2019-07-25 6:44 ` Nikolay Borisov
2019-07-25 6:58 ` Qu Wenruo
2019-07-31 13:47 ` David Sterba [this message]
2019-07-25 6:12 ` [PATCH v2 2/5] btrfs: extent-tree: Kill BUG_ON() in __btrfs_free_extent() and do better comment Qu Wenruo
2019-07-25 8:39 ` Nikolay Borisov
2019-07-25 9:31 ` Qu Wenruo
2019-07-30 14:59 ` kbuild test robot
2019-07-25 6:12 ` [PATCH v2 3/5] btrfs: Detect unbalanced tree with empty leaf before crashing btree operations Qu Wenruo
2019-07-25 9:26 ` Nikolay Borisov
2019-07-25 9:34 ` Qu Wenruo
2019-08-06 13:58 ` David Sterba
2019-08-06 14:04 ` Qu Wenruo
2019-08-06 17:47 ` David Sterba
2019-08-07 2:22 ` Qu Wenruo
2019-08-07 6:08 ` Qu Wenruo
2019-07-25 6:12 ` [PATCH v2 4/5] btrfs: extent-tree: Kill the BUG_ON() in insert_inline_extent_backref() Qu Wenruo
2019-07-25 10:20 ` Nikolay Borisov
2019-07-25 6:12 ` [PATCH v2 5/5] btrfs: ctree: Checking key orders before merged tree blocks Qu Wenruo
2019-07-25 6:49 ` [PATCH v2 0/5] btrfs: Enhanced runtime defence against fuzzed images Nikolay Borisov
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=20190731134725.GL28208@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=quwenruo.btrfs@gmx.com \
--cc=wqu@suse.com \
/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).