Page MenuHomeFreeBSD

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

Authored by alfredo on Feb 20 2020, 7:33 PM.
Referenced Files
F87081221: D23776.diff
Sat, Jun 29, 2:07 AM
Unknown Object (File)
Tue, Jun 18, 10:21 PM
Unknown Object (File)
Sat, Jun 15, 8:56 AM
Unknown Object (File)
Sat, Jun 15, 5:06 AM
Unknown Object (File)
Thu, Jun 13, 9:30 PM
Unknown Object (File)
Tue, Jun 11, 8:29 PM
Unknown Object (File)
May 17 2024, 6:48 PM
Unknown Object (File)
Apr 29 2024, 11:03 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