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  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 <>
To:	Jonathan Corbet <>
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
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists