archive mirror
 help / color / mirror / Atom feed
From: Al Viro <>
To: Liangyan <>
Cc: Joseph Qi <>,
	Miklos Szeredi <>,,
Subject: Re: [PATCH v2] ovl: fix dentry leak in ovl_get_redirect
Date: Mon, 21 Dec 2020 17:35:38 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Dec 22, 2020 at 12:51:27AM +0800, Liangyan wrote:
> This is the race scenario based on call trace we captured which cause the
> dentry leak.
>      CPU 0                                CPU 1
> ovl_set_redirect                       lookup_fast
>   ovl_get_redirect                       __d_lookup
>     dget_dlock
>       //no lock protection here            spin_lock(&dentry->d_lock)
>       dentry->d_lockref.count++            dentry->d_lockref.count++
> If we use dget_parent instead, we may have this race.
>      CPU 0                                    CPU 1
> ovl_set_redirect                           lookup_fast
>   ovl_get_redirect                           __d_lookup
>     dget_parent
>       raw_seqcount_begin(&dentry->d_seq)      spin_lock(&dentry->d_lock)
>       lockref_get_not_zero(&ret->d_lockref)   dentry->d_lockref.count++


lockref_get_not_zero() will observe ->d_lock held and fall back to
taking it.

The whole point of lockref is that counter and spinlock are next to each
other.  Fastpath in lockref_get_not_zero is cmpxchg on both, and
it is taken only if ->d_lock is *NOT* locked.  And the slow path
there will do spin_lock() around the manipulations of ->count.

Note that ->d_lock is simply ->d_lockref.lock; ->d_seq has nothing
to do with the whole thing.

The race in mainline is real; if you can observe anything of that
sort with dget_parent(), we have much worse problem.  Consider
dget() vs. lookup_fast() - no overlayfs weirdness in sight and the
same kind of concurrent access.

Again, lockref primitives can be safely mixed with other threads
doing operations on ->count while holding ->lock.

  reply	other threads:[~2020-12-21 18:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-20 12:09 [PATCH v2] ovl: fix dentry leak in ovl_get_redirect Liangyan
2020-12-21  6:14 ` Liangyan
2020-12-21  6:26 ` Al Viro
2020-12-21 11:14   ` Joseph Qi
2020-12-21 12:11     ` Al Viro
2020-12-21 16:51       ` Liangyan
2020-12-21 17:35         ` Al Viro [this message]
2020-12-21 18:15           ` Liangyan

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).