From: Kay Sievers <kay.sievers@vrfy.org>
To: Sergey Vlasov <vsu@altlinux.ru>
Cc: Al Viro <viro@ftp.linux.org.uk>,
Roderich.Schupp.extern@mch.siemens.de,
linux-kernel@vger.kernel.org,
linux-hotplug-devel@lists.sourceforge.net,
Greg KH <greg@kroah.com>
Subject: Re: Race between "mount" uevent and /proc/mounts?
Date: Tue, 01 Nov 2005 21:35:25 +0000 [thread overview]
Message-ID: <20051101213525.GA17207@vrfy.org> (raw)
In-Reply-To: <20051101195449.GA9162@procyon.home>
On Tue, Nov 01, 2005 at 10:54:49PM +0300, Sergey Vlasov wrote:
> On Tue, Nov 01, 2005 at 04:58:16AM +0100, Kay Sievers wrote:
> > On Tue, Nov 01, 2005 at 01:28:46AM +0100, Kay Sievers wrote:
> > > Ok, makes sense. The attached seems to work for me. If we can get
> > > something like this, we can remove the superblock claim/release events
> > > completely and just read the events from the /proc/mounts file itself.
>
> No, we need both events. When you need to tell the user when it is
> safe to disconnect the storage device, the event from detach_mnt() is
> useless - it happens too early. In fact, even the current way of
> sending the event from kill_block_super() is broken, because the event
> is generated before generic_shutdown_super() and sync_blockdev(), and
> writing out cached data may take some time.
>
> We could try to emit busy/free events from bd_claim() and
> bd_release(); this would be triggered by most "interesting" users
> (even opens with O_EXCL), but not by things like volume_id.
Hmm, HAL polls optical drives every 2 seconds with O_EXCL to detect media
changes. You need to do it EXCL, cause otherwise some cd burners fail.
> > New patch. Missed a check for namespace = NULL in detach_mnt().
> [skip]
> > +static unsigned int mounts_poll(struct file *file, poll_table *wait)
> > +{
> > + struct task_struct *task = proc_task(file->f_dentry->d_inode);
> > + struct namespace *namespace;
> > + int ret = 0;
> > +
> > + task_lock(task);
> > + namespace = task->namespace;
> > + if (namespace)
> > + get_namespace(namespace);
> > + task_unlock(task);
> > +
> > + if (!namespace)
> > + return -EINVAL;
> > +
> > + poll_wait(file, &mounts_wait, wait);
> > + if (namespace->mounts_poll_pending) {
> > + namespace->mounts_poll_pending = 0;
> > + ret = POLLIN | POLLRDNORM;
> > + }
>
> This assumes that there will be only one process per namespace which
> will call poll() on /proc/mounts. Even though someone may argue that
> it is the right approach (have a single process which watches
> /proc/mounts and broadcasts updates to other interested processes,
> e.g., over dbus), with the above implementation any unprivileged user
> can call poll() and interfere with the operation of that designated
> process.
Sure, capable(CAP_SYS_ADMIN) could prevent this.
Thanks,
Kay
-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
WARNING: multiple messages have this Message-ID (diff)
From: Kay Sievers <kay.sievers@vrfy.org>
To: Sergey Vlasov <vsu@altlinux.ru>
Cc: Al Viro <viro@ftp.linux.org.uk>,
Roderich.Schupp.extern@mch.siemens.de,
linux-kernel@vger.kernel.org,
linux-hotplug-devel@lists.sourceforge.net,
Greg KH <greg@kroah.com>
Subject: Re: Race between "mount" uevent and /proc/mounts?
Date: Tue, 1 Nov 2005 22:35:25 +0100 [thread overview]
Message-ID: <20051101213525.GA17207@vrfy.org> (raw)
In-Reply-To: <20051101195449.GA9162@procyon.home>
On Tue, Nov 01, 2005 at 10:54:49PM +0300, Sergey Vlasov wrote:
> On Tue, Nov 01, 2005 at 04:58:16AM +0100, Kay Sievers wrote:
> > On Tue, Nov 01, 2005 at 01:28:46AM +0100, Kay Sievers wrote:
> > > Ok, makes sense. The attached seems to work for me. If we can get
> > > something like this, we can remove the superblock claim/release events
> > > completely and just read the events from the /proc/mounts file itself.
>
> No, we need both events. When you need to tell the user when it is
> safe to disconnect the storage device, the event from detach_mnt() is
> useless - it happens too early. In fact, even the current way of
> sending the event from kill_block_super() is broken, because the event
> is generated before generic_shutdown_super() and sync_blockdev(), and
> writing out cached data may take some time.
>
> We could try to emit busy/free events from bd_claim() and
> bd_release(); this would be triggered by most "interesting" users
> (even opens with O_EXCL), but not by things like volume_id.
Hmm, HAL polls optical drives every 2 seconds with O_EXCL to detect media
changes. You need to do it EXCL, cause otherwise some cd burners fail.
> > New patch. Missed a check for namespace == NULL in detach_mnt().
> [skip]
> > +static unsigned int mounts_poll(struct file *file, poll_table *wait)
> > +{
> > + struct task_struct *task = proc_task(file->f_dentry->d_inode);
> > + struct namespace *namespace;
> > + int ret = 0;
> > +
> > + task_lock(task);
> > + namespace = task->namespace;
> > + if (namespace)
> > + get_namespace(namespace);
> > + task_unlock(task);
> > +
> > + if (!namespace)
> > + return -EINVAL;
> > +
> > + poll_wait(file, &mounts_wait, wait);
> > + if (namespace->mounts_poll_pending) {
> > + namespace->mounts_poll_pending = 0;
> > + ret = POLLIN | POLLRDNORM;
> > + }
>
> This assumes that there will be only one process per namespace which
> will call poll() on /proc/mounts. Even though someone may argue that
> it is the right approach (have a single process which watches
> /proc/mounts and broadcasts updates to other interested processes,
> e.g., over dbus), with the above implementation any unprivileged user
> can call poll() and interfere with the operation of that designated
> process.
Sure, capable(CAP_SYS_ADMIN) could prevent this.
Thanks,
Kay
next prev parent reply other threads:[~2005-11-01 21:35 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-10-25 13:20 Race between "mount" uevent and /proc/mounts? Schupp Roderich (extern) BenQ MD PD SWP 2 CM MCH
2005-10-25 14:00 ` Al Viro
2005-10-26 10:27 ` Sergey Vlasov
2005-10-26 10:27 ` Sergey Vlasov
2005-10-26 11:15 ` Al Viro
2005-10-26 11:15 ` Al Viro
2005-10-26 14:34 ` Kay Sievers
2005-10-26 14:34 ` Kay Sievers
2005-10-26 14:45 ` Xavier Bestel
2005-10-26 14:45 ` Xavier Bestel
2005-10-26 19:28 ` Al Viro
2005-10-26 19:28 ` Al Viro
2005-11-01 0:28 ` Kay Sievers
2005-11-01 0:28 ` Kay Sievers
2005-11-01 3:58 ` Kay Sievers
2005-11-01 3:58 ` Kay Sievers
2005-11-01 19:54 ` Sergey Vlasov
2005-11-01 19:54 ` Sergey Vlasov
2005-11-01 21:35 ` Kay Sievers [this message]
2005-11-01 21:35 ` Kay Sievers
2005-11-02 13:01 ` Sergey Vlasov
2005-11-02 13:01 ` Sergey Vlasov
2005-11-03 8:07 ` Al Viro
2005-11-03 8:07 ` Al Viro
2005-11-03 10:52 ` Sergey Vlasov
2005-11-03 10:52 ` Sergey Vlasov
2005-11-03 11:30 ` Al Viro
2005-11-03 11:30 ` Al Viro
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=20051101213525.GA17207@vrfy.org \
--to=kay.sievers@vrfy.org \
--cc=Roderich.Schupp.extern@mch.siemens.de \
--cc=greg@kroah.com \
--cc=linux-hotplug-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@ftp.linux.org.uk \
--cc=vsu@altlinux.ru \
/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.