public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation•org>
To: Du Cheng <ducheng2@gmail•com>, Matthew Wilcox <willy@infradead•org>
Cc: "David S. Miller" <davem@davemloft•net>,
	Jakub Kicinski <kuba@kernel•org>,
	netdev@vger•kernel.org, Shuah Khan <skhan@linuxfoundation•org>,
	syzbot+3eec59e770685e3dc879@syzkaller•appspotmail.com
Subject: Re: [PATCH v2] net:qrtr: fix atomic idr allocation in qrtr_port_assign()
Date: Sat, 27 Mar 2021 15:12:14 +0100	[thread overview]
Message-ID: <YF89PtWrs2N5XSgb@kroah.com> (raw)
In-Reply-To: <20210327140702.4916-1-ducheng2@gmail.com>

Adding the xarray maintainer...

On Sat, Mar 27, 2021 at 10:07:02PM +0800, Du Cheng wrote:
> add idr_preload() and idr_preload_end() around idr_alloc_u32(GFP_ATOMIC)
> due to internal use of per_cpu variables, which requires preemption
> disabling/enabling.
> 
> reported as "BUG: "using smp_processor_id() in preemptible" by syzkaller
> 
> Reported-by: syzbot+3eec59e770685e3dc879@syzkaller•appspotmail.com
> Signed-off-by: Du Cheng <ducheng2@gmail•com>
> ---
> changelog
> v1: change to GFP_KERNEL for idr_alloc_u32() but might sleep
> v2: revert to GFP_ATOMIC but add preemption disable/enable protection
> 
>  net/qrtr/qrtr.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> index edb6ac17ceca..6361f169490e 100644
> --- a/net/qrtr/qrtr.c
> +++ b/net/qrtr/qrtr.c
> @@ -722,17 +722,23 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
>  	mutex_lock(&qrtr_port_lock);
>  	if (!*port) {
>  		min_port = QRTR_MIN_EPH_SOCKET;
> +		idr_preload(GFP_ATOMIC);
>  		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> +		idr_preload_end();

This seems "odd" to me.  We are asking idr_alloc_u32() to abide by
GFP_ATOMIC, so why do we need to "preload" it with the same type of
allocation?

Is there something in the idr/radix/xarray code that can't really handle
GFP_ATOMIC during a "normal" idr allocation that is causing this warning
to be hit?  Why is this change the "correct" one?

thanks,

greg k-h

  reply	other threads:[~2021-03-27 14:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-27 14:07 [PATCH v2] net:qrtr: fix atomic idr allocation in qrtr_port_assign() Du Cheng
2021-03-27 14:12 ` Greg Kroah-Hartman [this message]
2021-03-27 14:25   ` Du Cheng
2021-03-27 14:31     ` Greg Kroah-Hartman
2021-03-27 15:51       ` Matthew Wilcox
2021-03-28  6:56         ` Greg Kroah-Hartman
2021-03-28 10:04           ` Matthew Wilcox
2021-03-29 10:55             ` Du Cheng
2021-03-29 11:09               ` Manivannan Sadhasivam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YF89PtWrs2N5XSgb@kroah.com \
    --to=gregkh@linuxfoundation$(echo .)org \
    --cc=davem@davemloft$(echo .)net \
    --cc=ducheng2@gmail$(echo .)com \
    --cc=kuba@kernel$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=skhan@linuxfoundation$(echo .)org \
    --cc=syzbot+3eec59e770685e3dc879@syzkaller$(echo .)appspotmail.com \
    --cc=willy@infradead$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox