Re: [PATCH 21/39] irqdomain: Introduce irq_resolve_mapping()

From: Qian Cai
Date: Wed Jun 09 2021 - 22:23:03 EST



On 5/20/2021 12:37 PM, Marc Zyngier wrote:
> Rework irq_find_mapping() to return an irq_desc pointer, and
> rename the result to irq_resolve_mapping().
>
> irq_find_mapping() is then rewritten in terms of ir_resolve_mapping().
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>

Marc, there is an use-after-free that "git blame" pointed to this commit. Any thoughts?

__irq_resolve_mapping+0x34c/0x360:
__irq_resolve_mapping at /usr/src/linux-next/kernel/irq/irqdomain.c:917

[ 52.773282][ C0] BUG: KASAN: use-after-free in __irq_resolve_mapping+0x34c/0x360
[ 52.780946][ C0] Read of size 8 at addr ffff00005e8e4950 by task swapper/0/0
[ 52.788256][ C0]
[ 52.790441][ C0] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 5.13.0-rc5-next-20210609+ #22
[ 52.800181][ C0] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020
[ 52.808706][ C0] Call trace:
[ 52.811845][ C0] dump_backtrace+0x0/0x3b8
[ 52.816207][ C0] show_stack+0x20/0x30
[ 52.820217][ C0] dump_stack_lvl+0x144/0x190
[ 52.824749][ C0] print_address_description.constprop.0+0x74/0x3c8
[ 52.831191][ C0] kasan_report+0x1f0/0x208
[ 52.835548][ C0] __asan_report_load8_noabort+0x34/0x60
[ 52.841034][ C0] __irq_resolve_mapping+0x34c/0x360
[ 52.846171][ C0] handle_domain_irq+0x88/0x168
[ 52.850875][ C0] gic_handle_irq+0x5c/0x150
[ 52.855319][ C0] call_on_irq_stack+0x28/0x50
[ 52.859936][ C0] do_interrupt_handler+0x80/0xa8
[ 52.864813][ C0] el1_interrupt+0x30/0x48
[ 52.869083][ C0] el1h_64_irq_handler+0x18/0x28
[ 52.873874][ C0] el1h_64_irq+0x74/0x78
[ 52.877968][ C0] cpuidle_enter_state+0x2a4/0xff0
[ 52.882934][ C0] cpuidle_enter+0x60/0xb0
[ 52.887203][ C0] call_cpuidle+0x44/0xc0
[ 52.891385][ C0] do_idle+0x36c/0x420
[ 52.895306][ C0] cpu_startup_entry+0x30/0x88
[ 52.899923][ C0] rest_init+0x380/0x4f0
[ 52.904017][ C0] arch_call_rest_init+0x18/0x24
[ 52.908809][ C0] start_kernel+0x3b4/0x3ec
[ 52.913165][ C0] __primary_switched+0xc0/0xc8
[ 52.917869][ C0]
[ 52.920051][ C0] Allocated by task 7:
[ 52.923970][ C0] kasan_save_stack+0x28/0x58
[ 52.928500][ C0] __kasan_kmalloc+0x8c/0xb0
[ 52.932942][ C0] kmem_cache_alloc_node_trace+0x288/0x3c8
[ 52.938600][ C0] __irq_domain_alloc_irqs+0x110/0x738
[ 52.943911][ C0] __msi_domain_alloc_irqs+0x1f8/0xc18
[ 52.949221][ C0] msi_domain_alloc_irqs+0x78/0xd0
[ 52.954184][ C0] pci_msi_setup_msi_irqs.isra.0+0x78/0xb8
[ 52.959843][ C0] __pci_enable_msix_range+0x5d8/0xc10
[ 52.965152][ C0] pci_alloc_irq_vectors_affinity+0x144/0x1e8
[ 52.971069][ C0] nvme_reset_work+0x338/0x2aa4 [nvme]
[ 52.976391][ C0] process_one_work+0x7e4/0x1998
[ 52.981182][ C0] worker_thread+0x334/0xad0
[ 52.985623][ C0] kthread+0x3ac/0x460
[ 52.989545][ C0] ret_from_fork+0x10/0x18
[ 52.993813][ C0]
[ 52.995995][ C0] Freed by task 7:
[ 52.999567][ C0] kasan_save_stack+0x28/0x58
[ 53.004100][ C0] kasan_set_track+0x28/0x40
[ 53.008548][ C0] kasan_set_free_info+0x28/0x50
[ 53.013339][ C0] __kasan_slab_free+0xfc/0x150
[ 53.018042][ C0] slab_free_freelist_hook+0x128/0x258
[ 53.023353][ C0] kfree+0x150/0x4c0
[ 53.027100][ C0] irq_domain_free_irq_data+0x94/0xe0
[ 53.032323][ C0] irq_domain_free_irqs+0x200/0x2d0
[ 53.037373][ C0] __msi_domain_free_irqs+0xbc/0x138
[ 53.042509][ C0] msi_domain_free_irqs+0x78/0xb8
[ 53.047385][ C0] free_msi_irqs+0x420/0x598
[ 53.051827][ C0] pci_disable_msix+0x250/0x330
[ 53.056528][ C0] pci_free_irq_vectors+0x20/0x38
[ 53.061404][ C0] nvme_reset_work+0x11dc/0x2aa4 [nvme]
[ 53.066809][ C0] process_one_work+0x7e4/0x1998
[ 53.071599][ C0] worker_thread+0x334/0xad0
[ 53.076040][ C0] kthread+0x3ac/0x460
[ 53.079962][ C0] ret_from_fork+0x10/0x18
[ 53.084230][ C0]
[ 53.086411][ C0] The buggy address belongs to the object at ffff00005e8e4940
[ 53.086411][ C0] which belongs to the cache kmalloc-64 of size 64
[ 53.100141][ C0] The buggy address is located 16 bytes inside of
[ 53.100141][ C0] 64-byte region [ffff00005e8e4940, ffff00005e8e4980)
[ 53.113090][ C0] The buggy address belongs to the page:
[ 53.118574][ C0] page:ffffffc00017a380 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xde8e
[ 53.128486][ C0] flags: 0x7ffff800000200(slab|node=0|zone=0|lastcpupid=0xfffff)
[ 53.136059][ C0] raw: 007ffff800000200 ffffffc00211b648 ffffffc00211ec88 ffff0000129103c0
[ 53.144494][ C0] raw: 0000000000000000 0000000001000100 00000001ffffffff 0000000000000000
[ 53.152927][ C0] page dumped because: kasan: bad access detected
[ 53.159189][ C0]
[ 53.161371][ C0] Memory state around the buggy address:
[ 53.166853][ C0] ffff00005e8e4800: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
[ 53.174765][ C0] ffff00005e8e4880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 53.182678][ C0] >ffff00005e8e4900: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
[ 53.190590][ C0] ^
[ 53.197114][ C0] ffff00005e8e4980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 53.205027][ C0] ffff00005e8e4a00: fc fc fc fc fc fc fc fc 00 00 00 00 00 00 00 00
[ 53.212939][ C0] ==================================================================
[ 53.220850][ C0] Disabling lock debugging due to kernel taint
[ 53.226850][ C0] irq 320, desc: ffff000840982800, depth: 1, count: 0, unhandled: 0
[ 53.234672][ C0] ->handle_irq(): ffff800010258d30, handle_bad_irq+0x0/0x570
[ 53.241980][ C0] ->irq_data.chip(): ffff800018d20520, no_irq_chip+0x0/0x760
[ 53.241988][ C0] ->action(): 0000000000000000


> ---
> include/linux/irqdomain.h | 18 +++++++++++++++---
> kernel/irq/irqdomain.c | 22 ++++++++++++++--------
> 2 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 2b696c9bcaaf..189f559fb26c 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -31,7 +31,8 @@
> #define _LINUX_IRQDOMAIN_H
>
> #include <linux/types.h>
> -#include <linux/irqhandler.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
> #include <linux/of.h>
> #include <linux/mutex.h>
> #include <linux/radix-tree.h>
> @@ -401,13 +402,24 @@ static inline unsigned int irq_create_mapping(struct irq_domain *host,
> return irq_create_mapping_affinity(host, hwirq, NULL);
> }
>
> +extern struct irq_desc *irq_resolve_mapping(struct irq_domain *domain,
> + irq_hw_number_t hwirq);
> +
> /**
> * irq_find_mapping() - Find a linux irq from a hw irq number.
> * @domain: domain owning this hardware interrupt
> * @hwirq: hardware irq number in that domain space
> */
> -extern unsigned int irq_find_mapping(struct irq_domain *host,
> - irq_hw_number_t hwirq);
> +static inline unsigned int irq_find_mapping(struct irq_domain *domain,
> + irq_hw_number_t hwirq)
> +{
> + struct irq_desc *desc = irq_resolve_mapping(domain, hwirq);
> +
> + if (unlikely(!desc))
> + return 0;
> +
> + return irq_desc_get_irq(desc);
> +}
>
> static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
> irq_hw_number_t hwirq)
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 8e55bb8c3073..13a3021e9370 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -876,29 +876,32 @@ void irq_dispose_mapping(unsigned int virq)
> EXPORT_SYMBOL_GPL(irq_dispose_mapping);
>
> /**
> - * irq_find_mapping() - Find a linux irq from a hw irq number.
> + * irq_resolve_mapping() - Find a linux irq from a hw irq number.
> * @domain: domain owning this hardware interrupt
> * @hwirq: hardware irq number in that domain space
> + *
> + * Returns the interrupt descriptor.
> */
> -unsigned int irq_find_mapping(struct irq_domain *domain,
> - irq_hw_number_t hwirq)
> +struct irq_desc *irq_resolve_mapping(struct irq_domain *domain,
> + irq_hw_number_t hwirq)
> {
> + struct irq_desc *desc = NULL;
> struct irq_data *data;
>
> /* Look for default domain if necessary */
> if (domain == NULL)
> domain = irq_default_domain;
> if (domain == NULL)
> - return 0;
> + return desc;
>
> if (irq_domain_is_nomap(domain)) {
> if (hwirq < domain->revmap_size) {
> data = irq_domain_get_irq_data(domain, hwirq);
> if (data && data->hwirq == hwirq)
> - return hwirq;
> + desc = irq_data_to_desc(data);
> }
>
> - return 0;
> + return desc;
> }
>
> rcu_read_lock();
> @@ -907,10 +910,13 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
> data = rcu_dereference(domain->revmap[hwirq]);
> else
> data = radix_tree_lookup(&domain->revmap_tree, hwirq);
> +
> + if (likely(data))
> + desc = irq_data_to_desc(data);
> rcu_read_unlock();
> - return data ? data->irq : 0;
> + return desc;
> }
> -EXPORT_SYMBOL_GPL(irq_find_mapping);
> +EXPORT_SYMBOL_GPL(irq_resolve_mapping);
>
> /**
> * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
>