Prev: RFC: direct MTD support for SquashFS
Next: [GIT] HID
From: Jiri Slaby on 16 Mar 2010 10:00
Hi,

Stanse found a locking problem in the following function:

static long vhost_net_set_backend(struct vhost_net *n, unsigned index,
int fd)
{
struct socket *sock, *oldsock;
struct vhost_virtqueue *vq;
int r;

mutex_lock(&n->dev.mutex);
r = vhost_dev_check_owner(&n->dev);
if (r)
goto err;

if (index >= VHOST_NET_VQ_MAX) {
r = -ENOBUFS;
goto err;
}
vq = n->vqs + index;
mutex_lock(&vq->mutex); <--- locked

/* Verify that ring has been setup correctly. */
if (!vhost_vq_access_ok(vq)) {
r = -EFAULT;
goto err; <--- not unlocked
}
sock = get_socket(fd);
if (IS_ERR(sock)) {
r = PTR_ERR(sock);
goto err; <--- not unlocked
}

/* start polling new socket */
oldsock = vq->private_data;
if (sock == oldsock)
goto done; <--- not unlocked

vhost_net_disable_vq(n, vq);
rcu_assign_pointer(vq->private_data, sock);
vhost_net_enable_vq(n, vq);
mutex_unlock(&vq->mutex);
done:
if (oldsock) {
vhost_net_flush_vq(n, index);
fput(oldsock->file);
}
err:
mutex_unlock(&n->dev.mutex);
return r;
}


I don't see how the lock is unlocked on the error paths and as it is not
on none of the them maybe I'm missing something?

thanks,
--
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Slaby on 16 Mar 2010 10:10
On 03/16/2010 02:58 PM, Jiri Slaby wrote:
> I don't see how the lock is unlocked on the error paths and as it is not
> on none of the them maybe I'm missing something?

And there is one more issue in vhost_set_vring, several returns from
VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL, VHOST_SET_VRING_ERR with the
vq->mutex held.

--
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michael S. Tsirkin on 16 Mar 2010 11:00
On Tue, Mar 16, 2010 at 03:01:51PM +0100, Jiri Slaby wrote:
> On 03/16/2010 02:58 PM, Jiri Slaby wrote:
>> I don't see how the lock is unlocked on the error paths and as it is not
>> on none of the them maybe I'm missing something?
>
> And there is one more issue in vhost_set_vring, several returns from
> VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL, VHOST_SET_VRING_ERR with the
> vq->mutex held.

Thanks!
I have patches for these that I'll push out shortly.

> --
> js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Dike on 16 Mar 2010 12:20
On Tue, Mar 16, 2010 at 02:58:23PM +0100, Jiri Slaby wrote:
> I don't see how the lock is unlocked on the error paths and as it is not
> on none of the them maybe I'm missing something?

I sent Michael a patch for this a couple of weeks ago.

Jeff

--
Work email - jdike at linux dot intel dot com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: RFC: direct MTD support for SquashFS
Next: [GIT] HID