Skip to content

Commit

Permalink
tun: Fix sk_sleep races when attaching/detaching
Browse files Browse the repository at this point in the history
As the sk_sleep wait queue actually lives in tfile, which may be
detached from the tun device, bad things will happen when we use
sk_sleep after detaching.

Since the tun device is the persistent data structure here (when
requested by the user), it makes much more sense to have the wait
queue live there.  There is no reason to have it in tfile at all
since the only time we can wait is if we have a tun attached.
In fact we already have a wait queue in tun_struct, so we might
as well use it.

Reported-by: Eric W. Biederman <ebiederm@xmission.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Herbert Xu authored and David S. Miller committed Apr 20, 2009
1 parent 9c3fea6 commit c40af84
Showing 1 changed file with 6 additions and 9 deletions.
15 changes: 6 additions & 9 deletions drivers/net/tun.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ struct tun_file {
atomic_t count;
struct tun_struct *tun;
struct net *net;
wait_queue_head_t read_wait;
};

struct tun_sock;
Expand Down Expand Up @@ -331,7 +330,7 @@ static void tun_net_uninit(struct net_device *dev)
/* Inform the methods they need to stop using the dev.
*/
if (tfile) {
wake_up_all(&tfile->read_wait);
wake_up_all(&tun->socket.wait);
if (atomic_dec_and_test(&tfile->count))
__tun_detach(tun);
}
Expand Down Expand Up @@ -398,7 +397,7 @@ static int tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
/* Notify and wake up reader process */
if (tun->flags & TUN_FASYNC)
kill_fasync(&tun->fasync, SIGIO, POLL_IN);
wake_up_interruptible(&tun->tfile->read_wait);
wake_up_interruptible(&tun->socket.wait);
return 0;

drop:
Expand Down Expand Up @@ -495,7 +494,7 @@ static unsigned int tun_chr_poll(struct file *file, poll_table * wait)

DBG(KERN_INFO "%s: tun_chr_poll\n", tun->dev->name);

poll_wait(file, &tfile->read_wait, wait);
poll_wait(file, &tun->socket.wait, wait);

if (!skb_queue_empty(&tun->readq))
mask |= POLLIN | POLLRDNORM;
Expand Down Expand Up @@ -768,7 +767,7 @@ static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv,
goto out;
}

add_wait_queue(&tfile->read_wait, &wait);
add_wait_queue(&tun->socket.wait, &wait);
while (len) {
current->state = TASK_INTERRUPTIBLE;

Expand Down Expand Up @@ -799,7 +798,7 @@ static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv,
}

current->state = TASK_RUNNING;
remove_wait_queue(&tfile->read_wait, &wait);
remove_wait_queue(&tun->socket.wait, &wait);

out:
tun_put(tun);
Expand Down Expand Up @@ -867,7 +866,6 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
struct sock *sk;
struct tun_struct *tun;
struct net_device *dev;
struct tun_file *tfile = file->private_data;
int err;

dev = __dev_get_by_name(net, ifr->ifr_name);
Expand Down Expand Up @@ -925,10 +923,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
if (!sk)
goto err_free_dev;

init_waitqueue_head(&tun->socket.wait);
sock_init_data(&tun->socket, sk);
sk->sk_write_space = tun_sock_write_space;
sk->sk_sndbuf = INT_MAX;
sk->sk_sleep = &tfile->read_wait;

tun->sk = sk;
container_of(sk, struct tun_sock, sk)->tun = tun;
Expand Down Expand Up @@ -1270,7 +1268,6 @@ static int tun_chr_open(struct inode *inode, struct file * file)
atomic_set(&tfile->count, 0);
tfile->tun = NULL;
tfile->net = get_net(current->nsproxy->net_ns);
init_waitqueue_head(&tfile->read_wait);
file->private_data = tfile;
return 0;
}
Expand Down

0 comments on commit c40af84

Please sign in to comment.