Re: [PATCH] sh: mm: Fix unprotected access to struct device

From: Geert Uytterhoeven
Date: Fri Apr 20 2018 - 05:59:21 EST


Hi Christoph,

On Fri, Apr 20, 2018 at 10:31 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Wed, Apr 18, 2018 at 03:13:14PM +0200, jacopo mondi wrote:
>> As long as it goes for arch/sh, the only user of dma_alloc_coherent()
>> is platform_resource_setup_memory(), and it has been fixed by this
>> patch.
>
> Great!
>
>> Unfortunately, as Thomas pointed out, there are drivers which calls
>> into this with the wrong 'struct device' as the sh_eth one he had fixed.
>
> Yes, we'll need fixes there. Other DMA ops implementations also look
> at struct device, so they generally are buggy.
>
>> I would then say that as long as it goes for the NULL case, we should be
>> fine now.
>
> Then I'd say skil that part, please.

The major reason for keeping the NULL WARN_ON() checks is to make it
obvious to the developer what is wrong, and fall back to the old behavior.

Without the checks, the kernel will just crash during early startup,
without a clue in the (missing) kernel output, usually leading to a
frustrating bisection experience (if the developer is sufficiently motivated,
at all).

Hence my vote for keeping the checks.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds