linux-kernel - Re: [RFC PATCH 2/3] fat: add renameat2 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] [day] [month] [year] [list]

Message-ID: <87a6b8tc0e.fsf@mail.parknet.co.jp>
Date:   Tue, 24 May 2022 02:05:53 +0900
From:   OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     Colin Walters <walters@...bum.org>, linux-kernel@...r.kernel.org,
        Peter Jones <pjones@...hat.com>,
        Alexander Larsson <alexl@...hat.com>,
        Alberto Ruiz <aruiz@...hat.com>,
        Christian Kellner <ckellner@...hat.com>,
        Lennart Poettering <lennart@...ttering.net>,
        Chung-Chiang Cheng <cccheng@...ology.com>
Subject: Re: [RFC PATCH 2/3] fat: add renameat2 RENAME_EXCHANGE flag support

Javier Martinez Canillas <javierm@...hat.com> writes:

>> So assuming we have this, I guess the flow could be:
>> 
>> 1. rename_exchange(old, new)
>> 2. syncfs()
>>
>
> Correct. In fact, Alex pointed me out that I should do sync in the test too
> before checking that the rename succeeded. I was mostly interested that the
> logic worked even if only the in-memory representation or page cache was
> used. But I've added a `sudo sync -f "${MNT_PATH}"` for the next iteration.
>  
>> ?  But that's assuming that the implementation of this doesn't e.g. have any "holes" where in theory we could flush an intermediate state.
>> 
>
> Ogawa said that didn't fully review it yet but gave useful feedback that I
> will also address in the next version. As said, is my first contribution to
> a filesystem driver so it would be good if people with more experience can
> let me know if there are holes in the implementation.

I'm not reading emails about ostree and stuff, so I may not understand
the issue though. If you are expecting the atomics on disk (not
in-core), rename exchange can't provide atomics on vfat without non
standard extension like adding journal or such. And even any syncfs(2)
can't prevent rename corruption, syncfs(2) can just only minimize the
race window.

If power failure happened on rename exchange, the file may lost in worst
case. (If had journal, file can recover to before or after rename
exchange while journal replay, but as you know vfat can't)

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>