Re: [PATCH v3 3/4] lib: logic_pio: Reject accesses to unregistered CPU MMIO regions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




To me, it's not good that these hwmon, watchdog, gpio, etc drivers
make unconstrained accesses to 0x2e and 0x4e ports (ignoring the
request_muxed_region() call).

The issue I see is that on an arm, IO space for some other device may
be mapped in this region, so it would not be right for these drivers
to access those same regions.

Yes, but then there _could_ be some arm or arm64 device supporting one
of those chips,
so we can not just add something like "depends on !(ARM || ARM64)".

This looks like what has been added for PPC in commmit 746cdfbf01c0.

However, agreed, it's not a good approach.


Is there any other platform check which can be made to ensure that
accesses these super-IO ports is appropriate?


Not that I know of. It would make some sense to provide API functions
for Super-IO accesses, but that would be a lot of work, and I guess
it isn't really valuable enough for anyone to pick up and do.

Normally, if you have such a system, the respective drivers should not be
built. After all, this isn't the only instance where drivers
unconditionally
access some io region, no matter if the underlying hardware exists or not.
The only real defense against that is to not build those drivers into
a given kernel.

If we're going to support a multi-plaform kernel for a given arch, then we
can't always avoid it.

It seems that the only solution on the table now is to discard these IO port
accesses on arm64 when the IO port are not mapped.

Hmm, how are you going to achieve that? I'm not sure we can guarantee a
synchronous abort, so I'd be nervous about anything that tries to handle
the exception after making the unmapped access.


Patches 2+3 in this series are the attempt to solve it.

The solution is not in handling an abort generated in accessing the unmapped region, but rather avoiding any potential abort and just discard the access if the region is unmapped. This is done in the IO port accessors, by checking if a PCI host has registered a region in "logical PIO" space.

For more info on logical PIO space, see commits 031e3601869c ("lib: Add generic PIO mapping method") and 5745392e0c2b ("PCI: Apply the new generic I/O management on PCI IO hosts").

In these patches, I add a check in memory-mapped IO port region accesses to ensure that a PCI host has registered a region, i.e. IO ports are mapped.

With these patches, we now get the following on an arm64 system with no PCI host:

root@(none)$ insmod hwmon/f71882fg.ko
[   30.950909] LOGIC PIO: PIO entry token 0x2e invalid
[   30.955795] LOGIC PIO: PIO entry token 0x2e invalid
[   30.960664] LOGIC PIO: PIO entry token 0x2e invalid
[   30.965531] LOGIC PIO: PIO entry token 0x2f invalid
[   30.970398] LOGIC PIO: PIO entry token 0x2e invalid
[   30.975264] LOGIC PIO: PIO entry token 0x2f invalid
[   30.980131] LOGIC PIO: PIO entry token 0x2e invalid
[   30.984997] LOGIC PIO: PIO entry token 0x4e invalid
[   30.989864] LOGIC PIO: PIO entry token 0x4e invalid
[   30.994730] LOGIC PIO: PIO entry token 0x4e invalid
[   30.999596] LOGIC PIO: PIO entry token 0x4f invalid
[   31.004463] LOGIC PIO: PIO entry token 0x4e invalid
[   31.009329] LOGIC PIO: PIO entry token 0x4f invalid
[   31.014195] LOGIC PIO: PIO entry token 0x4e invalid
insmod: can't insert 'hwmon/f71882fg.ko': No such device
root@(none)$

Note: I've removed a WARN_ONCE from the patches for this experiment. Maybe I'll permanently remove.

Without the patches:

root@(none)$ insmod hwmon/f71882fg.ko
[ 82.001270] Unable to handle kernel paging request at virtual address ffff7dfffee0002e
[   82.009194] Mem abort info:
[   82.011980]   ESR = 0x96000046
[   82.015025]   Exception class = DABT (current EL), IL = 32 bits
[   82.020934]   SET = 0, FnV = 0
[   82.023978]   EA = 0, S1PTW = 0
[   82.027108] Data abort info:
[   82.029975]   ISV = 0, ISS = 0x00000046
[   82.033800]   CM = 0, WnR = 1
[ 82.036757] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____) [ 82.043620] [ffff7dfffee0002e] pgd=000000000141c003, pud=000000000141d003, pmd=0000000000000000
[   82.052309] Internal error: Oops: 96000046 [#1] PREEMPT SMP
[   82.057867] Modules linked in: f71882fg(+)
[ 82.061953] CPU: 10 PID: 2731 Comm: insmod Not tainted 5.1.0-rc1-00003-g938306ea0c86-dirty #230 [ 82.070637] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018
[   82.079754] pstate: 80000005 (Nzcv daif -PAN -UAO)
[   82.084538] pc : logic_outb+0x54/0xb8
[   82.088188] lr : f71882fg_find+0x64/0x390 [f71882fg]
[   82.093139] sp : ffff000026a1baa0
[   82.096439] x29: ffff000026a1baa0 x28: ffff000008b98b10
[   82.101738] x27: ffff000026a1bdf0 x26: 0000000000000100
[   82.107037] x25: ffff801fb5391530 x24: ffff000011420000
[   82.112335] x23: ffff801fb68a3700 x22: ffff000011291000
[   82.117634] x21: 000000000000002e x20: 0000000000000087
[   82.122932] x19: ffff000026a1bb44 x18: ffffffffffffffff
[   82.128230] x17: 0000000000000000 x16: 0000000000000000
[   82.133528] x15: ffff00001127d6c8 x14: 0000000000000000
[   82.138826] x13: 0000000000000000 x12: 0000000000000000
[   82.144124] x11: ffff801ffbf92c80 x10: 0000801fead0f000
[   82.149422] x9 : 0000000000000000 x8 : ffff841fa6403680
[   82.154721] x7 : 0000000000000000 x6 : 000000000000003f
[   82.160019] x5 : ffff000011291360 x4 : 0000000000000000
[   82.165318] x3 : 0000000000ffbffe x2 : eed92a7132399200
[   82.170616] x1 : ffff7dfffee0002e x0 : ffff7dfffee00000
[   82.175915] Process insmod (pid: 2731, stack limit = 0x(____ptrval____))
[   82.182601] Call trace:
[   82.185035]  logic_outb+0x54/0xb8
[   82.188338]  f71882fg_find+0x64/0x390 [f71882fg]
[   82.192943]  f71882fg_init+0x38/0xc70 [f71882fg]
[   82.197548]  do_one_initcall+0x5c/0x198
[   82.201372]  do_init_module+0x54/0x1b0
[   82.205107]  load_module+0x1dc4/0x2158
[   82.208843]  __se_sys_init_module+0x14c/0x1e8
[   82.213186]  __arm64_sys_init_module+0x18/0x20
[   82.217618]  el0_svc_common+0x5c/0x100
[   82.221353]  el0_svc_handler+0x2c/0x80
[   82.225089]  el0_svc+0x8/0xc
[   82.227957] Code: d2bfdc00 f2cfbfe0 f2ffffe0 8b000021 (39000034)
[   82.234038] ---[ end trace 57421eeb25dcc011 ]---
Segmentation fault
root@(none)$

@Guenter, sorry for citing a hwmon driver again as triggering a crash...

John


Will

.






[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux