Skip to content

Commit

Permalink
pps: fix race in PPS_FETCH handler
Browse files Browse the repository at this point in the history
There was a race in PPS_FETCH ioctl handler when several processes want to
obtain PPS data simultaneously using sleeping PPS_FETCH.  They all sleep
most of the time in the system call.

With the old approach when the first process waiting on the pps queue is
waken up it makes new system call right away and zeroes pps->go.  So other
processes continue to sleep.  This is a clear race condition because of
the global 'go' variable.

With the new approach pps->last_ev holds some value increasing at each PPS
event.  PPS_FETCH ioctl handler saves current value to the local variable
at the very beginning so it can safely check that there is a new event by
just comparing both variables.

Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
ago authored and torvalds committed Jan 13, 2011
1 parent 86d921f commit 3003d55
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 6 deletions.
4 changes: 2 additions & 2 deletions drivers/pps/kapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)

/* Wake up if captured something */
if (captured) {
pps->go = ~0;
wake_up_interruptible(&pps->queue);
pps->last_ev++;
wake_up_interruptible_all(&pps->queue);

kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
}
Expand Down
10 changes: 7 additions & 3 deletions drivers/pps/pps.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,20 @@ static long pps_cdev_ioctl(struct file *file,

case PPS_FETCH: {
struct pps_fdata fdata;
unsigned int ev;

pr_debug("PPS_FETCH: source %d\n", pps->id);

err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
if (err)
return -EFAULT;

pps->go = 0;
ev = pps->last_ev;

/* Manage the timeout */
if (fdata.timeout.flags & PPS_TIME_INVALID)
err = wait_event_interruptible(pps->queue, pps->go);
err = wait_event_interruptible(pps->queue,
ev != pps->last_ev);
else {
unsigned long ticks;

Expand All @@ -159,7 +161,9 @@ static long pps_cdev_ioctl(struct file *file,

if (ticks != 0) {
err = wait_event_interruptible_timeout(
pps->queue, pps->go, ticks);
pps->queue,
ev != pps->last_ev,
ticks);
if (err == 0)
return -ETIMEDOUT;
}
Expand Down
2 changes: 1 addition & 1 deletion include/linux/pps_kernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct pps_device {
struct pps_ktime clear_tu;
int current_mode; /* PPS mode at event time */

int go; /* PPS event is arrived? */
unsigned int last_ev; /* last PPS event id */
wait_queue_head_t queue; /* PPS event queue */

unsigned int id; /* PPS source unique ID */
Expand Down

0 comments on commit 3003d55

Please sign in to comment.