linux-ext4 - Re: [PATCH 3/4] tests: new test to check quota after directory optimization

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] [day] [month] [year] [list]

Message-ID: <87r0fm5vze.fsf@brahms.olymp>
Date: Wed, 03 Apr 2024 16:03:49 +0100
From: Luis Henriques <luis.henriques@...ux.dev>
To: Andreas Dilger <adilger@...ger.ca>
Cc: Theodore Ts'o <tytso@....edu>,  linux-ext4@...r.kernel.org
Subject: Re: [PATCH 3/4] tests: new test to check quota after directory
 optimization

Luis Henriques <luis.henriques@...ux.dev> writes:

> Andreas Dilger <adilger@...ger.ca> writes:
>
>> On Mar 28, 2024, at 11:29 AM, Luis Henriques (SUSE) <luis.henriques@...ux.dev> wrote:
>>> 
>>> This new test validates e2fsck by verifying that quota data is updated
>>> after a directory optimization is performed.  It mimics fstest ext4/014
>>> by including a filesystem image where a file is created inside a new
>>> directory on the filesystem root and then root block 0 is wiped:
>>> 
>>>  # debugfs -w -R 'zap -f / 0' f_testnew/image
>>
>> I appreciate the test case, and I hate to be difficult, but IMHO this
>> test case is not ideal.  It is *still* reporting quota inconsistency
>> at the end, so it is difficult to see whether the patch is actually
>> improving anything or not?
>
> Maybe I misunderstood how the tests really work.  Here's what I
> understood:
>
> e2fsck is run twice.  During the first run the filesystem is recovered.
> And that's the output of expect.1 -- it reports the quota inconsistency
> because quota data needs to be fixed.  And it is fixed in that first run,
> where e2fsck returns '1' ("File system errors corrected").  The second
> time e2fsck is run (expect.2) it will do nothing, and '0' is returned
> because the filesystem hasn't been modified.
>
> Without the first patch in this series the second time e2fsck is executed
> it will still fail and report inconsistencies because the first time the
> fix wasn't correct.  (And after this second time the filesystem should
> actually be corrected, a third run of e2fsck should return '0'.)
>
>> This is because the image is testing a number of different things at
>> once (repairing the root inode, superblock, etc).  IMHO, it would be
>> better to have this test be specific to the directory shrink issue
>> (e.g. a large directory is created, many files are deleted from it,
>> then optimized), and ideally have a non-root user, group, and project
>> involved so that it is verifying that all of the quotas are updated.
>
> Right, that makes sense.  However, I'm failing to narrow the test to that
> specific case.  I've tried to create a bunch of files in a directory and
> used the debugfs 'kill_file' to remove files from that directory.
> However, in that case e2fsck isn't reporting quota inconsistencies as I
> would expect.  Which may hint at yet more quota-related bugs.  But I'm
> still looking.

OK, I _may_ have found a simple way to generate an image to test my patch.
Here's what I came up with:

    make testnew
    tune2fs -O quota f_testnew/image

    debugfs -w -R "ln lost+found foo" f_testnew/image
    debugfs -w -R "unlink lost+found" f_testnew/image

    echo "update quota on directory optimization" > f_testnew/name
    make testend
    mv f_testnew f_quota_shrinkdir

This will trigger a directory optimization after the recreation of the
lost+found directory.  Do you think this would be good enough?

Cheers,
-- 
Luis