From: Wen Yang <wenyang@linux.alibaba.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sasha Levin <sashal@kernel.org>,
Xunlei Pang <xlpang@linux.alibaba.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry
Date: Fri, 8 Jan 2021 00:21:38 +0800 [thread overview]
Message-ID: <e0fa1641-d00b-acfc-91d7-9eb16fb61664@linux.alibaba.com> (raw)
In-Reply-To: <X/b781Kwn48xq8aS@kroah.com>
在 2021/1/7 下午8:17, Greg Kroah-Hartman 写道:
> On Thu, Jan 07, 2021 at 03:52:12PM +0800, Wen Yang wrote:
>> The dentries such as /proc/<pid>/ns/ have the DCACHE_OP_DELETE flag, they
>> should be deleted when the process exits.
>>
>> Suppose the following race appears：
>>
>> release_task dput
>> -> proc_flush_task
>> -> dentry->d_op->d_delete(dentry)
>> -> __exit_signal
>> -> dentry->d_lockref.count-- and return.
>>
>> In the proc_flush_task(), if another process is using this dentry, it will
>> not be deleted. At the same time, in dput(), d_op->d_delete() can be executed
>> before __exit_signal(pid has not been hashed), d_delete returns false, so
>> this dentry still cannot be deleted.
>>
>> This dentry will always be cached (although its count is 0 and the
>> DCACHE_OP_DELETE flag is set), its parent denry will also be cached too, and
>> these dentries can only be deleted when drop_caches is manually triggered.
>>
>> This will result in wasted memory. What's more troublesome is that these
>> dentries reference pid, according to the commit f333c700c610 ("pidns: Add a
>> limit on the number of pid namespaces"), if the pid cannot be released, it
>> may result in the inability to create a new pid_ns.
>>
>> This issue was introduced by 60347f6716aa ("pid namespaces: prepare
>> proc_flust_task() to flush entries from multiple proc trees"), exposed by
>> f333c700c610 ("pidns: Add a limit on the number of pid namespaces"), and then
>> fixed by 7bc3e6e55acf ("proc: Use a list of inodes to flush from proc").
>
> Why are you just submitting a series for 4.9 and 4.19, what about 4.14?
> We can't have users move to a newer kernel and then experience old bugs,
> right?
>
Okay, the patches corresponding to 4.14 will be ready later.
> But the larger question is why are you backporting a whole new feature
> here? Why is CLONE_PIDFD needed? That feels really wrong...
>
The reason for backporting CLONE_PIDFD is because 7bc3e6e55acf ("proc:
Use a list of inodes to flush from proc") relies on wait_pidfd.lock.
There are indeed many associated modifications here. We are also testing
it. Please check the code more.
Thanks.
--
Best wishes,
Wen
next prev parent reply other threads:[~2021-01-07 16:22 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-07 7:52 [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 01/10] clone: add CLONE_PIDFD Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 02/10] pidfd: add polling support Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 03/10] proc: Pass file mode to proc_pid_make_inode Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 04/10] proc: Better ownership of files for non-dumpable tasks in user namespaces Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 05/10] proc: use %u for pid printing and slightly less stack Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 06/10] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 07/10] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 08/10] proc: Clear the pieces of proc_inode that proc_evict_inode cares about Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 09/10] proc: Use d_invalidate in proc_prune_siblings_dcache Wen Yang
2021-01-07 7:52 ` [PATCH v2 4.9 10/10] proc: Use a list of inodes to flush from proc Wen Yang
2021-01-07 12:17 ` [PATCH v2 4.9 00/10] fix a race in release_task when flushing the dentry Greg Kroah-Hartman
2021-01-07 16:21 ` Wen Yang [this message]
2021-01-07 18:28 ` Greg Kroah-Hartman
2021-01-08 2:42 ` Wen Yang
2021-01-09 12:36 ` Greg Kroah-Hartman
-- strict thread matches above, loose matches on Subject: below --
2021-01-07 7:34 Wen Yang
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=e0fa1641-d00b-acfc-91d7-9eb16fb61664@linux.alibaba.com \
--to=wenyang@linux.alibaba.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sashal@kernel.org \
--cc=xlpang@linux.alibaba.com \
/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.