Skip to content

Commit dd54bcf

Browse files
Nikolay Kuratovmstsirkin
authored andcommitted
vhost/net: Protect ubufs with rcu read lock in vhost_net_ubuf_put()
When operating on struct vhost_net_ubuf_ref, the following execution sequence is theoretically possible: CPU0 is finalizing DMA operation CPU1 is doing VHOST_NET_SET_BACKEND // ubufs->refcount == 2 vhost_net_ubuf_put() vhost_net_ubuf_put_wait_and_free(oldubufs) vhost_net_ubuf_put_and_wait() vhost_net_ubuf_put() int r = atomic_sub_return(1, &ubufs->refcount); // r = 1 int r = atomic_sub_return(1, &ubufs->refcount); // r = 0 wait_event(ubufs->wait, !atomic_read(&ubufs->refcount)); // no wait occurs here because condition is already true kfree(ubufs); if (unlikely(!r)) wake_up(&ubufs->wait); // use-after-free This leads to use-after-free on ubufs access. This happens because CPU1 skips waiting for wake_up() when refcount is already zero. To prevent that use a read-side RCU critical section in vhost_net_ubuf_put(), as suggested by Hillf Danton. For this lock to take effect, free ubufs with kfree_rcu(). Cc: [email protected] Fixes: 0ad8b48 ("vhost: fix ref cnt checking deadlock") Reported-by: Andrey Ryabinin <[email protected]> Suggested-by: Hillf Danton <[email protected]> Signed-off-by: Nikolay Kuratov <[email protected]> Message-Id: <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]>
1 parent a39d13e commit dd54bcf

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

drivers/vhost/net.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ struct vhost_net_ubuf_ref {
9999
atomic_t refcount;
100100
wait_queue_head_t wait;
101101
struct vhost_virtqueue *vq;
102+
struct rcu_head rcu;
102103
};
103104

104105
#define VHOST_NET_BATCH 64
@@ -250,9 +251,13 @@ vhost_net_ubuf_alloc(struct vhost_virtqueue *vq, bool zcopy)
250251

251252
static int vhost_net_ubuf_put(struct vhost_net_ubuf_ref *ubufs)
252253
{
253-
int r = atomic_sub_return(1, &ubufs->refcount);
254+
int r;
255+
256+
rcu_read_lock();
257+
r = atomic_sub_return(1, &ubufs->refcount);
254258
if (unlikely(!r))
255259
wake_up(&ubufs->wait);
260+
rcu_read_unlock();
256261
return r;
257262
}
258263

@@ -265,7 +270,7 @@ static void vhost_net_ubuf_put_and_wait(struct vhost_net_ubuf_ref *ubufs)
265270
static void vhost_net_ubuf_put_wait_and_free(struct vhost_net_ubuf_ref *ubufs)
266271
{
267272
vhost_net_ubuf_put_and_wait(ubufs);
268-
kfree(ubufs);
273+
kfree_rcu(ubufs, rcu);
269274
}
270275

271276
static void vhost_net_clear_ubuf_info(struct vhost_net *n)

0 commit comments

Comments
 (0)