linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
To: "Oliver Neukum" <oneukum@suse.de>
Cc: "Alan Stern" <stern@rowland.harvard.edu>,
	"Maneesh Soni" <maneesh@in.ibm.com>,
	gregkh@suse.de, linux-kernel@vger.kernel.org
Subject: Re: refcounting drivers' data structures used in sysfs buffers
Date: Mon, 12 Mar 2007 10:42:56 -0500	[thread overview]
Message-ID: <d120d5000703120842k4e134136v2fe56dcfc2264841@mail.gmail.com> (raw)
In-Reply-To: <200703121623.58607.oneukum@suse.de>

On 3/12/07, Oliver Neukum <oneukum@suse.de> wrote:
> Am Montag, 12. März 2007 15:57 schrieb Alan Stern:probably nece
> > On Mon, 12 Mar 2007, Oliver Neukum wrote:
> >
> > > > > Why? What's wrong with simply calling kref_get/put?
> > > >
> > > > It's the same old problem: the race between unbind and sysfs I/O.  What
> > > > good does holding a reference to the private data structure do if the
> > > > show/store method gets called after the driver has been unbound from the
> > > > device?  dev_get_drvdata() will no longer provide a valid pointer to the
> > > > private data, so the method will have no way to access it.  Hence the
> > > > method needs another argument.
> > >
> > > It does half the job. You can make sure the driver is not asked to access
> > > freed memory.
> > > It is true that a driver will have to mark that device "disconnected"
> > > and return errors if that device's attributes are referenced, but this can
> > > be done internally.
> >
> > No, you're missing the point.  Let's say driver A's disconnect() is
> > called, so the driver marks its private data structure as "disconnected"
> > and does dev_set_drvdata(NULL).  Then driver B is probed and bound to the
> > device, and it does its own dev_set_drvdata().  Then a user still holding
> > an open sysfs file reference for driver A calls a show() or store()
> > method.  The method will do dev_get_drvdata(), receiving the pointer to
> > driver B's private data.  Now you're in trouble, because A's method will
> > think it owns B's private data!
>
> Yes, I was missing the point. In consequence, drivers must not use
> dev_get_drvdata() to get their references to their private data. It's
> probably necessary to store it in struct sysfs_buffer and include that
> in the store/show callbacks.
> (The same does apply to interfaces of course)
>

Or drivers coudl verify that they still bound to the device they are
about to operate on (psmouse does this by taking a lock on device and
then checking if driver bound is the same address as psmouse). But I'd
rather get rid of all this clutter if we could sever sysfs access
after removing corresponding attributes.

-- 
Dmitry

  reply	other threads:[~2007-03-12 15:42 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-08 13:05 refcounting drivers' data structures used in sysfs buffers Oliver Neukum
2007-03-08 16:02 ` Alan Stern
2007-03-09  0:45   ` Oliver Neukum
2007-03-09 16:32     ` Alan Stern
2007-03-09 16:44       ` Oliver Neukum
2007-03-09 17:02         ` Dmitry Torokhov
2007-03-09 17:18           ` Oliver Neukum
2007-03-09 17:34             ` Dmitry Torokhov
2007-03-09 19:32               ` Alan Stern
2007-03-09 20:05                 ` Oliver Neukum
2007-03-09 20:27                   ` Alan Stern
2007-03-09 20:39                     ` Oliver Neukum
2007-03-09 20:08               ` Alan Stern
2007-03-09 20:48                 ` Oliver Neukum
2007-03-10 19:19                   ` Alan Stern
2007-03-12  8:54                     ` Oliver Neukum
2007-03-12 14:57                       ` Alan Stern
2007-03-12 15:23                         ` Oliver Neukum
2007-03-12 15:42                           ` Dmitry Torokhov [this message]
2007-03-12 15:59                             ` Oliver Neukum
2007-03-12 16:21                               ` Alan Stern
2007-03-12 18:25                                 ` Oliver Neukum
2007-03-12 19:31                                   ` Alan Stern
2007-03-12 19:49                                     ` Oliver Neukum
2007-03-12 20:03                                       ` Alan Stern
2007-03-12 20:15                                         ` Oliver Neukum
2007-03-12 20:31                                         ` Dmitry Torokhov
2007-03-12 20:45                                           ` Alan Stern
2007-03-12 21:31                                           ` Richard Purdie
2007-03-13 15:00                                             ` 2.6.21-rc suspend regression: sysfs deadlock Alan Stern
2007-03-13 18:42                                               ` Cornelia Huck
2007-03-13 21:20                                                 ` Linus Torvalds
2007-03-14 16:12                                                   ` Alan Stern
2007-03-14 18:43                                                     ` Cornelia Huck
2007-03-14 19:23                                                       ` Alan Stern
2007-03-15 10:27                                                         ` Cornelia Huck
2007-03-15 12:31                                                           ` Hugh Dickins
2007-03-15 13:02                                                             ` Oliver Neukum
2007-03-15 13:22                                                               ` Dmitry Torokhov
2007-03-15 13:59                                                                 ` Hugh Dickins
2007-03-15 14:27                                                           ` Alan Stern
2007-03-15 15:32                                                             ` Cornelia Huck
2007-03-15 16:29                                                             ` Hugh Dickins
2007-03-15 16:51                                                               ` Linus Torvalds
2007-03-15 19:50                                                                 ` [PATCH] sysfs and driver core: add callback helper, used by SCSI and S390 Alan Stern
2007-03-15 19:51                                                                 ` [PATCH] sysfs: reinstate exclusion between method calls and attribute unregistration Alan Stern
2007-03-13 19:00                                               ` 2.6.21-rc suspend regression: sysfs deadlock Hugh Dickins
2007-03-13 20:09                                                 ` Alan Stern
2007-03-13 20:55                                                   ` Hugh Dickins
2007-03-13 21:08                                                     ` Dmitry Torokhov
2007-03-13 21:20                                                     ` Alan Stern

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=d120d5000703120842k4e134136v2fe56dcfc2264841@mail.gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maneesh@in.ibm.com \
    --cc=oneukum@suse.de \
    --cc=stern@rowland.harvard.edu \
    /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).