linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Viro <viro@math.psu.edu>
To: Richard Gooch <rgooch@ras.ucalgary.ca>
Cc: Linus Torvalds <torvalds@transmeta.com>, linux-kernel@vger.kernel.org
Subject: Re: [RFC] killing DEVFS_FL_AUTO_OWNER
Date: Sun, 6 Oct 2002 16:07:45 -0400 (EDT)	[thread overview]
Message-ID: <Pine.GSO.4.21.0210061550100.25699-100000@weyl.math.psu.edu> (raw)
In-Reply-To: <200210061932.g96JW3527255@vindaloo.ras.ucalgary.ca>



On Sun, 6 Oct 2002, Richard Gooch wrote:

> Well, I can't comment on the video1394 driver. I don't really know why
> they are using DEVFS_FL_AUTO_OWNER. If their device node is safe to
> have rw-rw-rw- (like with PTY slaves), then it's not a problem.
> However, if the driver allows you to do Bad Things[tm] if you can read
> or write to the device node, then the driver is buggy, and is abusing
> DEVFS_FL_AUTO_OWNER.
> 
> So we should get input from the driver maintainer as to what the
> intent is.

1) current implementation does _not_ reset uid/gid after the first open().
It either stays as it was (until the d_delete) or gets reset to root.root.666
(after d_delete) and stays that way.

Notice that your code that sets it is in the very end of devfs_open() - in
the part that is run once.  df->open is never reset, so...

Oh, BTW - you have

	if (df->open) return 0;
	df->open = TRUE;

with no locking whatsoever - you'd reduced BKL-covered area so that it doesn't
cover that place.  With obvious consequences...

2) either applications do care to do chmod/chown (and in that case
DEVFS_FL_AUTO_OWNER is simply irrelevant), or they are
	* broken on non-devfs systems
	* broken on devfs systems due to (1)

Having world-readable video camera is an obvious security problem, so
applications _must_ deal with that, for non-devfs systems if nothing
else.

3) this crap is the only thing that still uses DEVFS_FL_AUTO_OWNER.


IOW, we should remove it and send heads-up to driver authors.  End of
story.

Another thing: when you do devfs_register(), the thing creates intermediate
directories.  However, devfs_unregister() on the same node doesn't undo
the effect of devfs_register() - directories stay around, even if nothing
else holds them.

Proposal: add a counter to devfs entries of directories so that
	* result of devfs_mk_dir would have it set to 1
	* creation of child in a directory would increment it by 1
	* removal of child would decrement it by 1
	* when counter drops to 0 (which means that directory had
been created implictly by devfs_register() and all children are gone)
we unregister directory.  That, in turn, can cause unregistering its
parent, etc.

That would cut down the amount of work done in drivers (esp. block device
drivers) and allow to simplify the ad-hackery in partitions/check.c.

It's trivial to implement, so if you have objections to that - tell it now.


  reply	other threads:[~2002-10-06 20:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-10-06 18:55 [RFC] killing DEVFS_FL_AUTO_OWNER Alexander Viro
2002-10-06 19:32 ` Richard Gooch
2002-10-06 20:07   ` Alexander Viro [this message]
2002-10-13  0:17     ` Richard Gooch

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=Pine.GSO.4.21.0210061550100.25699-100000@weyl.math.psu.edu \
    --to=viro@math.psu.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rgooch@ras.ucalgary.ca \
    --cc=torvalds@transmeta.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).