Page MenuHomeFreeBSD

[PowerPC] fix panic reading /dev/kmem on !DMAP machines
ClosedPublic

Authored by alfredo on Feb 20 2020, 7:33 PM.
Referenced Files
Unknown Object (File)
Mon, Nov 11, 5:17 AM
Unknown Object (File)
Mon, Nov 11, 12:56 AM
Unknown Object (File)
Oct 21 2024, 8:29 AM
Unknown Object (File)
Oct 17 2024, 12:10 AM
Unknown Object (File)
Oct 13 2024, 11:00 PM
Unknown Object (File)
Sep 30 2024, 2:49 PM
Unknown Object (File)
Sep 26 2024, 2:22 PM
Unknown Object (File)
Sep 25 2024, 4:08 PM

Details

Summary

This fixes /dev/kmem causing panic on machines not using DMAP.
The issue was found while running "lib/libkvm/kvm_geterr_test:kvm_geterr_positive_test_no_error" test case on QEMU VM without Huge Pages support.

Test Plan

Run 'kyua test -k /usr/tests/Kyuafile lib/libkvm/kvm_geterr_test:kvm_geterr_positive_test_no_error' on QEMU powerpc64 VM with and without huge pages

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 29515
Build 27383: arc lint + arc unit

Event Timeline

alfredo edited the summary of this revision. (Show Details)

test case improvement moved to D23783

updated to match behavior of amd64/arm64 when handling an error.

sys/powerpc/powerpc/mem.c
157

Don't you need to break here too (from the for and the while), instead of returning an error?

166

IIRC, you have noticed that kernacc() panics when !hw_direct_map and va was in DMAP range, because there were no vm_map's for DMAP range.
However, read/write access should also be checked when !hw_direct_map.

Instead, we should check if va is outside the DMAP range, or, to match the previous if logic, check if va is inside VM range:
(va >= VM_MIN_KERNEL_ADDRESS) && (va <= virtual_end)

167

A space is needed between ')' and '{'.

sys/powerpc/powerpc/mem.c
157

I decided to not break here because this is a check made prior any read/write, so it's a case of a "hard" error that I would like to return immediatelly.

We can set `error=EFAULT``` and break, then rely on "if (uio->uio_resid != orig_resid)" to return the error correctly. Do you have any preference?

166

Yeah, good reminder. I'll work on that, and it will require more tests.

sys/powerpc/powerpc/mem.c
157

I decided to not break here because this is a check made prior any read/write

Are you sure about that? This part is inside an outer while.
And, BTW, the break you've added below also occurs before the call to uiomove.

Adresses reviewers suggestions.

Nice, it looks good to me.

This revision is now accepted and ready to land.Mar 12 2020, 4:36 PM