linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wolfgang Frisch <wolfgang.frisch@suse.com>
To: Jan Kara <jack@suse.cz>, Ritesh Harjani <riteshh@linux.ibm.com>
Cc: Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH] ext4: catch integer overflow in ext4_cache_extents
Date: Wed, 15 Jul 2020 19:25:59 +0200	[thread overview]
Message-ID: <8bfb84e8-8699-0206-bf31-8ad9877d1986@suse.com> (raw)
In-Reply-To: <20200714123122.GG23073@quack2.suse.cz>


[-- Attachment #1.1: Type: text/plain, Size: 1952 bytes --]

On 14/07/2020 14.31, Jan Kara wrote:
> On Mon 13-07-20 19:14:47, Ritesh Harjani wrote:
>> On 7/13/20 6:28 PM, Jan Kara wrote:
>>> From: Wolfgang Frisch <wolfgang.frisch@suse.com>
>>>
>>> When extent tree is corrupted we can hit BUG_ON in
>>> ext4_es_cache_extent(). Check for this and abort caching instead of
>>> crashing the machine.
>>
>> Was it intentionally made corrupted by crafting a corrupted disk image?
> 
> I'm not sure how Wolfgang hit the issue. I'd expect some fs image
> fuzzing... Wolfgang?The bug was discovered accidentally when we tried to use a physically
defective USB flash drive. It was possible to partition and format the
device but the subsequent mount operation unearthed this problem.

As it was impossible to image the defective drive entirely, I used
blktrace(8) to gather a minimal list of read operations executed by
mount . A script then copied just the necessary blocks from the device
into a sparse QCOW2 image that is attached to the original bug report [1].

>> Are there more such logic in place which checks for such corruption at other
>> places?
> 
> That's a good question. But now that I'm looking at it ext4_ext_check()
> should actually catch a corruption like this. It is only the path in
> ext4_find_extent()->ext4_cache_extents() that can face the issue so
> probably instead of a fix in ext4_cache_extents() we should rather add more
> careful extent info checks for the extents contained directly in the inode.
> I'll look into it.

That sounds very reasonable. I see you already implemented it!

Best regards
Wolfgang

[1] https://bugzilla.suse.com/show_bug.cgi?id=1173485

-- 
Wolfgang Frisch <wolfgang.frisch@suse.com>
Security Engineer
OpenPGP fingerprint: A2E6 B7D4 53E9 544F BC13  D26B D9B3 56BD 4D4A 2D15
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nuremberg, Germany
(HRB 36809, AG Nürnberg)
Managing Director: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      parent reply	other threads:[~2020-07-15 17:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 12:58 [PATCH] ext4: catch integer overflow in ext4_cache_extents Jan Kara
2020-07-13 13:44 ` Ritesh Harjani
2020-07-14 12:31   ` Jan Kara
2020-07-15 11:53     ` Jan Kara
2020-07-15 17:25     ` Wolfgang Frisch [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=8bfb84e8-8699-0206-bf31-8ad9877d1986@suse.com \
    --to=wolfgang.frisch@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.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).