Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

virtio_pci: Fix admin vq cleanup by using correct info pointer

vp_modern_avq_cleanup() and vp_del_vqs() clean up admin vq
resources by virtio_pci_vq_info pointer. The info pointer of admin
vq is stored in vp_dev->admin_vq.info instead of vp_dev->vqs[].
Using the info pointer from vp_dev->vqs[] for admin vq causes a
kernel NULL pointer dereference bug.
In vp_modern_avq_cleanup() and vp_del_vqs(), get the info pointer
from vp_dev->admin_vq.info for admin vq to clean up the resources.
Also make info ptr as argument of vp_del_vq() to be symmetric with
vp_setup_vq().

vp_reset calls vp_modern_avq_cleanup, and causes the Call Trace:
==================================================================
BUG: kernel NULL pointer dereference, address:0000000000000000
...
CPU: 49 UID: 0 PID: 4439 Comm: modprobe Not tainted 6.11.0-rc5 #1
RIP: 0010:vp_reset+0x57/0x90 [virtio_pci]
Call Trace:
<TASK>
...
? vp_reset+0x57/0x90 [virtio_pci]
? vp_reset+0x38/0x90 [virtio_pci]
virtio_reset_device+0x1d/0x30
remove_vq_common+0x1c/0x1a0 [virtio_net]
virtnet_remove+0xa1/0xc0 [virtio_net]
virtio_dev_remove+0x46/0xa0
...
virtio_pci_driver_exit+0x14/0x810 [virtio_pci]
==================================================================

Fixes: 4c3b54af907e ("virtio_pci_modern: use completion instead of busy loop to wait on admin cmd result")
Signed-off-by: Feng Liu <feliu@nvidia.com>
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Message-Id: <20241024135406.81388-1-feliu@nvidia.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

authored by

Feng Liu and committed by
Michael S. Tsirkin
97ee04fe 7f8825b2

+20 -17
+18 -6
drivers/virtio/virtio_pci_common.c
··· 24 24 "Force legacy mode for transitional virtio 1 devices"); 25 25 #endif 26 26 27 + bool vp_is_avq(struct virtio_device *vdev, unsigned int index) 28 + { 29 + struct virtio_pci_device *vp_dev = to_vp_device(vdev); 30 + 31 + if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) 32 + return false; 33 + 34 + return index == vp_dev->admin_vq.vq_index; 35 + } 36 + 27 37 /* wait for pending irq handlers */ 28 38 void vp_synchronize_vectors(struct virtio_device *vdev) 29 39 { ··· 244 234 return vq; 245 235 } 246 236 247 - static void vp_del_vq(struct virtqueue *vq) 237 + static void vp_del_vq(struct virtqueue *vq, struct virtio_pci_vq_info *info) 248 238 { 249 239 struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); 250 - struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index]; 251 240 unsigned long flags; 252 241 253 242 /* ··· 267 258 void vp_del_vqs(struct virtio_device *vdev) 268 259 { 269 260 struct virtio_pci_device *vp_dev = to_vp_device(vdev); 261 + struct virtio_pci_vq_info *info; 270 262 struct virtqueue *vq, *n; 271 263 int i; 272 264 273 265 list_for_each_entry_safe(vq, n, &vdev->vqs, list) { 274 - if (vp_dev->per_vq_vectors) { 275 - int v = vp_dev->vqs[vq->index]->msix_vector; 266 + info = vp_is_avq(vdev, vq->index) ? vp_dev->admin_vq.info : 267 + vp_dev->vqs[vq->index]; 276 268 269 + if (vp_dev->per_vq_vectors) { 270 + int v = info->msix_vector; 277 271 if (v != VIRTIO_MSI_NO_VECTOR && 278 272 !vp_is_slow_path_vector(v)) { 279 273 int irq = pci_irq_vector(vp_dev->pci_dev, v); ··· 285 273 free_irq(irq, vq); 286 274 } 287 275 } 288 - vp_del_vq(vq); 276 + vp_del_vq(vq, info); 289 277 } 290 278 vp_dev->per_vq_vectors = false; 291 279 ··· 366 354 vring_interrupt, 0, 367 355 vp_dev->msix_names[msix_vec], vq); 368 356 if (err) { 369 - vp_del_vq(vq); 357 + vp_del_vq(vq, *p_info); 370 358 return ERR_PTR(err); 371 359 } 372 360
+1
drivers/virtio/virtio_pci_common.h
··· 178 178 #define VIRTIO_ADMIN_CMD_BITMAP 0 179 179 #endif 180 180 181 + bool vp_is_avq(struct virtio_device *vdev, unsigned int index); 181 182 void vp_modern_avq_done(struct virtqueue *vq); 182 183 int vp_modern_admin_cmd_exec(struct virtio_device *vdev, 183 184 struct virtio_admin_cmd *cmd);
+1 -11
drivers/virtio/virtio_pci_modern.c
··· 43 43 return 0; 44 44 } 45 45 46 - static bool vp_is_avq(struct virtio_device *vdev, unsigned int index) 47 - { 48 - struct virtio_pci_device *vp_dev = to_vp_device(vdev); 49 - 50 - if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) 51 - return false; 52 - 53 - return index == vp_dev->admin_vq.vq_index; 54 - } 55 - 56 46 void vp_modern_avq_done(struct virtqueue *vq) 57 47 { 58 48 struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); ··· 235 245 if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) 236 246 return; 237 247 238 - vq = vp_dev->vqs[vp_dev->admin_vq.vq_index]->vq; 248 + vq = vp_dev->admin_vq.info->vq; 239 249 if (!vq) 240 250 return; 241 251