Bug 206269 - KASAN: missed checks in ioread/write8/16/32_rep
Summary: KASAN: missed checks in ioread/write8/16/32_rep
Status: NEW
Alias: None
Product: Memory Management
Classification: Unclassified
Component: Sanitizers (show other bugs)
Hardware: All Linux
: P1 enhancement
Assignee: MM/Sanitizers virtual assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-21 13:20 UTC by Dmitry Vyukov
Modified: 2020-10-09 09:55 UTC (History)
2 users (show)

See Also:
Kernel Version: ALL
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Dmitry Vyukov 2020-01-21 13:20:07 UTC
ioread/write8/16/32_rep are similar to copy_to/from_user (for KASAN purposes). We miss checks there, we should check that we don't touch bad memory with these functions.
Comment 1 Aleksandr Nogikh 2020-10-08 13:23:30 UTC
Can you please clarify what check is required in this bug? These functions are implemented in C and are instrumented by the compiler. At least this covers the cases of ioread*_rep writing to bad kernel memory and iowrite*_rep reading from bad kernel memory.

There can be (?) situations when ioread*_rep is trying to read from bad MMIO/PMIO memory range or iowrite*_rep is trying to write to bad MMIO/PMIO memory range. But that would require adding checks to inb/outb/ioread*/iowrite*.
Comment 3 Aleksandr Nogikh 2020-10-09 09:29:38 UTC
So we need to detect cases when we read from (or write to) some kernel address instead of an address that is mapped to IO, right?

In this case, I think it also makes sense to instrument ioread8/16/.../iowrite8/..., as essentially they can do the same thing.

Probaly we should check the target IO addresses agains memory ranges allocated by the ioremap/memremap functions, not just the usual KASAN checks.
Comment 4 Dmitry Vyukov 2020-10-09 09:55:23 UTC
Humm... I can't figure out what exactly I meant when filed the issue. Maybe I assume that the access to the kernel buffer also happens in asm and is not visible to KASAN (otherwise why did I mention only _rep versions?). But it seems that it does because the loop part is implemented in C:
https://elixir.bootlin.com/linux/v5.9-rc8/source/include/asm-generic/io.h#L356

Hard to say how much value there is in checking the IO address.
I see at least in some cases the IO address is loaded from some kernel data structure:
https://elixir.bootlin.com/linux/v5.9-rc8/source/drivers/net/ethernet/smsc/smc91x.h#L1109
So potentially the IO address can be bad/uninit/wrong. But I don't know how much potential for such bugs there is in general.

> Probaly we should check the target IO addresses agains memory ranges
> allocated by the ioremap/memremap functions, not just the usual KASAN checks.

Good point. Such check is stricter and does not even need KASAN, and can be enabled with a separate debug config.
It's worth asking maintainers of this machinery re addition/value of such check.

Note You need to log in before you can comment on or make changes to this bug.