Re: Preemptible idr_alloc() in QRTR code

On Tue, Jan 26, 2021 at 06:36:02PM +0000, Mark Rutland wrote:

On Tue, Jan 26, 2021 at 11:00:05AM -0600, Bjorn Andersson wrote:

quoted

On Tue 26 Jan 10:21 CST 2021, Mark Rutland wrote:

quoted

On Tue, Jan 26, 2021 at 02:58:33PM +0000, Matthew Wilcox wrote:

quoted

On Tue, Jan 26, 2021 at 10:47:34AM +0000, Mark Rutland wrote:

quoted

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:

I sent a patch to fix this last June.  The maintainer seems to be
under the impression that I care an awful lot more about their
code than I do.

https://lore.kernel.org/netdev/20200605120037.17427-1-willy@infradead.org/ (local)

Ah; I hadn't spotted the (glaringly obvious) GFP_ATOMIC abuse, thanks
for the pointer, and sorry for the noise.

I'm afraid this isn't as obvious to me as it is to you. Are you saying
that one must not use GFP_ATOMIC in non-atomic contexts?

That said, glancing at the code I'm puzzled to why it would use
GFP_ATOMIC.

I'm also not entirely sure about the legitimacy of GFP_ATOMIC outside of
atomic contexts -- I couldn't spot any documentation saying that wasn't
legitimate, but Matthew's commit message implies so, and it sticks out
as odd.

It's actually an assumption in the radix tree code.  If you say you
can't be preempted by saying GFP_ATOMIC, it takes you at your word and
does some things which cannot be done in preemptable context.