From patchwork Sun Mar 19 21:35:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180607 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A51C0C7618A for ; Sun, 19 Mar 2023 21:35:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230168AbjCSVfv (ORCPT ); Sun, 19 Mar 2023 17:35:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230017AbjCSVfr (ORCPT ); Sun, 19 Mar 2023 17:35:47 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 133CA11676; Sun, 19 Mar 2023 14:35:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=8Q+rY3sEv8O/a1Hte6tCTS4ueqFBvl5vBQjw2THVgEs=; b=xx0XlBhQiPPXqsx0AbY+0td+J6 UUCh/x/XSkabldPx6kMcolXsH9lJT4bbWRKKYJ5m6BAP3OjqG6i2ev8+m/npQA7M2sVZ2dSNNjaYE k2mYrJYMqeRVw2Y+YNiw5hBCiUMSBtoXXp4pe67ys1pGHk6bR4/i62mf8C3Vjn3B1aa/htmoX2FER w5FXR9AFjKyamKJeKlrozTyOyzffiiHmsz1Kzod9+FGwxPw+s19k/VFAm5dZkAxNUG4XM2GC0XIx3 QTSxw8Dt5yPrqUzFiTRdIMuKSGPo2KnwxMJr8K0fkGgaG1oRGAr+lttmlxs14U0cfWqHRK5VwkHhs fq3GKRZQ==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0gp-007Vmr-1P; Sun, 19 Mar 2023 21:35:43 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [PATCH 1/5] module: add sanity check for ELF module section Date: Sun, 19 Mar 2023 14:35:38 -0700 Message-Id: <20230319213542.1790479-2-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319213542.1790479-1-mcgrof@kernel.org> References: <20230319213542.1790479-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: 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 --- kernel/module/main.c | 62 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index cf097ffe6a4a..e1a9dd51c036 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2,6 +2,7 @@ /* * Copyright (C) 2002 Richard Henderson * Copyright (C) 2001 Rusty Russell, 2002, 2010 Rusty Russell IBM. + * Copyright (C) 2023 Luis Chamberlain */ #define INCLUDE_VERMAGIC @@ -1656,6 +1657,7 @@ static int elf_validity_check(struct load_info *info) unsigned int i; Elf_Shdr *shdr, *strhdr; int err; + unsigned int num_mod_secs = 0, mod_idx; if (info->len < sizeof(*(info->hdr))) { pr_err("Invalid ELF header len %lu\n", info->len); @@ -1767,6 +1769,11 @@ static int elf_validity_check(struct load_info *info) i, shdr->sh_type); return err; } + if (strcmp(info->secstrings + shdr->sh_name, + ".gnu.linkonce.this_module") == 0) { + num_mod_secs++; + mod_idx = i; + } if (shdr->sh_flags & SHF_ALLOC) { if (shdr->sh_name >= strhdr->sh_size) { @@ -1779,6 +1786,52 @@ static int elf_validity_check(struct load_info *info) } } + /* + * The ".gnu.linkonce.this_module" ELF section is special. It is + * what modpost uses to refer to __this_module and let's use rely + * on THIS_MODULE to point to &__this_module properly. The kernel's + * modpost declares it on each modules's *.mod.c file. If the struct + * module of the kernel changes a full kernel rebuild is required. + * + * We have a few expectaions for this special section, the following + * code validates all this for us: + * + * o Only one section must exist + * o We expect the kernel to always have to allocate it: SHF_ALLOC + * o The section size must match the kernel's run time's struct module + * size + */ + if (num_mod_secs != 1) { + pr_err("Only one .gnu.linkonce.this_module section must exist.\n"); + goto no_exec; + } + + shdr = &info->sechdrs[mod_idx]; + + /* + * This is already implied on the switch above, however let's be + * pedantic about it. + */ + if (shdr->sh_type == SHT_NOBITS) { + pr_err(".gnu.linkonce.this_module section must have a size set\n"); + goto no_exec; + } + + if (!(shdr->sh_flags & SHF_ALLOC)) { + pr_err(".gnu.linkonce.this_module must occupy memory during process execution\n"); + goto no_exec; + } + + if (shdr->sh_size != sizeof(struct module)) { + pr_err(".gnu.linkonce.this_module section size must match the kernel's built struct module size at run time\n"); + goto no_exec; + } + + info->index.mod = mod_idx; + + /* This is temporary: point mod into copy of data. */ + info->mod = (void *)info->hdr + shdr->sh_offset; + return 0; no_exec: @@ -1925,15 +1978,6 @@ static int setup_load_info(struct load_info *info, int flags) return -ENOEXEC; } - info->index.mod = find_sec(info, ".gnu.linkonce.this_module"); - if (!info->index.mod) { - pr_warn("%s: No module found in object\n", - info->name ?: "(missing .modinfo section or name field)"); - return -ENOEXEC; - } - /* This is temporary: point mod into copy of data. */ - info->mod = (void *)info->hdr + info->sechdrs[info->index.mod].sh_offset; - /* * If we didn't load the .modinfo 'name' field earlier, fall back to * on-disk struct mod 'name' field. From patchwork Sun Mar 19 21:35:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180604 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 837A3C76195 for ; Sun, 19 Mar 2023 21:35:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230137AbjCSVfs (ORCPT ); Sun, 19 Mar 2023 17:35:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55446 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229734AbjCSVfp (ORCPT ); Sun, 19 Mar 2023 17:35:45 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0591111648; Sun, 19 Mar 2023 14:35:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=neDHB/WWg2awqij4jXWtBpsjGCoiwuHAl9xglimyywE=; b=303stu8OCdpqCRilISbltHeAbN LpTSgDaE73qLea6KutAQ3oKdfbr8JPNYVogqTx3IOrthWjMROrxivKnnu1cUVPu9BTvOlIKg2gOEv q9vnUiOcxcaPRFIEiSg2L7Yzc2cdJ9USoj0KkKNxiLbxAZBrs6z9vtHbR3YNUYRsnHIBZmowoRpfK ZonETc28eQOqhVd0MonM8f6CF6ZoZebs6j5WKq9Dxovs7FzAXDEDcDzS4CNaf70R0GsZ4rzobZmfi 64OCq5KvbAUXf079T14gUbGkW3QfwOeUHyKynwIr4BDOyUOgAc9JhaUPEarUTOJYBk+dLAQypb8tW /8JxAy9w==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0gp-007Vmt-1W; Sun, 19 Mar 2023 21:35:43 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [PATCH 2/5] module: add stop-grap sanity check on module memcpy() Date: Sun, 19 Mar 2023 14:35:39 -0700 Message-Id: <20230319213542.1790479-3-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319213542.1790479-1-mcgrof@kernel.org> References: <20230319213542.1790479-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: The integrity of the struct module we load is important, and although our ELF validator already checks that the module section must match struct module, add a stop-gap check before we memcpy() the final minted module. This also makes those inspecting the code what the goal is. While at it, clarify the goal behind updating the sh_addr address. The current comment is pretty misleading. Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index e1a9dd51c036..fbe62d1625bc 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2213,7 +2213,8 @@ static int move_module(struct module *mod, struct load_info *info) { int i; void *ptr; - enum mod_mem_type t; + enum mod_mem_type t = 0; + int ret = -ENOMEM; for_each_mod_mem_type(type) { if (!mod->mem[type].size) { @@ -2249,9 +2250,26 @@ static int move_module(struct module *mod, struct load_info *info) dest = mod->mem[type].base + (shdr->sh_entsize & SH_ENTSIZE_OFFSET_MASK); - if (shdr->sh_type != SHT_NOBITS) + if (shdr->sh_type != SHT_NOBITS) { + /* + * Our ELF checker already validated this, but let's + * be pedantic and make the goal clearer. We actually + * end up copying over all modifications made to the + * userspace copy of the entire struct module. + */ + if (i == info->index.mod && + (WARN_ON_ONCE(shdr->sh_size != sizeof(struct module)))) { + ret = -ENOEXEC; + goto out_enomem; + } memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size); - /* Update sh_addr to point to copy in image. */ + } + /* + * Update the userspace copy's ELF section address to point to + * our newly allocated memory as a pure convenience so that + * users of info can keep taking advantage and using the newly + * minted official memory area. + */ shdr->sh_addr = (unsigned long)dest; pr_debug("\t0x%lx %s\n", (long)shdr->sh_addr, info->secstrings + shdr->sh_name); @@ -2261,7 +2279,7 @@ static int move_module(struct module *mod, struct load_info *info) out_enomem: for (t--; t >= 0; t--) module_memory_free(mod->mem[t].base, t); - return -ENOMEM; + return ret; } static int check_export_symbol_versions(struct module *mod) From patchwork Sun Mar 19 21:35:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180606 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5A51C761A6 for ; Sun, 19 Mar 2023 21:35:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230157AbjCSVfu (ORCPT ); Sun, 19 Mar 2023 17:35:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55484 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229916AbjCSVfr (ORCPT ); Sun, 19 Mar 2023 17:35:47 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0574B113F1; Sun, 19 Mar 2023 14:35:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=HF9NF/yBwymtSuL/hDrmdyOKiRi1h3O7H2O+TxiSARc=; b=ZQQBxuuR02y60vCB+7GBdnQXcX YdbX8jiMZJZuVMw7Rvoc5g1FkMgeWyy8JbIeN348nQo2UHbjPs0Y1Zyx5K7182GpJ5LXFvYHBWL2W 3D/z8MZxcV6eTDd7Lct1y3nMwXGG+/RaKZeP1DbsZsOa7BxfjMS4Z+YveurMORBVdbh0lh1UPJK0m DK2LdFz3gPnrJ8zRyQmLf5VzouZqycTGLDQcWhg3WUgrAgjDUvy9UQeVL9mQWwVo8i98/WIbibUlh 6xsvX9HmyX0vc/asrGqOWfG+OAKffnE1O4OrekexWRkcFlBsREWOut5cuuXDuaB7hoFvvCLrP2bFg K8vhEKcQ==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0gp-007Vmv-1d; Sun, 19 Mar 2023 21:35:43 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [PATCH 3/5] module: move more elf validity checks to elf_validity_check() Date: Sun, 19 Mar 2023 14:35:40 -0700 Message-Id: <20230319213542.1790479-4-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319213542.1790479-1-mcgrof@kernel.org> References: <20230319213542.1790479-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: The symbol and strings section validation currently happen in setup_load_info() but since they are also doing validity checks move this to elf_validity_check(). Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 79 ++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index fbe62d1625bc..84a7f96cf35a 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1658,6 +1658,8 @@ static int elf_validity_check(struct load_info *info) Elf_Shdr *shdr, *strhdr; int err; unsigned int num_mod_secs = 0, mod_idx; + unsigned int num_info_secs = 0, info_idx; + unsigned int num_sym_secs = 0, sym_idx; if (info->len < sizeof(*(info->hdr))) { pr_err("Invalid ELF header len %lu\n", info->len); @@ -1761,6 +1763,8 @@ static int elf_validity_check(struct load_info *info) info->hdr->e_shnum); goto no_exec; } + num_sym_secs++; + sym_idx = i; fallthrough; default: err = validate_section_offset(info, shdr); @@ -1773,6 +1777,10 @@ static int elf_validity_check(struct load_info *info) ".gnu.linkonce.this_module") == 0) { num_mod_secs++; mod_idx = i; + } else if (strcmp(info->secstrings + shdr->sh_name, + ".modinfo") == 0) { + num_info_secs++; + info_idx = i; } if (shdr->sh_flags & SHF_ALLOC) { @@ -1786,6 +1794,27 @@ static int elf_validity_check(struct load_info *info) } } + if (num_info_secs > 1) { + pr_err("Only one .modinfo section must exist.\n"); + goto no_exec; + } else if (num_info_secs == 1) { + /* Try to find a name early so we can log errors with a module name */ + info->index.info = info_idx; + info->name = get_modinfo(info, "name"); + } + + if (num_sym_secs != 1) { + pr_warn("%s: module has no symbols (stripped?)\n", + info->name ?: "(missing .modinfo section or name field)"); + goto no_exec; + } + + /* Sets internal symbols and strings. */ + info->index.sym = sym_idx; + shdr = &info->sechdrs[sym_idx]; + info->index.str = shdr->sh_link; + info->strtab = (char *)info->hdr + info->sechdrs[info->index.str].sh_offset; + /* * The ".gnu.linkonce.this_module" ELF section is special. It is * what modpost uses to refer to __this_module and let's use rely @@ -1802,7 +1831,8 @@ static int elf_validity_check(struct load_info *info) * size */ if (num_mod_secs != 1) { - pr_err("Only one .gnu.linkonce.this_module section must exist.\n"); + pr_err("module %s: Only one .gnu.linkonce.this_module section must exist.\n", + info->name ?: "(missing .modinfo section or name field)"); goto no_exec; } @@ -1813,17 +1843,20 @@ static int elf_validity_check(struct load_info *info) * pedantic about it. */ if (shdr->sh_type == SHT_NOBITS) { - pr_err(".gnu.linkonce.this_module section must have a size set\n"); + pr_err("module %s: .gnu.linkonce.this_module section must have a size set\n", + info->name ?: "(missing .modinfo section or name field)"); goto no_exec; } if (!(shdr->sh_flags & SHF_ALLOC)) { - pr_err(".gnu.linkonce.this_module must occupy memory during process execution\n"); + pr_err("module %s: .gnu.linkonce.this_module must occupy memory during process execution\n", + info->name ?: "(missing .modinfo section or name field)"); goto no_exec; } if (shdr->sh_size != sizeof(struct module)) { - pr_err(".gnu.linkonce.this_module section size must match the kernel's built struct module size at run time\n"); + pr_err("module %s: .gnu.linkonce.this_module section size must match the kernel's built struct module size at run time\n", + info->name ?: "(missing .modinfo section or name field)"); goto no_exec; } @@ -1832,6 +1865,13 @@ static int elf_validity_check(struct load_info *info) /* This is temporary: point mod into copy of data. */ info->mod = (void *)info->hdr + shdr->sh_offset; + /* + * If we didn't load the .modinfo 'name' field earlier, fall back to + * on-disk struct mod 'name' field. + */ + if (!info->name) + info->name = info->mod->name; + return 0; no_exec: @@ -1954,37 +1994,6 @@ static int rewrite_section_headers(struct load_info *info, int flags) */ static int setup_load_info(struct load_info *info, int flags) { - unsigned int i; - - /* Try to find a name early so we can log errors with a module name */ - info->index.info = find_sec(info, ".modinfo"); - if (info->index.info) - info->name = get_modinfo(info, "name"); - - /* Find internal symbols and strings. */ - for (i = 1; i < info->hdr->e_shnum; i++) { - if (info->sechdrs[i].sh_type == SHT_SYMTAB) { - info->index.sym = i; - info->index.str = info->sechdrs[i].sh_link; - info->strtab = (char *)info->hdr - + info->sechdrs[info->index.str].sh_offset; - break; - } - } - - if (info->index.sym == 0) { - pr_warn("%s: module has no symbols (stripped?)\n", - info->name ?: "(missing .modinfo section or name field)"); - return -ENOEXEC; - } - - /* - * If we didn't load the .modinfo 'name' field earlier, fall back to - * on-disk struct mod 'name' field. - */ - if (!info->name) - info->name = info->mod->name; - if (flags & MODULE_INIT_IGNORE_MODVERSIONS) info->index.vers = 0; /* Pretend no __versions section! */ else From patchwork Sun Mar 19 21:35:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180602 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F517C6FD1F for ; Sun, 19 Mar 2023 21:35:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229755AbjCSVfp (ORCPT ); Sun, 19 Mar 2023 17:35:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55438 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229655AbjCSVfp (ORCPT ); Sun, 19 Mar 2023 17:35:45 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F35CC6584; Sun, 19 Mar 2023 14:35:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=8m8F125vs28BNQfwZCFnodLMrX3W0w1T4J8QWupY5ig=; b=GTtlsuGbgsuAp8vwH33WEU2s/U 4y007i5AhtjQCDMMtVpRq/ANNqlTYRm/yBTZa2fHfP+0O6GR957zE8L6ey3wWPLxdqRQFzmlrGxtG KwOXw5mehMbNhyhAnDViSSCSVNfQMuuD1Jxd40ZzYorqEd+4WSrx8fCcM4MVpmU+UiBK3oUJTEKTF C9P8phKz1aHFKqBeYgDPbfQYJrBftrMxsoWNNPpySFQCbznQ8wXwu+v4y+IutVMqcnFTLFGKA6w6y wooGsJZOBEzoGC9NbtzupCQrRV4ugwEqHRLyYMdTOSo7Jd7qO5rX+Bx9o5L4oPhs0obg/nUtxl4+3 34RDozdg==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0gp-007Vmx-1k; Sun, 19 Mar 2023 21:35:43 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [PATCH 4/5] module: merge remnants of setup_load_info() to elf validation Date: Sun, 19 Mar 2023 14:35:41 -0700 Message-Id: <20230319213542.1790479-5-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319213542.1790479-1-mcgrof@kernel.org> References: <20230319213542.1790479-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: The setup_load_info() was actually had ELF validation checks of its own. To later cache useful variables as an secondary step just means looping again over the ELF sections we just validated. We can simply keep tabs of the key sections of interest as we validate the module ELF section in one swoop, so do that and merge the two routines together. Expand a bit on the documentation / intent / goals. Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 60 ++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 84a7f96cf35a..929644d79d38 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1647,12 +1647,26 @@ static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr) } /* - * Sanity checks against invalid binaries, wrong arch, weird elf version. + * Check userspace passed ELF module against our expectations, and cache + * useful variables for further processing as we go. * - * Also do basic validity checks against section offsets and sizes, the + * This does basic validity checks against section offsets and sizes, the * section name string table, and the indices used for it (sh_name). + * + * As a last step, since we're already checking the ELF sections we cache + * useful variables which will be used later for our convenience: + * + * o pointers to section headers + * o cache the modinfo symbol section + * o cache the string symbol section + * o cache the module section + * + * As a last step we set info->mod to the temporary copy of the module in + * info->hdr. The final one will be allocated in move_module(). Any + * modifications we make to our copy of the module will be carried over + * to the final minted module. */ -static int elf_validity_check(struct load_info *info) +static int elf_validity_cache_copy(struct load_info *info, int flags) { unsigned int i; Elf_Shdr *shdr, *strhdr; @@ -1872,6 +1886,13 @@ static int elf_validity_check(struct load_info *info) if (!info->name) info->name = info->mod->name; + if (flags & MODULE_INIT_IGNORE_MODVERSIONS) + info->index.vers = 0; /* Pretend no __versions section! */ + else + info->index.vers = find_sec(info, "__versions"); + + info->index.pcpu = find_pcpusec(info); + return 0; no_exec: @@ -1984,26 +2005,6 @@ static int rewrite_section_headers(struct load_info *info, int flags) return 0; } -/* - * Set up our basic convenience variables (pointers to section headers, - * search for module section index etc), and do some basic section - * verification. - * - * Set info->mod to the temporary copy of the module in info->hdr. The final one - * will be allocated in move_module(). - */ -static int setup_load_info(struct load_info *info, int flags) -{ - if (flags & MODULE_INIT_IGNORE_MODVERSIONS) - info->index.vers = 0; /* Pretend no __versions section! */ - else - info->index.vers = find_sec(info, "__versions"); - - info->index.pcpu = find_pcpusec(info); - - return 0; -} - /* * These calls taint the kernel depending certain module circumstances */ static void module_augment_kernel_taints(struct module *mod, struct load_info *info) @@ -2809,17 +2810,10 @@ static int load_module(struct load_info *info, const char __user *uargs, /* * Do basic sanity checks against the ELF header and - * sections. - */ - err = elf_validity_check(info); - if (err) - goto free_copy; - - /* - * Everything checks out, so set up the section info - * in the info structure. + * sections. Cache useful sections and set the + * info->mod to the userspace passed struct module. */ - err = setup_load_info(info, flags); + err = elf_validity_cache_copy(info, flags); if (err) goto free_copy; From patchwork Sun Mar 19 21:35:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180603 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 48EC6C7618A for ; Sun, 19 Mar 2023 21:35:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230078AbjCSVfr (ORCPT ); Sun, 19 Mar 2023 17:35:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55456 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229824AbjCSVfp (ORCPT ); Sun, 19 Mar 2023 17:35:45 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A2F11164B; Sun, 19 Mar 2023 14:35:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=o7Go1puxe90FGMktwOjmsLknRJd3vVFjG7iQj4FUx+M=; b=XqMEHnA1Jxvrrb245/gV9tQJgM CKTzB8jqsj/Mrc4jQJulJjjvW6eSvvhDRJQbLLthWWGxdgzs8e4f8/HMftBfDxSu3htw9yjyqzbdn MR52AsAZ+FOCvqmkf3us3iGzPp8CH+PEQK+DCAV8CtKbtMXpwUtFC3Q5RqOOhcf6fmJpRuP60BIwR ibr7FJGZ1bxgKq3OkApLjLhfFPS9w9R58snlxbcBxwgpL2XLyrlavSNCIGS39tMMMP4bvuhMAGk2D fCdcpy95A7nX+nt+i6POnJs0gt71a61x82dcm/xEwhub3LSs3OSQbCghkvUjXi9XJIEmxXQC8b/jA MjmDTuVg==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0gp-007Vmz-1r; Sun, 19 Mar 2023 21:35:43 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [PATCH 5/5] module: fold usermode helper kmod into modules directory Date: Sun, 19 Mar 2023 14:35:42 -0700 Message-Id: <20230319213542.1790479-6-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319213542.1790479-1-mcgrof@kernel.org> References: <20230319213542.1790479-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: The kernel/kmod.c is already only built if we enabled modules, so just stuff it under kernel/module/kmod.c and unify the MAINTAINERS file for it. Signed-off-by: Luis Chamberlain --- MAINTAINERS | 13 +++---------- kernel/Makefile | 1 - kernel/module/Makefile | 4 +++- kernel/{ => module}/kmod.c | 0 4 files changed, 6 insertions(+), 12 deletions(-) rename kernel/{ => module}/kmod.c (100%) diff --git a/MAINTAINERS b/MAINTAINERS index 8d5bc223f305..1ca0e26aa9f8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11522,16 +11522,6 @@ F: include/linux/kmemleak.h F: mm/kmemleak.c F: samples/kmemleak/kmemleak-test.c -KMOD KERNEL MODULE LOADER - USERMODE HELPER -M: Luis Chamberlain -L: linux-kernel@vger.kernel.org -L: linux-modules@vger.kernel.org -S: Maintained -F: include/linux/kmod.h -F: kernel/kmod.c -F: lib/test_kmod.c -F: tools/testing/selftests/kmod/ - KMSAN M: Alexander Potapenko R: Marco Elver @@ -14083,8 +14073,11 @@ L: linux-kernel@vger.kernel.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next F: include/linux/module.h +F: include/linux/kmod.h F: kernel/module/ F: scripts/module* +F: lib/test_kmod.c +F: tools/testing/selftests/kmod/ MONOLITHIC POWER SYSTEM PMIC DRIVER M: Saravanan Sekar diff --git a/kernel/Makefile b/kernel/Makefile index 10ef068f598d..3dd4ea433ee9 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -13,7 +13,6 @@ obj-y = fork.o exec_domain.o panic.o \ async.o range.o smpboot.o ucount.o regset.o obj-$(CONFIG_USERMODE_DRIVER) += usermode_driver.o -obj-$(CONFIG_MODULES) += kmod.o obj-$(CONFIG_MULTIUSER) += groups.o ifdef CONFIG_FUNCTION_TRACER diff --git a/kernel/module/Makefile b/kernel/module/Makefile index 948efea81e85..5b1d26b53b8d 100644 --- a/kernel/module/Makefile +++ b/kernel/module/Makefile @@ -7,7 +7,9 @@ # and produce insane amounts of uninteresting coverage. KCOV_INSTRUMENT_module.o := n -obj-y += main.o strict_rwx.o +obj-y += main.o +obj-y += strict_rwx.o +obj-y += kmod.o obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o obj-$(CONFIG_MODULE_SIG) += signing.o obj-$(CONFIG_LIVEPATCH) += livepatch.o diff --git a/kernel/kmod.c b/kernel/module/kmod.c similarity index 100% rename from kernel/kmod.c rename to kernel/module/kmod.c