[PATCH] modpost: detect unterminated device id lists



On Wed, Sep 12, 2007 at 02:53:56PM -0700, Greg KH wrote:
On Wed, Sep 12, 2007 at 03:48:49PM +0400, Alexey Dobriyan wrote:
On 9/12/07, Jeff Garzik <jeff@xxxxxxxxxx> wrote:
Kees Cook wrote:
This patch against 2.6.23-rc6 fixes a couple drivers that do not
correctly terminate their pci_device_id lists. This results in garbage
being spewed into modules.pcimap when the module happens to not have
28 NULL bytes following the table, and/or the last PCI ID is actually
truncated from the table when calculating the modules.alias PCI aliases,
cause those unfortunate device IDs to not auto-load.

Signed-off-by: Kees Cook <kees@xxxxxxxxxx>

ACK

I mut say, non-terminated PCI ids lists are constant PITA. There should be
a way to a) put it in macro[1], so that terminator automatically added, and
b) still allow #ifdef inside table like, e.g. 8139too does.

[1] or not macro, because #ifdef inside macros aren't allowed.

If you know of a way to do this in an easier manner, patches are always
gladly accepted :)

This patch against 2.6.23-rc6 will cause modpost to fail if any device
id lists are incorrectly terminated, after reporting the offender.

Signed-off-by: Kees Cook <kees@xxxxxxxxxx>
---
linux-2.6.23-rc6/scripts/mod/file2alias.c | 39 ++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 7 deletions(-)
---
diff -urp -x '*.o' linux-2.6.23-rc6~/scripts/mod/file2alias.c linux-2.6.23-rc6/scripts/mod/file2alias.c
--- linux-2.6.23-rc6~/scripts/mod/file2alias.c 2007-09-11 23:17:49.000000000 -0700
+++ linux-2.6.23-rc6/scripts/mod/file2alias.c 2007-09-12 17:41:30.000000000 -0700
@@ -55,10 +55,13 @@ do {
* Check that sizeof(device_id type) are consistent with size of section
* in .o file. If in-consistent then userspace and kernel does not agree
* on actual size which is a bug.
+ * Also verify that the final entry in the table is all zeros.
**/
-static void device_id_size_check(const char *modname, const char *device_id,
- unsigned long size, unsigned long id_size)
+static void device_id_check(const char *modname, const char *device_id,
+ unsigned long size, unsigned long id_size,
+ void *symval)
{
+ int i;
if (size % id_size || size < id_size) {
fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
"of the size of section __mod_%s_device_table=%lu.\n"
@@ -66,6 +69,18 @@ static void device_id_size_check(const c
"in mod_devicetable.h\n",
modname, device_id, id_size, device_id, size, device_id);
}
+ /* Verify last one is a terminator */
+ for (i = 0; i < id_size; i++ ) {
+ if ( *(uint8_t*)(symval+size-id_size+i) ) {
+ fprintf(stderr,"%s: struct %s_device_id is %lu bytes. The last of %lu is:\n", modname, device_id, id_size, size / id_size);
+ for (i = 0; i < id_size; i++ ) {
+ fprintf(stderr,"0x%02x ", *(uint8_t*)(symval+size-id_size+i) );
+ }
+ fprintf(stderr,"\n");
+ fatal("%s: struct %s_device_id is not terminated "
+ "with a NULL entry!\n", modname, device_id);
+ }
+ }
}

/* USB is special because the bcdDevice can be matched against a numeric range */
@@ -168,7 +183,7 @@ static void do_usb_table(void *symval, u
unsigned int i;
const unsigned long id_size = sizeof(struct usb_device_id);

- device_id_size_check(mod->name, "usb", size, id_size);
+ device_id_check(mod->name, "usb", size, id_size, symval);

/* Leave last one: it's the terminator. */
size -= id_size;
@@ -505,7 +520,7 @@ static void do_table(void *symval, unsig
char alias[500];
int (*do_entry)(const char *, void *entry, char *alias) = function;

- device_id_size_check(mod->name, device_id, size, id_size);
+ device_id_check(mod->name, device_id, size, id_size, symval);
/* Leave last one: it's the terminator. */
size -= id_size;

@@ -527,14 +542,22 @@ void handle_moddevtable(struct module *m
Elf_Sym *sym, const char *symname)
{
void *symval;
+ char *zeros = NULL;

/* We're looking for a section relative symbol */
if (!sym->st_shndx || sym->st_shndx >= info->hdr->e_shnum)
return;

- symval = (void *)info->hdr
- + info->sechdrs[sym->st_shndx].sh_offset
- + sym->st_value;
+ /* Handle all-NULL symbols allocated into .bss */
+ if (info->sechdrs[sym->st_shndx].sh_type & SHT_NOBITS) {
+ zeros = calloc(1, sym->st_size);
+ symval = zeros;
+ }
+ else {
+ symval = (void *)info->hdr
+ + info->sechdrs[sym->st_shndx].sh_offset
+ + sym->st_value;
+ }

if (sym_is(symname, "__mod_pci_device_table"))
do_table(symval, sym->st_size,
@@ -599,6 +622,8 @@ void handle_moddevtable(struct module *m
do_table(symval, sym->st_size,
sizeof(struct parisc_device_id), "parisc",
do_parisc_entry, mod);
+
+ if (zeros) free(zeros);
}

/* Now add out buffered information to the generated C source */


--
Kees Cook
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: [PATCH] modpost: detect unterminated device id lists
    ... correctly terminate their pci_device_id lists. ... 28 NULL bytes following the table, and/or the last PCI ID is actually ... non-terminated PCI ids lists are constant PITA. ... or not macro, because #ifdef inside macros aren't allowed. ...
    (Linux-Kernel)
  • Re: [PATCH] EDAC: core EDAC support code
    ... I'm looking at the EDAC sysfs interface. ... the following files contain whitespace-delimited lists: ... PCI vendor-ID:device-ID series list. ... indicate that the memory type it represents is supported. ...
    (Linux-Kernel)
  • Re: expandable test if token-lists consist of the same set of tokens?
    ... \detokenize-ation of the two lists are the same, ... Check if it is possible to strip off surrounding braces (a ... I think you can test this by comparing \detokenize ... Otherwise terminate with \@secondoftwo. ...
    (comp.text.tex)
  • Re: PCI Resource Detection problem in Windows XP
    ... one of the lists does not include the memory mapped io. ... We have a problem in PCI driver development in Windows XP SP2. ... problem is that windows is not assigning memory mapped IO address to ... But windows is showing only the IRQ assigned in the resource tab from ...
    (microsoft.public.development.device.drivers)
  • Re: expandable test if token-lists consist of the same set of tokens?
    ... In case was enclosed into braces, ... terminate with false. ... And with both lists you can remove braces from the "first argument". ... I want an undefined-error if it comes to the "attempt" of ...
    (comp.text.tex)