mirror of
https://github.com/torvalds/linux.git
synced 2026-09-14 08:01:12 +02:00
virtio_console: refactor __send_to_port() buffer ownership
Modify __send_to_port() to take ownership of a struct port_buffer * instead of a void * raw buffer. Previously, put_chars() would pass a raw kmemdup'd buffer and free it immediately after __send_to_port() returned. This caused a potential Use-After-Free and data corruption if the virtqueue was shared with nonblocking writers, as virtqueue_get_buf() might return an older completed buffer, causing the newly added buffer to be kfree'd while the host is still DMAing from it. By transferring ownership of the allocated port_buffer to __send_to_port(), we ensure that the exact buffer returned by the host is the one that gets freed, resolving the memory lifecycle mismatch. Signed-off-by: Sungho Bae <baver.bae@lge.com> Link: https://patch.msgid.link/20260603183757.21587-2-baver.bae@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
c16ce856e4
commit
fc220d6be3
|
|
@ -410,7 +410,7 @@ static void reclaim_dma_bufs(void)
|
||||||
}
|
}
|
||||||
|
|
||||||
static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size,
|
static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size,
|
||||||
int pages)
|
int pages, gfp_t gfp)
|
||||||
{
|
{
|
||||||
struct port_buffer *buf;
|
struct port_buffer *buf;
|
||||||
|
|
||||||
|
|
@ -444,11 +444,10 @@ static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size
|
||||||
|
|
||||||
/* Increase device refcnt to avoid freeing it */
|
/* Increase device refcnt to avoid freeing it */
|
||||||
get_device(buf->dev);
|
get_device(buf->dev);
|
||||||
buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
|
buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma, gfp);
|
||||||
GFP_KERNEL);
|
|
||||||
} else {
|
} else {
|
||||||
buf->dev = NULL;
|
buf->dev = NULL;
|
||||||
buf->buf = kmalloc(buf_size, GFP_KERNEL);
|
buf->buf = kmalloc(buf_size, gfp);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!buf->buf)
|
if (!buf->buf)
|
||||||
|
|
@ -603,7 +602,7 @@ static void reclaim_consumed_buffers(struct port *port)
|
||||||
|
|
||||||
static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
|
static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
|
||||||
int nents, size_t in_count,
|
int nents, size_t in_count,
|
||||||
void *data, bool nonblock)
|
struct port_buffer *buf, bool nonblock)
|
||||||
{
|
{
|
||||||
struct virtqueue *out_vq;
|
struct virtqueue *out_vq;
|
||||||
int err;
|
int err;
|
||||||
|
|
@ -616,14 +615,14 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
|
||||||
|
|
||||||
reclaim_consumed_buffers(port);
|
reclaim_consumed_buffers(port);
|
||||||
|
|
||||||
err = virtqueue_add_outbuf(out_vq, sg, nents, data, GFP_ATOMIC);
|
err = virtqueue_add_outbuf(out_vq, sg, nents, buf, GFP_ATOMIC);
|
||||||
|
|
||||||
/* Tell Host to go! */
|
/* Tell Host to go! */
|
||||||
virtqueue_kick(out_vq);
|
virtqueue_kick(out_vq);
|
||||||
|
|
||||||
if (err) {
|
if (err) {
|
||||||
in_count = 0;
|
in_count = 0;
|
||||||
goto done;
|
goto free_and_done;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (out_vq->num_free == 0)
|
if (out_vq->num_free == 0)
|
||||||
|
|
@ -640,10 +639,19 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
|
||||||
* buffer and relax the spinning requirement. The downside is
|
* buffer and relax the spinning requirement. The downside is
|
||||||
* we need to kmalloc a GFP_ATOMIC buffer each time the
|
* we need to kmalloc a GFP_ATOMIC buffer each time the
|
||||||
* console driver writes something out.
|
* console driver writes something out.
|
||||||
|
*
|
||||||
|
* Spin until host returns the buffer.
|
||||||
|
* Capture the returned buf so we can free it.
|
||||||
|
* If broken, buf == NULL and buf stays in the vq;
|
||||||
|
* remove_vqs() will call virtqueue_detach_unused_buf() -> free_buf().
|
||||||
*/
|
*/
|
||||||
while (!virtqueue_get_buf(out_vq, &len)
|
while (!(buf = virtqueue_get_buf(out_vq, &len))
|
||||||
&& !virtqueue_is_broken(out_vq))
|
&& !virtqueue_is_broken(out_vq))
|
||||||
cpu_relax();
|
cpu_relax();
|
||||||
|
|
||||||
|
free_and_done:
|
||||||
|
if (buf)
|
||||||
|
free_buf(buf, false);
|
||||||
done:
|
done:
|
||||||
spin_unlock_irqrestore(&port->outvq_lock, flags);
|
spin_unlock_irqrestore(&port->outvq_lock, flags);
|
||||||
|
|
||||||
|
|
@ -824,14 +832,14 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
|
||||||
|
|
||||||
count = min((size_t)(32 * 1024), count);
|
count = min((size_t)(32 * 1024), count);
|
||||||
|
|
||||||
buf = alloc_buf(port->portdev->vdev, count, 0);
|
buf = alloc_buf(port->portdev->vdev, count, 0, GFP_KERNEL);
|
||||||
if (!buf)
|
if (!buf)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
ret = copy_from_user(buf->buf, ubuf, count);
|
ret = copy_from_user(buf->buf, ubuf, count);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
ret = -EFAULT;
|
free_buf(buf, true);
|
||||||
goto free_buf;
|
return -EFAULT;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|
@ -843,15 +851,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
|
||||||
*/
|
*/
|
||||||
nonblock = true;
|
nonblock = true;
|
||||||
sg_init_one(sg, buf->buf, count);
|
sg_init_one(sg, buf->buf, count);
|
||||||
ret = __send_to_port(port, sg, 1, count, buf, nonblock);
|
return __send_to_port(port, sg, 1, count, buf, nonblock);
|
||||||
|
|
||||||
if (nonblock && ret > 0)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
free_buf:
|
|
||||||
free_buf(buf, true);
|
|
||||||
out:
|
|
||||||
return ret;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
struct sg_list {
|
struct sg_list {
|
||||||
|
|
@ -940,7 +940,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
|
||||||
goto error_out;
|
goto error_out;
|
||||||
|
|
||||||
occupancy = pipe_buf_usage(pipe);
|
occupancy = pipe_buf_usage(pipe);
|
||||||
buf = alloc_buf(port->portdev->vdev, 0, occupancy);
|
buf = alloc_buf(port->portdev->vdev, 0, occupancy, GFP_KERNEL);
|
||||||
|
|
||||||
if (!buf) {
|
if (!buf) {
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
|
|
@ -954,11 +954,12 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
|
||||||
sg_init_table(sgl.sg, sgl.size);
|
sg_init_table(sgl.sg, sgl.size);
|
||||||
ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
|
ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
|
||||||
pipe_unlock(pipe);
|
pipe_unlock(pipe);
|
||||||
|
|
||||||
if (likely(ret > 0))
|
if (likely(ret > 0))
|
||||||
ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
|
ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
|
||||||
|
else
|
||||||
if (unlikely(ret <= 0))
|
|
||||||
free_buf(buf, true);
|
free_buf(buf, true);
|
||||||
|
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
error_out:
|
error_out:
|
||||||
|
|
@ -1116,21 +1117,25 @@ static ssize_t put_chars(u32 vtermno, const u8 *buf, size_t count)
|
||||||
{
|
{
|
||||||
struct port *port;
|
struct port *port;
|
||||||
struct scatterlist sg[1];
|
struct scatterlist sg[1];
|
||||||
void *data;
|
struct port_buffer *pbuf;
|
||||||
int ret;
|
|
||||||
|
|
||||||
port = find_port_by_vtermno(vtermno);
|
port = find_port_by_vtermno(vtermno);
|
||||||
if (!port)
|
if (!port)
|
||||||
return -EPIPE;
|
return -EPIPE;
|
||||||
|
|
||||||
data = kmemdup(buf, count, GFP_ATOMIC);
|
pbuf = alloc_buf(port->portdev->vdev, count, 0, GFP_ATOMIC);
|
||||||
if (!data)
|
if (!pbuf)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
sg_init_one(sg, data, count);
|
memcpy(pbuf->buf, buf, count);
|
||||||
ret = __send_to_port(port, sg, 1, count, data, false);
|
pbuf->len = count;
|
||||||
kfree(data);
|
sg_init_one(sg, pbuf->buf, count);
|
||||||
return ret;
|
|
||||||
|
/*
|
||||||
|
* Ownership of pbuf is transferred to __send_to_port().
|
||||||
|
* Do not touch or free pbuf after this call.
|
||||||
|
*/
|
||||||
|
return __send_to_port(port, sg, 1, count, pbuf, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|
@ -1303,7 +1308,7 @@ static int fill_queue(struct virtqueue *vq, spinlock_t *lock)
|
||||||
|
|
||||||
nr_added_bufs = 0;
|
nr_added_bufs = 0;
|
||||||
do {
|
do {
|
||||||
buf = alloc_buf(vq->vdev, PAGE_SIZE, 0);
|
buf = alloc_buf(vq->vdev, PAGE_SIZE, 0, GFP_KERNEL);
|
||||||
if (!buf)
|
if (!buf)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user