Bug 194974 - Limit /dev/mem access to addresses in phys_avail[] - i.e. actual memory
Summary: Limit /dev/mem access to addresses in phys_avail[] - i.e. actual memory
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Ed Maste
URL:
Keywords:
Depends on:
Blocks: 231027
  Show dependency treegraph
 
Reported: 2014-11-12 22:17 UTC by Ed Maste
Modified: 2019-07-05 18:43 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer 2014-11-12 22:17:35 UTC
Running a mismatched lsof on my ~10.1 desktop resulted in an MCA report and hard hang:

MCA: Bank 1, Status 0xbf80000000200001
MCA: Global Cap 0x0000000000000c09, Status 0x0000000000000005
MCA: Vendor "GenuineIntel", ID 0x306a9, APIC ID 2
MCA: CPU 2 UNCOR PCC unclassified error
MCA: Address 0xfee00080
MCA: Misc 0x86
MCA: Bank 1, Status 0xbf80000000200001
MCA: Global Cap 0x0000000000000c09, Status 0x0000000000000005
MCA: Vendor "GenuineIntel", ID 0x306a9, APIC ID 3
MCA: CPU 3 UNCOR PCC unclassified error
MCA: Address 0xfee00080
MCA: Misc 0x86

Discussed with jhb@, presumably this happens after access to some device register. We should by default limit /dev/mem access to addresses in phys_avail[], but include a sysctl to enable such access for expert debugging.
Comment 1 Larry Rosenman freebsd_committer 2014-11-13 16:21:40 UTC
Let me know if I need to get Vic involved.

Larry Rosenman
Maintainer, sysutils/lsof
Comment 2 Alan Cox freebsd_committer 2015-01-02 17:30:18 UTC
Use dump_avail[] rather than phys_avail[].  Between the time that we first initialize phys_avail[] and finally execute vm_page_startup(), we remove pages from phys_avail[] for various purposes.  Also, dump_avail[] includes pages containing the kernel code and data that are excluded from phys_avail[].  In summary, dump_avail[] more accurately describes the physical memory of the machine.

See the arm version of this code.  It already implements the check against dump_avail[].
Comment 3 John Baldwin freebsd_committer freebsd_triage 2018-10-04 15:59:25 UTC
I do think we want some kind of expert mode to permit kgdb to examine things that aren't memory (e.g. reading registers from a 64-bit BAR that may be above Maxmem).  That said, I would be fine with that mode not being on by default.
Comment 4 Mitchell Horne freebsd_committer 2019-07-03 16:37:54 UTC
This is simple to implement for some architectures like arm, but it might be impractical on others. I tried making the fix on amd64 but it broke acpidump(8), since the acpi tables are excluded from dump_avail[].

I'm not sure if there's an easy solution to that, and there may be other cases of userland utilities that are similarly affected if they rely on reading device memory through /dev/mem.
Comment 5 John Baldwin freebsd_committer freebsd_triage 2019-07-05 18:43:56 UTC
For amd64 you might try using the EFI memory map or SMAP as the source of truth instead of dump_avail (and only fallback to dump_avail as a last resort if either of those isn't present).  You would then perhaps permit access by default to some memory types listed in SMAP/EFI that aren't plain RAM.  I think ACPI tables can be listed in one of those (firmware memory or some such).  Arguably, we should perhaps be including those pages in dump_avail but not phys_avail anyway since it might be nice to have firmware tables in crash dumps.  You'd have to dump_add_page them, but still.