All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gabriel Krisman Bertazi <krisman@suse.de>
To: Eric Biggers <ebiggers@kernel.org>
Cc: viro@zeniv.linux.org.uk, brauner@kernel.org, tytso@mit.edu,
	jaegeuk@kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Gabriel Krisman Bertazi <krisman@collabora.com>
Subject: Re: [PATCH v4 3/7] libfs: Validate negative dentries in case-insensitive directories
Date: Thu, 03 Aug 2023 13:37:45 -0400	[thread overview]
Message-ID: <875y5w10ye.fsf@suse.de> (raw)
In-Reply-To: <20230729042048.GB4171@sol.localdomain> (Eric Biggers's message of "Fri, 28 Jul 2023 21:20:48 -0700")

Eric Biggers <ebiggers@kernel.org> writes:

> On Thu, Jul 27, 2023 at 01:28:39PM -0400, Gabriel Krisman Bertazi wrote:
>>   - In __lookup_slow, either the parent inode is read locked by the
>>     caller (lookup_slow), or it is called with no flags (lookup_one*).
>>     The read lock suffices to prevent ->d_name modifications, with the
>>     exception of one case: __d_unalias, will call __d_move to fix a
>>     directory accessible from multiple dentries, which effectively swaps
>>     ->d_name while holding only the shared read lock.  This happens
>>     through this flow:
>> 
>>     lookup_slow()  //LOOKUP_CREATE
>>       d_lookup()
>>         ->d_lookup()
>>           d_splice_alias()
>>             __d_unalias()
>>               __d_move()
>> 
>>     Nevertheless, this case is not a problem because negative dentries
>>     are not allowed to be moved with __d_move.
>
> Isn't it possible for a negative dentry to become a positive one concurrently?

Do you mean d_splice_alias racing with a dentry instantiation and
__d_move being called on a negative dentry that is turning positive?

It is not possible for __d_move to be called with a negative dentry for
d_splice_alias, since the inode->i_lock is locked during __d_find_alias,
so it can't race with __d_instantiate or d_add. Then, __d_find_alias
can't find negative dentries in the first place, so we either have a
positive dentry, in which case __d_move is fine with regard to
d_revalidate_name, or we don't have any aliases and don't call
__d_move.

Can you clarify what problem you see here?

-- 
Gabriel Krisman Bertazi

WARNING: multiple messages have this Message-ID (diff)
From: Gabriel Krisman Bertazi <krisman@suse.de>
To: Eric Biggers <ebiggers@kernel.org>
Cc: brauner@kernel.org, tytso@mit.edu,
	linux-f2fs-devel@lists.sourceforge.net, viro@zeniv.linux.org.uk,
	linux-fsdevel@vger.kernel.org, jaegeuk@kernel.org,
	linux-ext4@vger.kernel.org,
	Gabriel Krisman Bertazi <krisman@collabora.com>
Subject: Re: [f2fs-dev] [PATCH v4 3/7] libfs: Validate negative dentries in case-insensitive directories
Date: Thu, 03 Aug 2023 13:37:45 -0400	[thread overview]
Message-ID: <875y5w10ye.fsf@suse.de> (raw)
In-Reply-To: <20230729042048.GB4171@sol.localdomain> (Eric Biggers's message of "Fri, 28 Jul 2023 21:20:48 -0700")

Eric Biggers <ebiggers@kernel.org> writes:

> On Thu, Jul 27, 2023 at 01:28:39PM -0400, Gabriel Krisman Bertazi wrote:
>>   - In __lookup_slow, either the parent inode is read locked by the
>>     caller (lookup_slow), or it is called with no flags (lookup_one*).
>>     The read lock suffices to prevent ->d_name modifications, with the
>>     exception of one case: __d_unalias, will call __d_move to fix a
>>     directory accessible from multiple dentries, which effectively swaps
>>     ->d_name while holding only the shared read lock.  This happens
>>     through this flow:
>> 
>>     lookup_slow()  //LOOKUP_CREATE
>>       d_lookup()
>>         ->d_lookup()
>>           d_splice_alias()
>>             __d_unalias()
>>               __d_move()
>> 
>>     Nevertheless, this case is not a problem because negative dentries
>>     are not allowed to be moved with __d_move.
>
> Isn't it possible for a negative dentry to become a positive one concurrently?

Do you mean d_splice_alias racing with a dentry instantiation and
__d_move being called on a negative dentry that is turning positive?

It is not possible for __d_move to be called with a negative dentry for
d_splice_alias, since the inode->i_lock is locked during __d_find_alias,
so it can't race with __d_instantiate or d_add. Then, __d_find_alias
can't find negative dentries in the first place, so we either have a
positive dentry, in which case __d_move is fine with regard to
d_revalidate_name, or we don't have any aliases and don't call
__d_move.

Can you clarify what problem you see here?

-- 
Gabriel Krisman Bertazi


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2023-08-03 17:37 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-27 17:28 [PATCH v4 0/7] Support negative dentries on case-insensitive ext4 and f2fs Gabriel Krisman Bertazi
2023-07-27 17:28 ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 1/7] fs: Expose name under lookup to d_revalidate hook Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-28 14:00   ` Christian Brauner
2023-07-28 14:00     ` [f2fs-dev] " Christian Brauner
2023-07-28 15:49     ` Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 2/7] fs: Add DCACHE_CASEFOLDED_NAME flag Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-29  4:34   ` Eric Biggers
2023-07-29  4:34     ` [f2fs-dev] " Eric Biggers
2023-07-27 17:28 ` [PATCH v4 3/7] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-28 13:06   ` Christian Brauner
2023-07-28 13:06     ` [f2fs-dev] " Christian Brauner
2023-07-28 15:09     ` Gabriel Krisman Bertazi
2023-07-29  4:18       ` Eric Biggers
2023-07-29  4:18         ` [f2fs-dev] " Eric Biggers
2023-07-29  4:20   ` Eric Biggers
2023-07-29  4:20     ` [f2fs-dev] " Eric Biggers
2023-08-03 17:37     ` Gabriel Krisman Bertazi [this message]
2023-08-03 17:37       ` Gabriel Krisman Bertazi
2023-08-04  4:41       ` Eric Biggers
2023-08-04  4:41         ` [f2fs-dev] " Eric Biggers
2023-08-08  1:33         ` Gabriel Krisman Bertazi
2023-08-08  1:33           ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-29  4:51   ` Eric Biggers
2023-07-29  4:51     ` [f2fs-dev] " Eric Biggers
2023-08-03 16:56     ` Gabriel Krisman Bertazi
2023-08-03 16:56       ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 4/7] libfs: Chain encryption checks after case-insensitive revalidation Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 6/7] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 17:28 ` [PATCH v4 7/7] f2fs: " Gabriel Krisman Bertazi
2023-07-27 17:28   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 18:13 ` [PATCH v4 0/7] Support negative dentries on case-insensitive ext4 and f2fs Theodore Ts'o
2023-07-27 18:13   ` [f2fs-dev] " Theodore Ts'o
2023-07-27 18:39   ` Gabriel Krisman Bertazi
2023-07-27 18:39     ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-07-27 19:41     ` Theodore Ts'o
2023-07-27 19:41       ` [f2fs-dev] " Theodore Ts'o
2023-07-28  7:45   ` Christian Brauner
2023-07-28  7:45     ` [f2fs-dev] " Christian Brauner
2023-07-28 11:21 ` Christian Brauner
2023-07-28 11:21   ` [f2fs-dev] " Christian Brauner

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=875y5w10ye.fsf@suse.de \
    --to=krisman@suse.de \
    --cc=brauner@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=krisman@collabora.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /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.