linux-kernel - Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 21 Mar 2016 19:33:04 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Shawn Lin <shawn.lin@...k-chips.com>
Cc:	Mark Brown <broonie@...nel.org>, linux-spi@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>
Subject: Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE_DEFER

Shawn,

On Mon, Mar 21, 2016 at 7:03 PM, Shawn Lin <shawn.lin@...k-chips.com> wrote:
>> ...but, looking at this, presumably before landing any patch that made
>> dma_request_slave_channel() return -EPROBE_DEFER you'd need to modify
>> _all_ users of dma_request_slave_channel to handle error pointers
>> being returned.  Right now dma_request_slave_channel() says it returns
>> a pointer to a channel or NULL and the function explicitly avoids
>> returning any errors.  That might be possible, but it's a big
>> change...
>
>
> At first glance, it's a big change, but maybe not really.
> Almost all of them use the templet like:
> ch = dma_request_slave_channel
> if (!ch)
>         balabala....
>
> It's same for all the non-null return pointer/non-zero value ?
>
> So from my view, we can safely change dma_request_slave_channel,
> and leave the caller here. I presumably the respective
> drivers will graduately migrate to check the return value with
> EPROBE_DEFER if they do care this issue. Otherwise, we believe
> they don't suffer the changes we make, just as what they did in the
> past. Does that make sense?

...but if you return ERR_PTR(-EPROBE_DEFER) and don't change existing
callers, then existing callers will think you've returned a valid
pointer when you really returned an error pointer.  They'll pass this
error pointer around like it's a valid "struct dma_chan", won't then?

Actually, could your code just call
dma_request_slave_channel_reason().  Oh, looks like that's exactly
what you want.  See commit 0ad7c00057dc ("dma: add channel request API
that supports deferred probe").  Oh, but I'm looking at 4.4.  Looking
at linuxnext, it looks like this got renamed to dma_request_chan().
...so you need to use that, no?

Strange, but on 4.4 there was some extra code in
dma_request_slave_channel() that wasn't in
dma_request_slave_channel_reason().  ...but looks like that all got
cleaned up in the same CL that added the new name.

-Doug