Skip to content

Commit 4675282

Browse files
committed
module: add sanity check for ELF module section
The ELF ".gnu.linkonce.this_module" section is special, it is what we use to construct the struct module __this_module, which THIS_MODULE points to. When userspace loads a module we always deal first with a copy of the userspace buffer, and twiddle with the userspace copy's version of the struct module. Eventually we allocate memory to do a memcpy() of that struct module, under the assumption that the module size is right. But we have no validity checks against the size or the requirements for the section. Add some validity checks for the special module section early and while at it, cache the module section index early, so we don't have to do that later. While at it, just move over the assigment of the info->mod to make the code clearer. The validity checker also adds an explicit size check to ensure the module section size matches the kernel's run time size for sizeof(struct module). This should prevent sloppy loads of modules which are built today *without* actually increasing the size of the struct module. A developer today can for example expand the size of struct module, rebuild a directoroy 'make fs/xfs/' for example and then try to insmode the driver there. That module would in effect have an incorrect size. This new size check would put a stop gap against such mistakes. This also makes the entire goal of ".gnu.linkonce.this_module" pretty clear. Before this patch verification of the goal / intent required some Indian Jones whips, torches and cleaning up big old spider webs. Signed-off-by: Luis Chamberlain <[email protected]>
1 parent 419e1a2 commit 4675282

File tree

1 file changed

+53
-9
lines changed

1 file changed

+53
-9
lines changed

kernel/module/main.c

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
/*
33
* Copyright (C) 2002 Richard Henderson
44
* Copyright (C) 2001 Rusty Russell, 2002, 2010 Rusty Russell IBM.
5+
* Copyright (C) 2023 Luis Chamberlain <[email protected]>
56
*/
67

78
#define INCLUDE_VERMAGIC
@@ -1656,6 +1657,7 @@ static int elf_validity_check(struct load_info *info)
16561657
unsigned int i;
16571658
Elf_Shdr *shdr, *strhdr;
16581659
int err;
1660+
unsigned int num_mod_secs = 0, mod_idx;
16591661

16601662
if (info->len < sizeof(*(info->hdr))) {
16611663
pr_err("Invalid ELF header len %lu\n", info->len);
@@ -1767,6 +1769,11 @@ static int elf_validity_check(struct load_info *info)
17671769
i, shdr->sh_type);
17681770
return err;
17691771
}
1772+
if (strcmp(info->secstrings + shdr->sh_name,
1773+
".gnu.linkonce.this_module") == 0) {
1774+
num_mod_secs++;
1775+
mod_idx = i;
1776+
}
17701777

17711778
if (shdr->sh_flags & SHF_ALLOC) {
17721779
if (shdr->sh_name >= strhdr->sh_size) {
@@ -1779,6 +1786,52 @@ static int elf_validity_check(struct load_info *info)
17791786
}
17801787
}
17811788

1789+
/*
1790+
* The ".gnu.linkonce.this_module" ELF section is special. It is
1791+
* what modpost uses to refer to __this_module and let's use rely
1792+
* on THIS_MODULE to point to &__this_module properly. The kernel's
1793+
* modpost declares it on each modules's *.mod.c file. If the struct
1794+
* module of the kernel changes a full kernel rebuild is required.
1795+
*
1796+
* We have a few expectaions for this special section, the following
1797+
* code validates all this for us:
1798+
*
1799+
* o Only one section must exist
1800+
* o We expect the kernel to always have to allocate it: SHF_ALLOC
1801+
* o The section size must match the kernel's run time's struct module
1802+
* size
1803+
*/
1804+
if (num_mod_secs != 1) {
1805+
pr_err("Only one .gnu.linkonce.this_module section must exist.\n");
1806+
goto no_exec;
1807+
}
1808+
1809+
shdr = &info->sechdrs[mod_idx];
1810+
1811+
/*
1812+
* This is already implied on the switch above, however let's be
1813+
* pedantic about it.
1814+
*/
1815+
if (shdr->sh_type == SHT_NOBITS) {
1816+
pr_err(".gnu.linkonce.this_module section must have a size set\n");
1817+
goto no_exec;
1818+
}
1819+
1820+
if (!(shdr->sh_flags & SHF_ALLOC)) {
1821+
pr_err(".gnu.linkonce.this_module must occupy memory during process execution\n");
1822+
goto no_exec;
1823+
}
1824+
1825+
if (shdr->sh_size != sizeof(struct module)) {
1826+
pr_err(".gnu.linkonce.this_module section size must match the kernel's built struct module size at run time\n");
1827+
goto no_exec;
1828+
}
1829+
1830+
info->index.mod = mod_idx;
1831+
1832+
/* This is temporary: point mod into copy of data. */
1833+
info->mod = (void *)info->hdr + shdr->sh_offset;
1834+
17821835
return 0;
17831836

17841837
no_exec:
@@ -1925,15 +1978,6 @@ static int setup_load_info(struct load_info *info, int flags)
19251978
return -ENOEXEC;
19261979
}
19271980

1928-
info->index.mod = find_sec(info, ".gnu.linkonce.this_module");
1929-
if (!info->index.mod) {
1930-
pr_warn("%s: No module found in object\n",
1931-
info->name ?: "(missing .modinfo section or name field)");
1932-
return -ENOEXEC;
1933-
}
1934-
/* This is temporary: point mod into copy of data. */
1935-
info->mod = (void *)info->hdr + info->sechdrs[info->index.mod].sh_offset;
1936-
19371981
/*
19381982
* If we didn't load the .modinfo 'name' field earlier, fall back to
19391983
* on-disk struct mod 'name' field.

0 commit comments

Comments
 (0)