Package: mcelog
Version: 128+dfsg-1
The README for mcelog claims:
It also opens /dev/mem to parse the BIOS DMI tables. It is careful
to close the file descriptor and unmap any mappings after using them.
The last part of the second sentence above is incorrect: the entries
mapping persists until closedmi() is called or mcelog exits. This exposes
the problem described in #816621.
I have developed and tested (on both versions 104-1 and 128+dfsg-1)
a simple patch (attached) to correct this problem.
Note, however, Ben Hutchings' comment to #816621 announcing that /dev/mem
is likely to go away soon. That means someone should implement alternative
code for mcelog to look up DMI data through the sysfs interface, preferably
in time for stretch.
Make a private copy of DMI data and munmap() /dev/mem without delay.
It was observed that on some systems mcelog, when running as a daemon,
would cause read failures on /sys/firmware/dmi/entries/*/raw due to
conflicting page cache modes.
To avoid this, only hold on to the memory mapping for the time it takes to
make a private copy of the data.
A better approach might be for mcelog to get the DMI information from
/sys/firmware/dmi/entries/ instead of /dev/mem whenever possible, but
this requires more code.
--- a/dmi.c
+++ b/dmi.c
@@ -55,7 +55,6 @@
} __attribute__((packed));
static struct dmi_entry *entries;
-static int entrieslen;
static int numentries;
static int dmi_length;
static struct dmi_entry **handle_to_entry;
@@ -190,6 +189,9 @@
int opendmi(void)
{
struct anchor *a, *abase;
+ void *ebase;
+ size_t entrieslen;
+ off_t eoffset;
void *p, *q;
int pagesize = getpagesize();
int memfd;
@@ -262,18 +264,18 @@
if (verbose)
printf("DMI tables at %x, %u bytes, %u entries\n",
a->table, a->length, a->numentries);
- corr = a->table - round_down(a->table, pagesize);
- entrieslen = round_up(a->table + a->length, pagesize) -
- round_down(a->table, pagesize);
- entries = mmap(NULL, entrieslen,
- PROT_READ, MAP_SHARED, memfd,
- round_down(a->table, pagesize));
- if (entries == (struct dmi_entry *)-1) {
+ eoffset = round_down(a->table, pagesize);
+ corr = a->table - eoffset;
+ entrieslen = round_up(a->table + a->length, pagesize) - eoffset;
+ ebase = mmap(NULL, entrieslen,
+ PROT_READ, MAP_SHARED, memfd, eoffset);
+ if (ebase == MAP_FAILED) {
Eprintf("Cannot mmap SMBIOS tables at %x", a->table);
- entries = NULL;
goto out_mmap;
}
- entries = (struct dmi_entry *)(((char *)entries) + corr);
+ entries = xalloc(a->length);
+ memcpy(entries, ebase+corr, a->length);
+ munmap(ebase, entrieslen);
numentries = a->numentries;
dmi_length = a->length;
fill_handles();
@@ -624,11 +626,10 @@
{
if (!entries)
return;
- munmap(entries, entrieslen);
- entries = NULL;
FREE(dmi_dimms);
FREE(dmi_arrays);
FREE(dmi_ranges);
FREE(dmi_array_ranges);
FREE(handle_to_entry);
+ FREE(entries);
}
