netdev - Preemptible idr_alloc() in QRTR code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210126104734.GB80448@C02TD0UTHF1T.local>
Date:   Tue, 26 Jan 2021 10:47:34 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Cc:     Matthew Wilcox <willy@...radead.org>,
        Courtney Cavin <courtney.cavin@...ymobile.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Preemptible idr_alloc() in QRTR code

Hi,

When fuzzing arm64 with Syzkaller, I'm seeing some splats where
this_cpu_ptr() is used in the bowels of idr_alloc(), by way of
radix_tree_node_alloc(), in a preemptible context:

| BUG: using smp_processor_id() in preemptible [00000000] code: syz-executor.1/32582
| caller is debug_smp_processor_id+0x24/0x30
| CPU: 3 PID: 32582 Comm: syz-executor.1 Not tainted 5.11.0-rc4-next-20210125-00001-gf57e7edf910d #3
| Hardware name: linux,dummy-virt (DT)
| Call trace:
|  dump_backtrace+0x0/0x4a8
|  show_stack+0x34/0x88
|  dump_stack+0x1d4/0x2a0
|  check_preemption_disabled+0x1b8/0x210
|  debug_smp_processor_id+0x24/0x30
|  radix_tree_node_alloc.constprop.17+0x26c/0x3d0
|  radix_tree_extend+0x200/0x420
|  idr_get_free+0x63c/0xa38
|  idr_alloc_u32+0x164/0x2a0
|  __qrtr_bind.isra.8+0x350/0x658
|  qrtr_bind+0x18c/0x218
|  __sys_bind+0x1fc/0x238
|  __arm64_sys_bind+0x78/0xb0
|  el0_svc_common+0x1ac/0x4c8
|  do_el0_svc+0xfc/0x150
|  el0_svc+0x24/0x38
|  el0_sync_handler+0x134/0x180
|  el0_sync+0x154/0x180

It's not clear to me whether this is a bug in the caller or a bug the
implementation of idr_alloc(). The kerneldoc for idr_alloc() mentions
that callers must provide their own locking (and in this case a mutex is
used), but doesn't mention that preemption must be disabled.

Is this an intentional requirement that's simply missing from the
documentation and requires a change to the QRTR code, or is this
something to fix within the bowels of idr_alloc() and its callees?

Thanks,
Mark.