linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.de>
To: dsterba@suse.cz, Qu Wenruo <quwenruo.btrfs@gmx.com>,
	Qu Wenruo <wqu@suse.com>,
	linux-btrfs@vger.kernel.org, Nikolay Borisov <nborisov@suse.com>
Subject: Re: [PATCh v2 8/9] btrfs: tree-checker: Verify inode item
Date: Wed, 27 Mar 2019 08:13:35 +0800	[thread overview]
Message-ID: <32920968-24ef-34c1-0216-ef2ebb999747@suse.de> (raw)
In-Reply-To: <20190326160248.GV10640@twin.jikos.cz>



On 2019/3/27 上午12:02, David Sterba wrote:
> On Mon, Mar 25, 2019 at 12:27:24PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2019/3/20 下午2:37, Qu Wenruo wrote:
>>> There is a report in kernel bugzilla about mismatch file type in dir
>>> item and inode item.
>>>
>>> This inspires us to check inode mode in inode item.
>>>
>>> This patch will check the following members:
>>> - inode key objectid
>>>   Should be ROOT_DIR_DIR or [256, (u64)-256] or FREE_INO.
>>>
>>> - inode key offset
>>>   Should be 0
>>>
>>> - inode item generation
>>> - inode item transid
>>>   No newer than sb generation + 1.
>>>   The +1 is for log tree.
>>>
>>> - inode item mode
>>>   No unknown bits.
>>>   No invalid S_IF* bit.
>>>   NOTE: S_IFMT check is not enough, need to check every know type.
>>>
>>> - inode item nlink
>>>   Dir should have no more link than 1.
>>>
>>> - inode item flags
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>>
>> There is some bug report of kernel producing free space cache inode with
>> mode 0, which is invalid and can be detected by this patch.
>>
>> Although the patch itself is good, I'm afraid we need to address the
>> invalid inode mode created by old kernel in btrfs-progs at least before
>> merging this patch into upstream.
> 
> Can this be addressed on the kernel side? Like detecting the invalid
> mode, print a warning and the fix on the next write. The progs can
> detect and fix that too of course.

So far even on older fs images (like those in btrfs-progs fsck tests), I
noticed no such invalid free space inode at all.

And from the history of that code, the mode is fixed to 100600 since 2010.
Currently I believe it's uncommon to see that case.

Furthermore, the btrfs-progs fix for such case is already submitted, as
long as we have a minor release to include that fix, it should be OK.

> 
> So I'll keep the patch working as-is, we can relax the error to a
> warning if we're out of time or find out that it needs to be that way
> due to backward compatibilit reasons.
> 

  reply	other threads:[~2019-03-27  0:14 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20  6:37 [PATCh v2 0/9] btrfs: tree-checker: More enhancement for fuzzed Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 1/9] btrfs: Move btrfs_check_chunk_valid() to tree-check.[ch] and export it Qu Wenruo
2019-03-20 10:34   ` Johannes Thumshirn
2019-03-25 17:06   ` David Sterba
2019-03-25 23:02     ` Qu Wenruo
2019-03-26 14:34       ` David Sterba
2019-03-20  6:37 ` [PATCh v2 2/9] btrfs: tree-checker: Make chunk item checker more readable Qu Wenruo
2019-03-20 10:41   ` Johannes Thumshirn
2019-03-26 15:08     ` David Sterba
2019-03-20  6:37 ` [PATCh v2 3/9] btrfs: tree-checker: Make btrfs_check_chunk_valid() return EUCLEAN instead of EIO Qu Wenruo
2019-03-20 10:44   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 4/9] btrfs: tree-checker: Check chunk item at tree block read time Qu Wenruo
2019-03-20 10:56   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 5/9] btrfs: tree-checker: Verify dev item Qu Wenruo
2019-03-20 11:51   ` Johannes Thumshirn
2019-03-20 11:53     ` Qu Wenruo
2019-03-25 17:04       ` David Sterba
2019-04-06  1:07   ` Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 6/9] btrfs: Check the first key and level for cached extent buffer Qu Wenruo
2019-03-20 12:02   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 7/9] btrfs: tree-checker: Enhance chunk checker to validate chunk profiler Qu Wenruo
2019-03-20 12:38   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 8/9] btrfs: tree-checker: Verify inode item Qu Wenruo
2019-03-20 13:27   ` Johannes Thumshirn
2019-03-25  4:27   ` Qu Wenruo
2019-03-26 16:02     ` David Sterba
2019-03-27  0:13       ` Qu Wenruo [this message]
2019-03-26 15:27   ` David Sterba
2019-03-28 13:38   ` David Sterba
2019-03-28 13:42     ` Qu Wenruo
2019-03-28 13:57       ` David Sterba
2019-03-28 14:00         ` Qu Wenruo
2019-03-28 14:07           ` David Sterba
2019-03-28 14:13             ` Qu Wenruo
2019-03-28 14:25               ` David Sterba
2019-03-28 23:49                 ` Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 9/9] btrfs: inode: Verify inode mode to avoid NULL pointer dereference Qu Wenruo
2019-03-20 13:33   ` Johannes Thumshirn
2019-03-28 13:53   ` David Sterba
2019-03-28 13:58     ` Qu Wenruo
2019-03-28 14:02       ` David Sterba
2019-03-28 15:48 ` [PATCh v2 0/9] btrfs: tree-checker: More enhancement for fuzzed David Sterba

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=32920968-24ef-34c1-0216-ef2ebb999747@suse.de \
    --to=wqu@suse.de \
    --cc=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).