All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xavier Roche <xavier.roche@algolia.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Matthew Wilcox <willy@infradead.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Subject: Re: race between vfs_rename and do_linkat (mv and link)
Date: Wed, 16 Feb 2022 14:18:14 +0100	[thread overview]
Message-ID: <20220216131814.GA2463301@xavier-xps> (raw)
In-Reply-To: <YgzRwhavapo69CAn@miu.piliscsaba.redhat.com>

On Wed, Feb 16, 2022 at 11:28:18AM +0100, Miklos Szeredi wrote:
> Something like this:
> diff --git a/fs/namei.c b/fs/namei.c
> index 3f1829b3ab5b..dd6908cee49d 100644

Tested-by: Xavier Roche <xavier.roche@algolia.com>

I confirm this completely fixes at least the specific race. Tested on a
unpatched and then patched 5.16.5, with the trivial bash test, and then
with a C++ torture test.

Before:
-------

$ time ./linkbug
Failed after 4 with No such file or directory

real	0m0,004s
user	0m0,000s
sys	0m0,004s

After:
------

(no error after ten minutes of running the program)

Torture test program:
---------------------

/* Linux rename vs. linkat race condition.
 * Rationale: both (1) moving a file to a target and (2) linking the target to a file in parallel leads to a race
 * on Linux kernel.
 * Sample file courtesy of Xavier Grand at Algolia
 * g++ -pthread linkbug.c -o linkbug
 */

#include <thread>
#include <unistd.h>
#include <assert.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <iostream>
#include <string.h>

static const char* producedDir = "/tmp";
static const char* producedFile = "/tmp/file.txt";
static const char* producedTmpFile = "/tmp/file.txt.tmp";
static const char* producedThreadDir = "/tmp/tmp";
static const char* producedThreadFile = "/tmp/file.txt.tmp.2";

bool createFile(const char* path)
{
    const int fdOut = open(path,
                           O_WRONLY | O_CREAT | O_TRUNC | O_EXCL | O_CLOEXEC,
                           S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
    assert(fdOut != -1);
    assert(write(fdOut, "Foo", 4) == 4);
    assert(close(fdOut) == 0);
    return true;
}

void func()
{
    int nbSuccess = 0;
    // Loop producedThread a hardlink of the file
    while (true) {
        if (link(producedFile, producedThreadFile) != 0) {
            std::cout << "Failed after " << nbSuccess << " with " << strerror(errno) << std::endl;
            exit(EXIT_FAILURE);
        } else {
            nbSuccess++;
        }
        assert(unlink(producedThreadFile) == 0);
    }
}

int main()
{
    // Setup env
    unlink(producedTmpFile);
    unlink(producedFile);
    unlink(producedThreadFile);
    createFile(producedFile);
    mkdir(producedThreadDir, 0777);

    // Async thread doing a hardlink and moving it
    std::thread t(func);
    // Loop creating a .tmp and moving it
    while (true) {
        assert(createFile(producedTmpFile));
        assert(rename(producedTmpFile, producedFile) == 0);
    }
    return 0;
}

  reply	other threads:[~2022-02-16 13:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-14 21:07 fs: race between vfs_rename and do_linkat (mv and link) Xavier Roche
2022-02-15  9:56 ` Miklos Szeredi
2022-02-15 13:37   ` Al Viro
2022-02-15 16:06     ` Al Viro
2022-02-15 16:17       ` Matthew Wilcox
2022-02-15 16:20         ` Al Viro
2022-02-16  9:28           ` Miklos Szeredi
2022-02-16 10:28             ` Miklos Szeredi
2022-02-16 13:18               ` Xavier Roche [this message]
2022-02-16 13:37                 ` Miklos Szeredi
2022-02-18 15:37                   ` Miklos Szeredi
2022-02-15 16:18       ` Al Viro
2022-02-15 16:56       ` Xavier Roche

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=20220216131814.GA2463301@xavier-xps \
    --to=xavier.roche@algolia.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.