Re: Race between "mount" uevent and /proc/mounts?

[Kay Sievers <kay.sievers@vrfy.org>]

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/