linux-ext4 - Re: [PATCH v3] ext4: fix bug for rename with RENAME

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, 22 Jan 2021 11:20:51 -0800
From:   harshad shirwadkar <harshadshirwadkar@...il.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Amir Goldstein <amir73il@...il.com>,
        "Theodore Ts'o" <tytso@....edu>, yangerkun <yangerkun@...wei.com>,
        Ext4 <linux-ext4@...r.kernel.org>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jan Kara <jack@...e.cz>, "zhangyi (F)" <yi.zhang@...wei.com>,
        lihaotian <lihaotian9@...wei.com>, lutianxiong@...wei.com,
        linfeilong <linfeilong@...wei.com>,
        fstests <fstests@...r.kernel.org>,
        Vijaychidambaram Velayudhan Pillai <vijay@...utexas.edu>
Subject: Re: [PATCH v3] ext4: fix bug for rename with RENAME_WHITEOUT

Thanks Amir for pointing that out. Yes we are missing fast commit
tracking in whiteout. I'll send out a fix for that.

> But I must say it would have been very hard to catch missing ext4_fc_track_*
> without specialized fs fuzzer such as the CrashMonkey generated tests.

I agree, it's been on my to-do list to run CrashMonkey tests with fast
commits. I'm curious what kind of CrashMonkey tests you ran that
helped you catch this? Were you running Overlayfs on top of Ext4 with
fast commits?

Thanks,
Harshad

On Wed, Jan 20, 2021 at 12:42 AM Miklos Szeredi <miklos@...redi.hu> wrote:
>
> On Wed, Jan 20, 2021 at 7:57 AM Amir Goldstein <amir73il@...il.com> wrote:
>
> > And as long as I am ranting, I'd like to point out that it is a shame
> > that whiteout
> > was not implemented as a special (constant) inode whose nlink is irrelevant
> > (or a special dirent with d_ino 0 and d_type DT_WHT for that matter).
> > It would have been a rather small RO_COMPAT on-disk change for ext4.
> > It could also be implemented in slightly more backward compat manner by
> > maintaining a valid nlink and postpone setting the RO_COMPAT flag until
> > EXT4_LINK_MAX is reached.
> >
> > As things stand now, overlayfs makes an effort to maintain a singleton
> > hardlinked whiteout inode, without being able to use it with RENAME_WHITEOUT
> > and filesystems have to take special care to journal the metadata of all
> > individual whiteout inodes, without any added value to the only user
> > (overlayfs).
> >
> > But I guess that train has left the station long ago...
>
> Not so, I believe.  Kernel internal interfaces are easy to change, and
> adding support for DT_WHT to overlayfs would mostly be a trivial
> undertaking.
>
> The big issue (as always) is userspace API's and not introducing
> DT_WHT there was a very deliberate choice.  Adding a translation layer
> from an internal whiteout representation to the userspace API also
> does not seem to be a very complex problem, but I haven't looked into
> that deeply.
>
> So AFAICS there's really nothing preventing the addition of whiteout
> objects to filesystems, other than developer dedication.
>
> Thanks,
> Miklos