All of lore.kernel.org
 help / color / mirror / Atom feed
From: Karl Mehltretter <kmehltretter@gmail.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux.dev, jasowangio@gmail.com,
	 eperezma@redhat.com, xuanzhuo@linux.alibaba.com,
	jiri@resnulli.us,  sashiko-bot@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/3] virtio_pci_modern: move avq cleanup from reset to del_vqs
Date: Sat, 12 Sep 2026 00:48:54 +0200	[thread overview]
Message-ID: <aqSDrWpFy84CxCoP@gmail.com> (raw)
In-Reply-To: <d0b3de807c800befe9adce1cefb2d064d9fe0bec.1789160982.git.mst@redhat.com>

On Fri, Sep 11, 2026 at 05:20:15PM +0100, Michael S. Tsirkin wrote:
> New in v3. v2 dropped sync from modern vp_reset in one combined
> patch, leaving avq_cleanup in vp_reset. v3 moves avq_cleanup out
> of vp_reset entirely into vp_del_vqs, fixing the race. Adds NULL
> check for admin_vq.info for find_vqs error paths.
> 

I think patch 3 introduces a use-after-free in the MSI-X setup error
path.

With VP_VQ_VECTOR_POLICY_EACH, vp_setup_vq() stores the admin queue's
info pointer before request_irq(). If request_irq() fails, vp_del_vq()
frees info but leaves admin_vq.info set.

vp_find_vqs_msix() then calls vp_del_vqs(). vp_del_vqs() now calls
vp_modern_avq_cleanup() first and dereferences the freed pointer:

    vq = vp_dev->admin_vq.info->vq;

No interrupt needs to fire.

I reproduced this on v3 with KASAN by returning -ENOMEM only for the
admin queue's request_irq():

    BUG: KASAN: slab-use-after-free in vp_modern_avq_cleanup+0xf4/0x110

KASAN shows the allocation in vp_setup_vq(), the free in
vp_find_one_vq_msix(), and the access in vp_modern_avq_cleanup(). Adding
*p_info = NULL after vp_del_vq() made the same test fall back and boot
without a KASAN report.

Thanks,
Karl

  parent reply	other threads:[~2026-09-11 22:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 21:20 [PATCH v3 0/3] virtio: fix callback synchronization and avq cleanup on reset Michael S. Tsirkin
2026-09-11 21:20 ` [PATCH v3 1/3] virtio: synchronize callbacks after device reset Michael S. Tsirkin
2026-09-11 21:37   ` sashiko-bot
2026-09-11 22:50   ` Karl Mehltretter
2026-09-11 21:20 ` [PATCH v3 3/3] virtio_pci_modern: move avq cleanup from reset to del_vqs Michael S. Tsirkin
2026-09-11 21:38   ` sashiko-bot
2026-09-11 22:48   ` Karl Mehltretter [this message]
2026-09-11 22:51     ` Michael S. Tsirkin
2026-09-11 21:20 ` [PATCH v3 2/3] virtio_pci_legacy: drop callback sync on reset Michael S. Tsirkin
2026-09-11 21:38   ` sashiko-bot
2026-09-11 22:51   ` Karl Mehltretter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqSDrWpFy84CxCoP@gmail.com \
    --to=kmehltretter@gmail.com \
    --cc=eperezma@redhat.com \
    --cc=jasowangio@gmail.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=sashiko-bot@kernel.org \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.