On 8/12/21 9:09 AM, Maximilian Heyne wrote:
> There is a TOCTOU issue in set_evtchn_to_irq. Rows in the evtchn_to_irq
> mapping are lazily allocated in this function. The check whether the row
> is already present and the row initialization is not synchronized. Two
> threads can at the same time allocate a new row for evtchn_to_irq and
> add the irq mapping to the their newly allocated row. One thread will
> overwrite what the other has set for evtchn_to_irq[row] and therefore
> the irq mapping is lost. This will trigger a BUG_ON later in
> bind_evtchn_to_cpu:
>
> INFO: pci 0000:1a:15.4: [1d0f:8061] type 00 class 0x010802
> INFO: nvme 0000:1a:12.1: enabling device (0000 -> 0002)
> INFO: nvme nvme77: 1/0/0 default/read/poll queues
> CRIT: kernel BUG at drivers/xen/events/events_base.c:427!
> WARN: invalid opcode: 0000 [#1] SMP NOPTI
> WARN: Workqueue: nvme-reset-wq nvme_reset_work [nvme]
> WARN: RIP: e030:bind_evtchn_to_cpu+0xc2/0xd0
> WARN: Call Trace:
> WARN: set_affinity_irq+0x121/0x150
> WARN: irq_do_set_affinity+0x37/0xe0
> WARN: irq_setup_affinity+0xf6/0x170
> WARN: irq_startup+0x64/0xe0
> WARN: __setup_irq+0x69e/0x740
> WARN: ? request_threaded_irq+0xad/0x160
> WARN: request_threaded_irq+0xf5/0x160
> WARN: ? nvme_timeout+0x2f0/0x2f0 [nvme]
> WARN: pci_request_irq+0xa9/0xf0
> WARN: ? pci_alloc_irq_vectors_affinity+0xbb/0x130
> WARN: queue_request_irq+0x4c/0x70 [nvme]
> WARN: nvme_reset_work+0x82d/0x1550 [nvme]
> WARN: ? check_preempt_wakeup+0x14f/0x230
> WARN: ? check_preempt_curr+0x29/0x80
> WARN: ? nvme_irq_check+0x30/0x30 [nvme]
> WARN: process_one_work+0x18e/0x3c0
> WARN: worker_thread+0x30/0x3a0
> WARN: ? process_one_work+0x3c0/0x3c0
> WARN: kthread+0x113/0x130
> WARN: ? kthread_park+0x90/0x90
> WARN: ret_from_fork+0x3a/0x50
>
> This patch sets evtchn_to_irq rows via a cmpxchg operation so that they
> will be set only once. The row is now cleared before writing it to
> evtchn_to_irq in order to not create a race once the row is visible for
> other threads.
>
> While at it, do not require the page to be zeroed, because it will be
> overwritten with -1's in clear_evtchn_to_irq_row anyway.
>
> Signed-off-by: Maximilian Heyne <mhe...@amazon.de>
> Fixes: d0b075ffeede ("xen/events: Refactor evtchn_to_irq array to be
> dynamically allocated")
Reviewed-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
