Page MenuHomeFreeBSD

Don't permit mappings of invalid physical addresses on x86 via /dev/mem.
ClosedPublic

Authored by jhb on Aug 3 2016, 4:55 PM.
Tags
None
Referenced Files
F101908534: D7408.diff
Tue, Nov 5, 11:58 AM
Unknown Object (File)
Sat, Oct 12, 6:50 PM
Unknown Object (File)
Sep 24 2024, 11:23 AM
Unknown Object (File)
Sep 23 2024, 5:15 PM
Unknown Object (File)
Sep 23 2024, 12:30 PM
Unknown Object (File)
Sep 18 2024, 7:30 PM
Unknown Object (File)
Sep 18 2024, 7:16 AM
Unknown Object (File)
Sep 17 2024, 7:41 PM
Subscribers

Details

Summary

Don't permit mappings of invalid physical addresses on x86 via /dev/mem.

While here, add a bounds check on the physical address used with
memrw on i386. (amd64 already has a check)

Test Plan
  • mmap an invalid address via /dev/mem. Triggers a panic in an unpatched kernel on access.
  • I have only tested amd64 so far, need to verify on i386 still.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4699
Build 4753: arc lint + arc unit

Event Timeline

jhb retitled this revision from to Don't permit mappings of invalid physical addresses on x86 via /dev/mem..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added a reviewer: kib.

I think that this may be not completely correct for i386. cpu_maxphyaddr contains raw value from CPUID. On non-PAE systems the result could be that offsets are allowed behind 4G. The result would be non-fatal, but incorrect.

x86/nexus.c has same bug, it seems.

We always set cpu_maxphyaddr. If the cpuid leaf isn't available we set it to either 32 or 36 depending on PAE:

if (cpu_exthigh >= 0x80000008) {
        do_cpuid(0x80000008, regs);
        cpu_maxphyaddr = regs[0] & 0xff;
        cpu_procinfo2 = regs[2];
} else {
        cpu_maxphyaddr = (cpu_feature & CPUID_PAE) != 0 ? 36 : 32;
}
In D7408#154024, @jhb wrote:

We always set cpu_maxphyaddr. If the cpuid leaf isn't available we set it to either 32 or 36 depending on PAE:

I mean that if CPUID leaf is available and typically wider than 32, on !PAE config the offsets > 4G are not representable. They will be wrapped mod 4G, which, as I said above, not fatal, but not correct either.

Hmm, so perhaps the code to set cpu_maxphyaddr should force it to 32 in the i386 && !PAE case?

In D7408#154070, @jhb wrote:

Hmm, so perhaps the code to set cpu_maxphyaddr should force it to 32 in the i386 && !PAE case?

I suspect this might be too limiting overall. If you want to avoid duplicated limiting code repeated several times for i386, perhaps inline function is due in x86_var.h ?

static inline vm_paddr_t
cpu_getmaxusedphyaddr(void)
{
#if defined(__i386__) && !defined(PAE)
   return (0xffffffffU);
#else
   return (1ULL << cpu_maxphyaddr);
#endif
}

I do not think that PAE_TABLES option should result in the full width return.

jhb edited edge metadata.
  • Export maxphyaddr() as x86-wide cpu_getmaxphyaddr().
  • Drop debug printf.
  • Rebase after EFER_NXE fix and /dev/kmem mmap removal.
sys/i386/i386/mem.c
112–116

It was important to check uio_offset and not 'pa' to reject addresses above 4G on i386 'pa' was only 32 bits and would silently truncate on assignment (I originally checked pa). Also, using trunc_page() instead of inlining it seemed more consistent (I can't recall if it mattered for PAE as well)

Oh, and I (finally) tested this on both PAE and !PAE on i386 (PAE testing was stuck on the EFER_NXE bug)

kib edited edge metadata.
This revision is now accepted and ready to land.Oct 26 2016, 7:23 PM
This revision was automatically updated to reflect the committed changes.