archive mirror
 help / color / mirror / Atom feed
From: Greg KH <>
To: Luis Chamberlain <>
Subject: Re: [PATCH v4] sysfs: fix kobject refcount to address races with kobject removal
Date: Thu, 24 Jun 2021 13:09:03 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Wed, Jun 23, 2021 at 02:50:07PM -0700, Luis Chamberlain wrote:
> It's possible today to have a device attribute read or store
> race against device removal. This is known to happen as follows:
> write system call -->
>   ksys_write () -->
>     vfs_write() -->
>       __vfs_write() -->
>         kernfs_fop_write_iter() -->
>           sysfs_kf_write() -->
>             dev_attr_store() -->
>               null reference
> This happens because the dev_attr->store() callback can be
> removed prior to its call, after dev_attr_store() was initiated.
> The null dereference is possible because the sysfs ops can be
> removed on module removal, for instance, when device_del() is
> called, and a sysfs read / store is not doing any kobject reference
> bumps either. This allows a read/store call to initiate, a
> device_del() to kick off, and then the read/store call can be
> gone by the time to execute it.
> The sysfs filesystem is not doing any kobject reference bumps during a
> read / store ops to prevent this.
> To fix this in a simplified way, just bump the kobject reference when
> we create a directory and remove it on directory removal.
> The big unfortunate eye-sore is addressing the manual kobject reference
> assumption on the networking code, which leads me to believe we should
> end up replacing that eventually with another sort of check.
> Suggested-by: Greg Kroah-Hartman <>
> Signed-off-by: Luis Chamberlain <>
> ---
> This v4 moves to fixing the race condition on dev_attr_store() and
> dev_attr_read() to sysfs by bumping the kobject reference count
> on directory creation / deletion as suggested by Greg.

This looks good.

It's late in the development cycle, I'll hold off on adding this to my
tree until 5.14-rc1 is out because of:

> Unfortunately at least the networking core has a manual refcount
> assumption, which needs to be adjusted to account for this change.
> This should also mean there is runtime for other kobjects which may
> not be explored yet which may need fixing as well. We may want to
> change the check to something else on the networking front, but its
> not clear to me yet what to use.

That's crazy what networking is doing here, hopefully no one else is.
If they are, let's shake it out in linux-next to find the problems which
is why a good "soak" there is a good idea.

thanks for making this change and sticking with it!

Oh, and with this change, does your modprobe/rmmod crazy test now work?

greg k-h

  parent reply	other threads:[~2021-06-24 11:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23 21:50 [PATCH v4] sysfs: fix kobject refcount to address races with kobject removal Luis Chamberlain
2021-06-23 22:59 ` Kees Cook
2021-06-24  1:09   ` Luis Chamberlain
2021-06-24 11:06   ` Greg KH
2021-06-24 11:09 ` Greg KH [this message]
2021-06-25 21:55   ` Luis Chamberlain
2021-07-01 22:48     ` Luis Chamberlain
2021-07-02  1:04       ` Luis Chamberlain
2021-07-21 11:30       ` Greg KH
2021-07-22 21:31         ` Luis Chamberlain
2021-07-23 11:14           ` Greg KH
2021-07-23 17:35             ` 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:

* 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).