linux-kernel - Re: [PATCH 10/10] send events using read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 03 Nov 2009 19:55:48 -0500
From:	Eric Paris <eparis@...hat.com>
To:	Jonathan Corbet <corbet@....net>
Cc:	linux-kernel@...r.kernel.org, linus-fsdevel@...r.kernel.org,
	viro@...iv.linux.org.uk, hch@...radead.org, agruen@...e.de,
	vegard.nossum@...il.com
Subject: Re: [PATCH 10/10] send events using read

On Tue, 2009-11-03 at 16:59 -0700, Jonathan Corbet wrote:
> Hi, Eric,
> 
> This is not a full review, but I did notice a problem as I was trying to
> figure out the new API...

This is a rip off of inotify which was introduced in commit 3632dee2f8b
from Vegard Nossum back in January.  I can't seem to find any discussion
of this before it went into Linus' tree, so if someone knows how this
patch got to Linus and what was said about it, I'd like to know.  Thanks
for unwittingly finding and inotify bug!

I looked back over it and it looks to me like it will work although
there may be a race like situation if there are multiple things trying
to read events.  I can see how with perfect timing and precision Task A
might try to get the mutex and it is being held by task B.  Task B drops
the mutex and Task A gets it, this causes Task A to be TASK_RUNNABLE.
Lets assume Task B runs back around the loop and tries to get it while
Task A still holds it.  Task A will drop the mutex and Task B gets it,
now B is runnable.  Repeat until infinity with perfect timing!  It's not
really a live-lock, if either of them ever gets the mutex uncontested or
if an event ever arrives they are going to sleep and/or break the cycle.

Certainly I'll take a look at it.

> > +static ssize_t fanotify_read(struct file *file, char __user *buf,
> > +			     size_t count, loff_t *pos)
> > +{
> > +	struct fsnotify_group *group;
> > +	struct fsnotify_event *kevent;
> > +	char __user *start;
> > +	int ret;
> > +	DEFINE_WAIT(wait);
> > +
> > +	start = buf;
> > +	group = file->private_data;
> > +
> > +	pr_debug("%s: group=%p\n", __func__, group);
> > +
> > +	while (1) {
> > +		prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
> > +
> > +		mutex_lock(&group->notification_mutex);
> > +		kevent = get_one_event(group, count);
> > +		mutex_unlock(&group->notification_mutex);
> 
> prepare_to_wait(), among other things, sets the task state.  But then you
> go into various sleeping calls (mutex_lock(), for starters); that will undo
> what prepare_to_wait has done.  You'll be back in TASK_RUNNING at this
> point, so you'll never sleep.

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