linux-ext4 - Re: [PATCH] e2fsck: delay quotas loading in release_orphan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Aug 2023 09:20:18 +0800
From:   Baokun Li <libaokun1@...wei.com>
To:     Andreas Dilger <adilger@...ger.ca>
CC:     Jan Kara <jack@...e.cz>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Theodore Ts'o <tytso@....edu>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Zhang Yi <yi.zhang@...wei.com>, <yangerkun@...wei.com>,
        <yukuai3@...wei.com>, Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH] e2fsck: delay quotas loading in release_orphan_inodes()

On 2023/8/25 3:49, Andreas Dilger wrote:
> On Aug 23, 2023, at 8:27 PM, Baokun Li <libaokun1@...wei.com> wrote:
>> On 2023/8/24 1:05, Jan Kara wrote:
>>> On Thu 17-08-23 16:18:28, Baokun Li wrote:
>>>> After 7d79b40b ("e2fsck: adjust quota counters when clearing orphaned
>>>> inodes"), we load all the quotas before we process the orphaned inodes,
>>>> and when we load the quotas, we check the checsum of the bbitmap for each
>>>> group. If one of the bbitmap checksums is wrong, the following error will
>>>> be reported:
>>>>
>>>> “Error initializing quota context in support library:
>>>>   Block bitmap checksum does not match bitmap”
>>>>
>>>> But loading quotas comes before checking the current superblock for the
>>>> EXT2_ERROR_FS flag, which makes it impossible to use e2fsck to repair any
>>>> image that contains orphan inodes and has the wrong bbitmap checksum.
>>>> So delaying quota loading until after the EXT2_ERROR_FS judgment avoids
>>>> the above problem.
>>>>
>>>> Signed-off-by: Baokun Li <libaokun1@...wei.com>
>>> This certainly looks better but I wonder if there still isn't a problem if
>>> the bitmap checksums are wrong but EXT2_ERROR_FS is not set. Shouldn't we
>>> rather move the initialization of the quota files after the call to
>>> e2fsck_read_bitmaps()?
>> When the bitmap checksums are wrong but EXT2_ERROR_FS is not set, we must
>> have lost some data (error flag or group descriptor or bitmap), so there is
>> something wrong with the kernel at this time, so I don't think we should
>> fix the image directly, but rather let the user realize that something is
>> wrong with the filesystem logic.
>>
>> Moreover, if we don't care how this happened, but just want to fix the image,
>> we only need to run "e2fsck -a" twice. After merging in the current patch, we
>> always empty the orphan list before loading the quotas, and EXT2_ERROR_FS
>> is set when loading the quotas fails, so this will be fixed the second time
>> you run e2fsck. It will not happen that every e2fsck will fail like it did
>> before.
> I recall that Ted prefers e2fsck to fix everything in a single pass, or at
> worst if a fatal problem hit during the run it should restart itself so
> that it will fix all of the problems before exiting.
>
> Cheers, Andreas
>
>
As mentioned earlier, when a bitmap checksums error occurs but EXT2_ERROR_FS
is not set, something may be wrong, so we should stop and tell the 
developer what
may be wrong, rather than just fixing it to hide the possible problem, 
which can
make it harder to locate some of the issues.

Cheers!
-- 
With Best Regards,
Baokun Li
.