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
Unknown Object (File)
Mon, Sep 2, 10:02 PM
Unknown Object (File)
Thu, Aug 22, 12:20 PM
Unknown Object (File)
Aug 11 2024, 3:14 PM
Unknown Object (File)
Aug 6 2024, 8:23 AM
Unknown Object (File)
Jul 28 2024, 11:22 PM
Unknown Object (File)
Jul 28 2024, 5:14 PM
Unknown Object (File)
Jul 20 2024, 9:22 PM
Unknown Object (File)
Jul 17 2024, 3:58 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 Not Applicable
Unit
Tests Not Applicable

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
111 ↗(On Diff #21706)

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.