linux-kernel - Re: qrtr/mhi: NULL-deref with in-kernel pd-mapper

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-prev] [day] [month] [year] [list]
Message-ID: <730a6e17-04f5-41b5-a0d8-7220b8c59b58@quicinc.com>
Date: Tue, 5 Nov 2024 10:26:40 -0800
From: Chris Lew <quic_clew@...cinc.com>
To: Johan Hovold <johan@...nel.org>
CC: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Bjorn Andersson
	<bjorn.andersson@....qualcomm.com>,
        Qiang Yu <quic_qianyu@...cinc.com>,
        Jeffrey Hugo <quic_jhugo@...cinc.com>,
        Krishna chaitanya chundru
	<quic_krichai@...cinc.com>,
        Konrad Dybcio <konradybcio@...nel.org>, <mhi@...ts.linux.dev>,
        <linux-arm-msm@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: qrtr/mhi: NULL-deref with in-kernel pd-mapper



On 11/4/2024 9:08 PM, Johan Hovold wrote:
> On Mon, Nov 04, 2024 at 04:26:15PM -0800, Chris Lew wrote:
>> On 11/1/2024 8:01 AM, Johan Hovold wrote:
> 
>>> [    8.825593] Unable to handle kernel NULL pointer dereference at virtual
>>> address 0000000000000034
>>> .
> 
>>> [    9.002030] CPU: 10 UID: 0 PID: 11 Comm: kworker/u48:0 Not tainted 6.12.0-rc5 #4
>>> [    9.029550] Hardware name: Qualcomm CRD, BIOS 6.0.231221.BOOT.MXF.2.4-00348.1-HAMOA-1 12/21/2023
>>> [    9.029552] Workqueue: qrtr_ns_handler qrtr_ns_worker [qrtr]
>>> [    9.061350] pstate: a1400005 (NzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
>>> [    9.061353] pc : mhi_gen_tre+0x44/0x224 [mhi]
>>> [    9.106931] lr : mhi_gen_tre+0x40/0x224 [mhi]
>>> [    9.106934] sp : ffff8000800fb7d0
>>> [    9.106935] x29: ffff8000800fb7d0 x28: ffff6db7852bd000 x27: ffff800082490188
>>> [    9.120382] dwc3 a000000.usb: Adding to iommu group 5
>>> [    9.133750]
>>> [    9.133752] x26: 0000000000000000 x25: ffff6db783e65080 x24: ffff80008248ff88
>>> [    9.133754] x23: 0000000000000000 x22: ffff80008248ff80 x21: ffff8000800fb890
>>> [    9.133756] x20: 0000000000000000 x19: 0000000000000002 x18: 000000000005cf20
>>> [    9.133758] x17: 0000000000000028 x16: 0000000000000000
>>> [    9.172738]  x15: ffffa5834131fbd0
>>> [    9.172741] x14: ffffa5834137caf0 x13: 000000000000ce30 x12: ffff6db7808bc028
>>> [    9.172743] x11: ffffa58341993000 x10: 0000000000000000 x9 : 00000000cf3f2b90
>>> [    9.172745] x8 : 0000000094e5072b x7 : 00000000000404ce x6 : ffffa5834162cfb0
>>> [    9.172747] x5 : 000000000000008b x4 : ffffa583419cddf0 x3 : 0000000000000007
>>> [    9.172750] x2 : 0000000000000000
>>> [    9.192697]  x1 : 000000000000000a x0 : ffff6db7808bb700
>>> [    9.192700] Call trace:
>>> [    9.192701]  mhi_gen_tre+0x44/0x224 [mhi]
>>> [    9.192704]  mhi_queue+0x74/0x194 [mhi]
>>> [    9.192706]  mhi_queue_skb+0x5c/0x8c [mhi]
>>> [    9.210985]  qcom_mhi_qrtr_send+0x6c/0x160 [qrtr_mhi]
>>> [    9.210989]  qrtr_node_enqueue+0xd0/0x4a0 [qrtr]
>>> [    9.210992]  qrtr_bcast_enqueue+0x78/0xe8 [qrtr]
>>> [    9.225530]  qrtr_sendmsg+0x15c/0x33c [qrtr]
>>> [    9.225532]  sock_sendmsg+0xc0/0xec
>>> [    9.240436]  kernel_sendmsg+0x30/0x40
>>> [    9.240438]  service_announce_new+0xbc/0x1c4 [qrtr]
>>> [    9.240440]  qrtr_ns_worker+0x714/0x794 [qrtr]
>>> [    9.240441]  process_one_work+0x210/0x614
>>> [    9.254527]  worker_thread+0x23c/0x378
>>> [    9.254529]  kthread+0x124/0x128
>>> [    9.254531]  ret_from_fork+0x10/0x20
>>> [    9.254534] Code: aa0003f9 aa1b03e0 94001a4d f9401b14 (3940d280)
>>> [    9.267369] ---[ end trace 0000000000000000 ]---
>>> [    9.267371] Kernel panic - not syncing: Oops: Fatal exception in interrupt
>>
>> Thanks for reporting this.
> 
> Thanks for taking a look, Chris.
> 
>> I'm not sure the in-kernel pd-mapper should be affecting this path. I
>> think this is for WLAN since it is the mhi qrtr and I'm not aware of
>> WLAN needing to listen to the pd-mapper framework.
> 
> This function is called for both the WWAN and WLAN on this machine, and
> it seems like the modem is typically probed first and around the time
> when I saw the NULL-deref.
> 
> [    8.802728] mhi-pci-generic 0005:01:00.0: mhi_gen_tre - buf_info = ffff800080d75000, offsetof(buf_info->used) = 0x34
> ...
> [    9.980638] ath12k_pci 0004:01:00.0: mhi_gen_tre - buf_info = ffff800081d35000, offsetof(buf_info->used) = 0x34
>   
>> The offset seems to be mapped back to
>> linux/drivers/bus/mhi/host/main.c:1220, I had some extra debug configs
>> enabled so not sure the offset is still valid.
>>
>> 	WARN_ON(buf_info->used);
>> 	buf_info->pre_mapped = info->pre_mapped;
>>
>> This looks like the null pointer would happen if qrtr tried to send
>> before mhi_channel_prepare() is called.
> 
> I didn't look closely at the code, but I can confirm that the offset of
> buf_info->used is indeed 0x34, which could indicate that it's the
> 
> 	buf_info = buf_ring->wp;
> 
> pointer that was NULL.
> 
>> I think we have a patch that might fix this, let me dig it up and send
>> it out.
> 
> Would that patch still help?
> 
> 	https://lore.kernel.org/lkml/20241104-qrtr_mhi-v1-1-79adf7e3bba5@quicinc.com/
>

Yea this is the exact patch I had in mind, didnt realize the patch was 
already sent a while back.

> I naively tried adding a sleep after registering the endpoint, but that
> is at least not sufficient to trigger the NULL-deref.
> 

Looking at the callstack, this is broadcasting a NEW_SERVER notification 
from qrtr_ns. I think you can force this by starting and stopping some 
qmi service with the added sleep. Do you have tqftpserv or diag-router 
in your environment? Those will open qmi services, so starting and 
stopping those will cause the new_server broadcast in qrtr_ns.

> Johan