Commit edffc217 authored by Guillaume Nault's avatar Guillaume Nault Committed by David S. Miller

ppp: lock ppp->flags in ppp_read() and ppp_poll()

ppp_read() and ppp_poll() can be called concurrently with ppp_ioctl().
In this case, ppp_ioctl() might call ppp_ccp_closed(), which may update
ppp->flags while ppp_read() or ppp_poll() is reading it.
The update done by ppp_ccp_closed() isn't atomic due to the bit mask
operation ('ppp->flags &= ~(SC_CCP_OPEN | SC_CCP_UP)'), so concurrent
readers might get transient values.
Reading incorrect ppp->flags may disturb the 'ppp->flags & SC_LOOP_TRAFFIC'
test in ppp_read() and ppp_poll(), which in turn can lead to improper
decision on whether the PPP unit file is ready for reading or not.

Since ppp_ccp_closed() is protected by the Rx and Tx locks (with
ppp_lock()), taking the Rx lock is enough for ppp_read() and ppp_poll()
to guarantee that ppp_ccp_closed() won't update ppp->flags
concurrently.

The same reasoning applies to ppp->n_channels. The 'n_channels' field
can also be written to concurrently by ppp_ioctl() (through
ppp_connect_channel() or ppp_disconnect_channel()). These writes aren't
atomic (simple increment/decrement), but are protected by both the Rx
and Tx locks (like in the ppp->flags case). So holding the Rx lock
before reading ppp->n_channels also prevents concurrent writes.
Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent a7f4b988
...@@ -443,10 +443,15 @@ static ssize_t ppp_read(struct file *file, char __user *buf, ...@@ -443,10 +443,15 @@ static ssize_t ppp_read(struct file *file, char __user *buf,
* network traffic (demand mode). * network traffic (demand mode).
*/ */
struct ppp *ppp = PF_TO_PPP(pf); struct ppp *ppp = PF_TO_PPP(pf);
ppp_recv_lock(ppp);
if (ppp->n_channels == 0 && if (ppp->n_channels == 0 &&
(ppp->flags & SC_LOOP_TRAFFIC) == 0) (ppp->flags & SC_LOOP_TRAFFIC) == 0) {
ppp_recv_unlock(ppp);
break; break;
} }
ppp_recv_unlock(ppp);
}
ret = -EAGAIN; ret = -EAGAIN;
if (file->f_flags & O_NONBLOCK) if (file->f_flags & O_NONBLOCK)
break; break;
...@@ -532,9 +537,12 @@ static unsigned int ppp_poll(struct file *file, poll_table *wait) ...@@ -532,9 +537,12 @@ static unsigned int ppp_poll(struct file *file, poll_table *wait)
else if (pf->kind == INTERFACE) { else if (pf->kind == INTERFACE) {
/* see comment in ppp_read */ /* see comment in ppp_read */
struct ppp *ppp = PF_TO_PPP(pf); struct ppp *ppp = PF_TO_PPP(pf);
ppp_recv_lock(ppp);
if (ppp->n_channels == 0 && if (ppp->n_channels == 0 &&
(ppp->flags & SC_LOOP_TRAFFIC) == 0) (ppp->flags & SC_LOOP_TRAFFIC) == 0)
mask |= POLLIN | POLLRDNORM; mask |= POLLIN | POLLRDNORM;
ppp_recv_unlock(ppp);
} }
return mask; return mask;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment