ppp: ensure file->private_data can't be overridden
Locking ppp_mutex must be done before dereferencing file->private_data,
otherwise it could be modified before ppp_unattached_ioctl() takes the
lock. This could lead ppp_unattached_ioctl() to override ->private_data,
thus leaking reference to the ppp_file previously pointed to.
v2: lock all ppp_ioctl() instead of just checking private_data in
ppp_unattached_ioctl(), to avoid ambiguous behaviour.
Fixes: f3ff8a4d80
("ppp: push BKL down into the driver")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
c8f5d29899
commit
e8e56ffd9d
|
@ -575,7 +575,7 @@ static int get_filter(void __user *arg, struct sock_filter **p)
|
|||
|
||||
static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
||||
{
|
||||
struct ppp_file *pf = file->private_data;
|
||||
struct ppp_file *pf;
|
||||
struct ppp *ppp;
|
||||
int err = -EFAULT, val, val2, i;
|
||||
struct ppp_idle idle;
|
||||
|
@ -585,9 +585,14 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
void __user *argp = (void __user *)arg;
|
||||
int __user *p = argp;
|
||||
|
||||
if (!pf)
|
||||
return ppp_unattached_ioctl(current->nsproxy->net_ns,
|
||||
mutex_lock(&ppp_mutex);
|
||||
|
||||
pf = file->private_data;
|
||||
if (!pf) {
|
||||
err = ppp_unattached_ioctl(current->nsproxy->net_ns,
|
||||
pf, file, cmd, arg);
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (cmd == PPPIOCDETACH) {
|
||||
/*
|
||||
|
@ -602,7 +607,6 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
* this fd and reopening /dev/ppp.
|
||||
*/
|
||||
err = -EINVAL;
|
||||
mutex_lock(&ppp_mutex);
|
||||
if (pf->kind == INTERFACE) {
|
||||
ppp = PF_TO_PPP(pf);
|
||||
rtnl_lock();
|
||||
|
@ -616,15 +620,13 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
} else
|
||||
pr_warn("PPPIOCDETACH file->f_count=%ld\n",
|
||||
atomic_long_read(&file->f_count));
|
||||
mutex_unlock(&ppp_mutex);
|
||||
return err;
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (pf->kind == CHANNEL) {
|
||||
struct channel *pch;
|
||||
struct ppp_channel *chan;
|
||||
|
||||
mutex_lock(&ppp_mutex);
|
||||
pch = PF_TO_CHANNEL(pf);
|
||||
|
||||
switch (cmd) {
|
||||
|
@ -646,17 +648,16 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
err = chan->ops->ioctl(chan, cmd, arg);
|
||||
up_read(&pch->chan_sem);
|
||||
}
|
||||
mutex_unlock(&ppp_mutex);
|
||||
return err;
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (pf->kind != INTERFACE) {
|
||||
/* can't happen */
|
||||
pr_err("PPP: not interface or channel??\n");
|
||||
return -EINVAL;
|
||||
err = -EINVAL;
|
||||
goto out;
|
||||
}
|
||||
|
||||
mutex_lock(&ppp_mutex);
|
||||
ppp = PF_TO_PPP(pf);
|
||||
switch (cmd) {
|
||||
case PPPIOCSMRU:
|
||||
|
@ -831,7 +832,10 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
default:
|
||||
err = -ENOTTY;
|
||||
}
|
||||
|
||||
out:
|
||||
mutex_unlock(&ppp_mutex);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
|
@ -844,7 +848,6 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
|
|||
struct ppp_net *pn;
|
||||
int __user *p = (int __user *)arg;
|
||||
|
||||
mutex_lock(&ppp_mutex);
|
||||
switch (cmd) {
|
||||
case PPPIOCNEWUNIT:
|
||||
/* Create a new ppp unit */
|
||||
|
@ -894,7 +897,7 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
|
|||
default:
|
||||
err = -ENOTTY;
|
||||
}
|
||||
mutex_unlock(&ppp_mutex);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue