Differential D40028
linuxkpi: Handle direct-mapped addresses in linux_free_kmem() Authored by markj on May 9 2023, 7:22 PM. Tags None Referenced Files
Subscribers
Details
See the analysis in PR 271333. It is possible for driver code to Fix the problem by making linux_free_kmem() check the address to see PR: 271333 Untested so far.
Diff Detail
Event TimelineComment Actions Two comments:
/usr/img/freebsd.git/sys/amd64/include/param.h:#define INKERNEL(va) (((va) >= DMAP_MIN_ADDRESS && (va) < DMAP_MAX_ADDRESS) \
Comment Actions sys/amd64/compile/GENERIC/machine/param.h:#define INKERNEL(va) (((va) >= DMAP_MIN_ADDRESS && (va) < DMAP_MAX_ADDRESS) \ sys/amd64/compile/GENERIC/machine/vmparam.h:#define DMAP_MAX_ADDRESS KV4ADDR(DMPML4I + NDMPML4E, 0, 0, 0) sys/amd64/compile/GENERIC/assym.inc:#define DMAP_MAX_ADDRESS 0xfffffc0000000000 sys/amd64/include/param.h:#define INKERNEL(va) (((va) >= DMAP_MIN_ADDRESS && (va) < DMAP_MAX_ADDRESS) \ sys/amd64/include/vmparam.h:#define DMAP_MAX_ADDRESS KV4ADDR(DMPML4I + NDMPML4E, 0, 0, 0) sys/arm64/include/vmparam.h:#define DMAP_MAX_ADDRESS (0xffffff0000000000UL) sys/arm64/include/vmparam.h:#define DMAP_MAX_PHYSADDR (dmap_phys_max) sys/cddl/contrib/opensolaris/uts/common/sys/idmap.h:#define IDMAP_MAX_DOOR_RPC (256 * 1024) sys/contrib/openzfs/include/os/freebsd/spl/sys/idmap.h:#define IDMAP_MAX_DOOR_RPC (256 * 1024) sys/dev/hid/hidmap.h:#define HIDMAP_MAX_MAPS 4 sys/powerpc/include/vmparam.h:#define DMAP_MAX_ADDRESS 0xc007ffffffffffffUL sys/powerpc/include/vmparam.h:#define DMAP_MAX_ADDRESS 0xbfffffffUL sys/riscv/include/vmparam.h:#define DMAP_MAX_ADDRESS (0xfffffff000000000UL) sys/riscv/include/vmparam.h:#define DMAP_MAX_PHYSADDR (dmap_phys_max) @kib : Shouldn't DMAP_MAX_ADDRESS and DMAP_MAX_PHYSADDR be inclusive maximums, so that we can cover the full 64-bit range? I see we have both variants in the kernel?
Comment Actions How did you come to that conclusion? On amd64 DMAP_MAX_ADDRESS is KV4ADDR(DMPML4I + NDMPML4E, 0, 0, 0) = 0xfffff80000000000 + NDMPML4E*0x8000000000. On arm64 it is 0xffffff0000000000UL.
That is not equivalent, and would re-introduce the bug. The problem is that we're treating a page mapped in the range [VM_MIN_KERNEL_ADDRESS, VM_MAX_KERNEL_ADDRESS] as being mapped in the range [DMAP_MIN_ADDRESS, DMAP_MAX_ADDRESS].
Indeed, this doesn't compile on !DMAP platforms. Comment Actions @markj : Could you figure out if VM_MAX_KERNEL_ADDRESS is inclusive or exclusive. To me it looks platform dependent. Not that it matters, because I suppose allocations won't happen for one byte anyway .. though ... grep -r VM_MAX_KERNEL_ADDRESS /usr/src/sys/ | grep define Comment Actions It is exclusive. The range of addresses in the kernel map is [VM_MIN_KERNEL_ADDRESS, VM_MAX_KERNEL_ADDRESS).
Comment Actions @markj: How did you come to that conclusion? On amd64 DMAP_MAX_ADDRESS is KV4ADDR(DMPML4I + NDMPML4E, 0, 0, 0) = 0xfffff80000000000 + NDMPML4E*0x8000000000. On arm64 it is 0xffffff0000000000UL. I just grepped and found: /usr/src/sys/amd64/compile/GENERIC/machine/param.h:#define INKERNEL(va) (((va) >= DMAP_MIN_ADDRESS && (va) < DMAP_MAX_ADDRESS) \ powerpc seems inclusive. You see the 0xf's going all the way down. So I immediately thought there is something not clear there. If the value is exclusive you can cover -1ULL ... Is that an issue? --HPS
Comment Actions Indeed, I think powerpc should be fixed.
I don't think it's an issue here, since addr should be page-aligned (I added an assertion to this effect). Comment Actions Change looks good, and the questions raised have been answered. One last thought: Maybe KASSERT() the "order" is in some sane range, so the shift operation is not invalid. Should also be done for allocations. Comment Actions
Done here since existing functions also need updating: https://reviews.freebsd.org/D40398 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||