Re: cdev/devm_* issues (was Re: [TECH TOPIC] Rust for Linux)

[Linus Walleij <linus.walleij@linaro.org>]

On Fri, Jul 9, 2021 at 6:54 PM Wedson Almeida Filho <wedsonaf@google.com> wrote:

> In preparation for writing the abstractions to implement a gpio driver in Rust,
> I was reading through some of the code you describe above.

Nice, bonus review :)

> Unless I'm missing something (very much possible!), this "numbing" seems to not
> be synchronised, that is, there are still race windows when userspace may cause
> UAFs in the kernel.

That's possible.

> For example, gpiochip_remove sets gdev->chip to NULL; gpio_ioctl returns -ENODEV
> if gdev->chip is NULL, which I believe is an instance of what you describe
> above.

Yes.

> However, what ensures that it remains non-null?
(...)
> I see that in functions
> called by gpio_ioctl (e.g., lineinfo_get), gdev->chip is used as if it were
> guaranteed to be valid.
(...)
> Is my reading correct or is there some synchronisation that I'm missing?

No there are definately possible synchronization bugs there.

We probably need a few more
if (!gdev->chip) return -ENODEV;
in some of these callbacks for example.
There are probably also more narrow possible sync bugs.

They are a bit hard to reproduce in practice because people do not
unplug their GPIO devices so much, the one case that is used a bit
would be USB-based GPIO expanders which happens on e.g.
serial dongles (FTDI with additional GPIO is the most common).
These are used in practice for controlling lab boards and stuff
but when people unplug them it is usually part of tearing down an
entire setup so the circumstances are a bit chaotic and subtle
bugs are not noticed.

Yours,
Linus Walleij