Details

Reviewers

emaste
markj
jhb
aokblast

Commits

rG7b2702ee25f5: sys: add safe_read(9)
rG67d61d18bb8d: amd64: extract uiomove_mem() from memrw()

Summary

amd64: extract uiomove_mem() from memrw()


The MD function with MI interface to provide a way to read arbitrary
(canonical) KVA.  amd64 only for now.

Tested by:      aokblast

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Mar 29 2025, 5:18 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 29 2025, 5:18 PM

kib requested review of this revision.Mar 29 2025, 5:18 PM

kib mentioned this in D46193: sys: implement Kernel CFI from clang.Mar 29 2025, 5:20 PM

Slightly different version, avoid the thread-private flag.

emaste added a subscriber: emaste.Jul 30 2025, 11:00 PM

aokblast added a subscriber: aokblast.Sat, May 23, 2:49 PM

kib updated this revision to Diff 178476.Sat, May 23, 3:21 PM

kib retitled this revision from safe_read to sys: add safe_read(9).

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

kib added reviewers: emaste, markj, jhb.

markj added inline comments.Sat, May 23, 5:40 PM

sys/amd64/amd64/trap.c
797 ↗	(On Diff #178476)	Maybe it should be formulated as `if (__predict_false(!usermode && ...`?
sys/sys/systm.h
557 ↗	(On Diff #178476)
559 ↗	(On Diff #178476)

kib marked 2 inline comments as done.Sat, May 23, 5:58 PM

kib added inline comments.

sys/amd64/amd64/trap.c
797 ↗	(On Diff #178476)	I believe that the first check for tf_rip makes the shortest path through the condition: userspace must work hard to fault with this specific %rip, and kernel most likely would not fault on safe_read_read with other conditions false.

Edit the comment.

Complete overhaul. Stop modifying page fault handler at all. Reuse code from mem.c.

markj added inline comments.Tue, May 26, 2:28 PM

sys/amd64/amd64/machdep.c
1868 ↗	(On Diff #178524)	uio_rw is indeed used. uio_segflg too, if we end up calling uiomove().

Fill uio_seg and uio_rw.

markj added inline comments.Tue, May 26, 3:55 PM

sys/amd64/amd64/machdep.c
1868 ↗	(On Diff #178524)	IMO it would be better to explicitly initialize `uio_td`, either to `curthread` or NULL.
sys/amd64/amd64/uio_machdep.c
194	So with this check and the DMAP boundary check above, we can only use safe_read() to access the kernel map and direct map. Is that the intent?

kib marked an inline comment as done.Tue, May 26, 4:34 PM

kib added inline comments.

sys/amd64/amd64/uio_machdep.c
194	I believe it is enough for the use at hand, D46193. More, returning to the 'faulting' implementation, I now think that it opened the access too wide, it should check at least that the address is in KVA. Of course we can return to the 'faulting' way of doing things for other addresses which cannot be handled by pmap_extract(kernel_pmap). I thought that avoiding adding a check to the page fault path is good.