Re: [PATCH] net:qrtr: fix allocator flag of idr_alloc_u32() in qrtr_port_assign()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Mar 26, 2021 at 11:33:45AM +0800, Du Cheng wrote:
> 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@syzkaller.appspotmail.com
> Signed-off-by: Du Cheng <ducheng2@gmail.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)?

Try it and see.  But how would that resolve this issue?  Those other
structures would also need to allocate memory at this point in time and
you need to tell it if it can sleep or not.

> 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);

Are you sure that you can sleep in this code path?

thanks,

greg k-h