From: Tejun Heo <tj@kernel.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: gregkh@linuxfoundation.org, akpm@linux-foundation.org,
minchan@kernel.org, jeyu@kernel.org, shuah@kernel.org,
rdunlap@infradead.org, rafael@kernel.org, masahiroy@kernel.org,
ndesaulniers@google.com, yzaikin@google.com, nathan@kernel.org,
ojeda@kernel.org, penguin-kernel@i-love.sakura.ne.jp,
vitor@massaru.org, elver@google.com, jarkko@kernel.org,
glider@google.com, rf@opensource.cirrus.com,
stephen@networkplumber.org, David.Laight@aculab.com,
bvanassche@acm.org, jolsa@kernel.org,
andriy.shevchenko@linux.intel.com, trishalfonso@google.com,
andreyknvl@gmail.com, jikos@kernel.org, mbenes@suse.com,
ngupta@vflare.org, sergey.senozhatsky.work@gmail.com,
reinette.chatre@intel.com, fenghua.yu@intel.com, bp@alien8.de,
x86@kernel.org, hpa@zytor.com, lizefan.x@bytedance.com,
hannes@cmpxchg.org, daniel.vetter@ffwll.ch, bhelgaas@google.com,
kw@linux.com, dan.j.williams@intel.com, senozhatsky@chromium.org,
hch@lst.de, joe@perches.com, hkallweit1@gmail.com,
axboe@kernel.dk, jpoimboe@redhat.com, tglx@linutronix.de,
keescook@chromium.org, rostedt@goodmis.org, peterz@infradead.org,
linux-spdx@vger.kernel.org, linux-doc@vger.kernel.org,
linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kselftest@vger.kernel.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org,
copyleft-next@lists.fedorahosted.org
Subject: Re: [PATCH v7 09/12] sysfs: fix deadlock race with module removal
Date: Mon, 20 Sep 2021 07:53:16 -1000 [thread overview]
Message-ID: <YUjKjLzqpcxjRyit@slm.duckdns.org> (raw)
In-Reply-To: <20210918050430.3671227-10-mcgrof@kernel.org>
Hello,
On Fri, Sep 17, 2021 at 10:04:27PM -0700, Luis Chamberlain wrote:
> If try_module_get() fails we fail the operation on the kernfs node.
>
> We use a try method as a full lock means we'd then make our sysfs
> attributes busy us out from possible module removal, and so userspace
> could force denying module removal, a silly form of "DOS" against module
> removal. A try lock on the module removal ensures we give priority to
> module removal and interacting with sysfs attributes only comes second.
> Using a full lock could mean for instance that if you don't stop poking
> at sysfs files you cannot remove a module.
I find this explanation odd because there's no real equivalent to locking
the module (as opposed to try locking) because you can't wait for the
removal to finish and then grant the lock, so any operation which increases
the reference *has* to be a try method unless the caller already holds a
reference to the same module and thus knows that the module is already
pinned. The code isn't wrong, so maybe just drop the related paragraphs in
the commit message?
> static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> struct kernfs_node *parent,
> const char *name, umode_t mode,
> + struct module *owner,
> kuid_t uid, kgid_t gid,
> unsigned flags)
Is there a particular reason why @owner is added between @mode and @uid?
Sitting between two fs attributes seems a bit awkward. Maybe it can just be
the last one?
Thanks.
--
tejun
next prev parent reply other threads:[~2021-09-21 0:05 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210918050430.3671227-1-mcgrof@kernel.org>
2021-09-18 5:04 ` [PATCH v7 01/12] LICENSES: Add the copyleft-next-0.3.1 license Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 02/12] testing: use the copyleft-next-0.3.1 SPDX tag Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 03/12] selftests: add tests_sysfs module Luis Chamberlain
2021-09-20 21:38 ` Bart Van Assche
2021-09-18 5:04 ` [PATCH v7 04/12] kernfs: add initial failure injection support Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 05/12] test_sysfs: add support to use kernfs failure injection Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 06/12] kernel/module: add documentation for try_module_get() Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 07/12] fs/kernfs/symlink.c: replace S_IRWXUGO with 0777 on kernfs_create_link() Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 08/12] fs/sysfs/dir.c: replace S_IRWXU|S_IRUGO|S_IXUGO with 0755 sysfs_create_dir_ns() Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 09/12] sysfs: fix deadlock race with module removal Luis Chamberlain
2021-09-20 17:53 ` Tejun Heo [this message]
2021-09-20 19:15 ` Luis Chamberlain
2021-09-20 19:22 ` Tejun Heo
2021-09-20 19:38 ` Luis Chamberlain
2021-09-20 20:52 ` Dan Williams
2021-09-20 21:17 ` Luis Chamberlain
2021-09-20 21:55 ` Dan Williams
2021-09-21 0:03 ` Luis Chamberlain
2021-09-20 21:36 ` Bart Van Assche
2021-09-20 21:43 ` Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 10/12] test_sysfs: enable deadlock tests by default Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 11/12] zram: fix crashes with cpu hotplug multistate Luis Chamberlain
2021-09-18 5:04 ` [PATCH v7 12/12] zram: use ATTRIBUTE_GROUPS to fix sysfs deadlock module removal Luis Chamberlain
2021-09-20 17:55 ` [PATCH v7 00/12 (RESEND)] syfs: generic deadlock fix with " Tejun Heo
2021-09-27 15:11 ` Luis Chamberlain
[not found] <20210917194709.3562413-1-mcgrof@kernel.org>
[not found] ` <20210917194709.3562413-10-mcgrof@kernel.org>
[not found] ` <c70dcb03e27e43c5b5311e184357df39@AcuMS.aculab.com>
2021-09-21 15:48 ` [PATCH v7 09/12] sysfs: fix deadlock race " Luis Chamberlain
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=YUjKjLzqpcxjRyit@slm.duckdns.org \
--to=tj@kernel.org \
--cc=David.Laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=axboe@kernel.dk \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=bvanassche@acm.org \
--cc=cgroups@vger.kernel.org \
--cc=copyleft-next@lists.fedorahosted.org \
--cc=dan.j.williams@intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=elver@google.com \
--cc=fenghua.yu@intel.com \
--cc=glider@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=hannes@cmpxchg.org \
--cc=hch@lst.de \
--cc=hkallweit1@gmail.com \
--cc=hpa@zytor.com \
--cc=jarkko@kernel.org \
--cc=jeyu@kernel.org \
--cc=jikos@kernel.org \
--cc=joe@perches.com \
--cc=jolsa@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=keescook@chromium.org \
--cc=kw@linux.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-spdx@vger.kernel.org \
--cc=lizefan.x@bytedance.com \
--cc=masahiroy@kernel.org \
--cc=mbenes@suse.com \
--cc=mcgrof@kernel.org \
--cc=minchan@kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=ngupta@vflare.org \
--cc=ojeda@kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=peterz@infradead.org \
--cc=rafael@kernel.org \
--cc=rdunlap@infradead.org \
--cc=reinette.chatre@intel.com \
--cc=rf@opensource.cirrus.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=shuah@kernel.org \
--cc=stephen@networkplumber.org \
--cc=tglx@linutronix.de \
--cc=trishalfonso@google.com \
--cc=vitor@massaru.org \
--cc=x86@kernel.org \
--cc=yzaikin@google.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 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).