netdev - [PATCH] net:qrtr: fix allocator flag of idr_alloc_u32() in qrtr_port

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: <20210326033345.162531-1-ducheng2@gmail.com>
Date:   Fri, 26 Mar 2021 11:33:45 +0800
From:   Du Cheng <ducheng2@...il.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Du Cheng <ducheng2@...il.com>,
        syzbot+3eec59e770685e3dc879@...kaller.appspotmail.com
Subject: [PATCH] net:qrtr: fix allocator flag of idr_alloc_u32() in qrtr_port_assign()

change the allocator flag of idr_alloc_u32 from GFP_ATOMIC to
GFP_KERNEL, as GFP_ATOMIC caused BUG: "using smp_processor_id() in
preemptible" as reported by syzkaller.

Reported-by: syzbot+3eec59e770685e3dc879@...kaller.appspotmail.com
Signed-off-by: Du Cheng <ducheng2@...il.com>
---
Hi David & Jakub,

Although this is a simple fix to make syzkaller happy, I feel that maybe a more
proper fix is to convert qrtr_ports from using IDR to radix_tree (which is in
fact xarray) ? 

I found some previous work done in 2019 by Matthew Wilcox:
https://lore.kernel.org/netdev/20190820223259.22348-1-willy@infradead.org/t/#mcb60ad4c34e35a6183c7353c8a44ceedfcff297d
but that was not merged as of now. My wild guess is that it was probably
in conflicti with the conversion of radix_tree to xarray during 2020, and that
might cause the direct use of xarray in qrtr.c unfavorable.

Shall I proceed with converting qrtr_pors to use radix_tree (or just xarray)?

Regards,
Du Cheng

 net/qrtr/qrtr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index edb6ac17ceca..ee42e1e1d4d4 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -722,17 +722,17 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
 	mutex_lock(&qrtr_port_lock);
 	if (!*port) {
 		min_port = QRTR_MIN_EPH_SOCKET;
-		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
+		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, QRTR_MAX_EPH_SOCKET, GFP_KERNEL);
 		if (!rc)
 			*port = min_port;
 	} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
 		rc = -EACCES;
 	} else if (*port == QRTR_PORT_CTRL) {
 		min_port = 0;
-		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 0, GFP_ATOMIC);
+		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 0, GFP_KERNEL);
 	} else {
 		min_port = *port;
-		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, *port, GFP_ATOMIC);
+		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, *port, GFP_KERNEL);
 		if (!rc)
 			*port = min_port;
 	}
-- 
2.27.0