Received: by 2002:a17:90a:9307:0:0:0:0 with SMTP id p7csp5143602pjo; Tue, 11 Feb 2020 04:49:40 -0800 (PST) X-Google-Smtp-Source: APXvYqyRq6cOd6xSeP78cYR/xax0IsqPhYCbEI3aINSs12WCPORxMPJ7qEB+3Z2w5pRE19X4aPL5 X-Received: by 2002:a9d:6212:: with SMTP id g18mr5324269otj.187.1581425380052; Tue, 11 Feb 2020 04:49:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1581425380; cv=none; d=google.com; s=arc-20160816; b=scGrYaA2/0cLTCBpjP9McA7ixYvPmmuISm6pbqVShoZu8WPCm4b3GkLd1wsXZqEoYv gay23+4iXN1hyLBZdbKBOBWxMve8KsGLKgSyIq5wgSH1Mbx4I/nQCbQl22QK9/gIH5rg NMtETM+ziQG8aGVk08ocGbAF+e6lTfkx5dvyzJ7nwBwYy0ILC8wY/wbQWltRyGwqBNdp bUkDbolB6a+oh7yroAlI4CW4gRP95s63RCwsfKGF2SvmrKv2zbrCIaf6iG34rfNjV2ZL tkDldrJx1aigFw1vQO9Gr6t4IsZz+2UzGZVUR5RYAOYnQXjrnv1dvOnuikkHsD5n5gGb gcDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=7BowaXXtrGhzbA5yB96wprgUIHsCOmamm03bGQekn+Q=; b=YwZOZOIrz7hMYbMWnkh2NdSsFTdgK85yYPvzbQ5qeywm3yl2uYqjmv1zr63WGuR4F3 3BpYM8fQ1HGS+ly1Pr1yCGSAZN94KA4/Qh7SsvrJ9YvgXV8KPmdS2vtq5x8InhF9yPUo XMBrf4soheruwO2xuE1Xz3ICTGbrRj0MJLaTUaC7xGtFSu+2HvlT9t2F2qooprJpauLD 1+DkStDzkSs/kZg0r0TaPvIWAopU6ieZHE0rizYcD1WyXbfjhIraHxyvhvme1Qx33oNq ciyUfOlfzw0v4u71/2eMQNvhnv6X1VI3cOsKL5Cj5xpNfC8YFKEC/VCWJy6Ce1rfFf34 Kbog== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=2V2oxVmA; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w29si2097905oth.313.2020.02.11.04.49.28; Tue, 11 Feb 2020 04:49:40 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=2V2oxVmA; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728835AbgBKMjj (ORCPT + 99 others); Tue, 11 Feb 2020 07:39:39 -0500 Received: from mail.kernel.org ([198.145.29.99]:54150 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727954AbgBKMji (ORCPT ); Tue, 11 Feb 2020 07:39:38 -0500 Received: from linux-8ccs (p5B2812F9.dip0.t-ipconnect.de [91.40.18.249]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DDBA3206DB; Tue, 11 Feb 2020 12:39:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1581424778; bh=kzLCgKTPkI+N6fyK0lx70rE29ThRwi68n5bZdG+YZWU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=2V2oxVmA3ZU9a3hXs6yXvGpPxs249Jihi0gD1I+npFyX5WLNIBfkEiWnv+t6HTIsq alLzC957QPlWyp7aJSFqZnxFabvuVMly0YqQHOsvF1W2x+m3S8AOba6/o9VdNFMHFa BQnWtgzovTJpLev0wCvkAr5HrszBO0shgMuryLYc= Date: Tue, 11 Feb 2020 13:39:32 +0100 From: Jessica Yu To: Kees Cook Cc: Kristen Carlson Accardi , tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com, arjan@linux.intel.com, rick.p.edgecombe@intel.com, x86@kernel.org, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: Re: [RFC PATCH 10/11] module: Reorder functions Message-ID: <20200211123932.GA29598@linux-8ccs> References: <20200205223950.1212394-1-kristen@linux.intel.com> <20200205223950.1212394-11-kristen@linux.intel.com> <202002060440.E7ED91B@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <202002060440.E7ED91B@keescook> X-OS: Linux linux-8ccs 4.12.14-lp150.12.61-default x86_64 User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +++ Kees Cook [06/02/20 04:41 -0800]: >On Wed, Feb 05, 2020 at 02:39:49PM -0800, Kristen Carlson Accardi wrote: >> If a module has functions split out into separate text sections >> (i.e. compiled with the -ffunction-sections flag), reorder the >> functions to provide some code diversification to modules. >> >> Signed-off-by: Kristen Carlson Accardi > >Probably a good idea to add Jessica to CC in next version: > Jessica Yu Thanks :) >Reviewed-by: Kees Cook > >-Kees > >> --- >> kernel/module.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 85 insertions(+) >> >> diff --git a/kernel/module.c b/kernel/module.c >> index b56f3224b161..231563e95e61 100644 >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -53,6 +53,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> #include >> #include "module-internal.h" >> >> @@ -3245,6 +3247,87 @@ static int find_module_sections(struct module *mod, struct load_info *info) >> return 0; >> } >> >> +/* >> + * shuffle_text_list() >> + * Use a Fisher Yates algorithm to shuffle a list of text sections. >> + */ >> +static void shuffle_text_list(Elf_Shdr **list, int size) >> +{ >> + int i; >> + unsigned int j; >> + Elf_Shdr *temp; >> + >> + for (i = size - 1; i > 0; i--) { >> + /* >> + * TBD - seed. We need to be able to use a known >> + * seed so that we can non-randomly randomize for >> + * debugging. >> + */ >> + >> + // pick a random index from 0 to i >> + get_random_bytes(&j, sizeof(j)); >> + j = j % (i + 1); >> + >> + temp = list[i]; >> + list[i] = list[j]; >> + list[j] = temp; >> + } >> +} >> + >> +/* >> + * randomize_text() >> + * Look through the core section looking for executable code sections. >> + * Store sections in an array and then shuffle the sections >> + * to reorder the functions. >> + */ >> +static void randomize_text(struct module *mod, struct load_info *info) >> +{ >> + int i; >> + int num_text_sections = 0; >> + Elf_Shdr **text_list; >> + int size = 0; >> + int max_sections = info->hdr->e_shnum; >> + unsigned int sec = find_sec(info, ".text"); >> + >> + if (!IS_ENABLED(CONFIG_FG_KASLR) || !kaslr_enabled()) >> + return; Maybe put this conditional before the call to randomize_text() in move_module()? Just for code readability reasons (i.e., we only call randomize_text() when CONFIG_FG_KASLR || kaslr_enabled()). But this is just a matter of preference. >> + >> + if (sec == 0) >> + return; >> + >> + text_list = kmalloc_array(max_sections, sizeof(*text_list), GFP_KERNEL); >> + if (text_list == NULL) >> + return; >> + >> + for (i = 0; i < max_sections; i++) { >> + Elf_Shdr *shdr = &info->sechdrs[i]; >> + const char *sname = info->secstrings + shdr->sh_name; >> + >> + if (!(shdr->sh_flags & SHF_ALLOC) || >> + !(shdr->sh_flags & SHF_EXECINSTR) || >> + strstarts(sname, ".init")) >> + continue; >> + >> + text_list[num_text_sections] = shdr; >> + num_text_sections++; >> + } >> + >> + shuffle_text_list(text_list, num_text_sections); >> + >> + for (i = 0; i < num_text_sections; i++) { >> + Elf_Shdr *shdr = text_list[i]; >> + unsigned int infosec; >> + const char *sname; >> + >> + sname = info->secstrings + shdr->sh_name; sname doesn't appear to be used after this (?) >> + infosec = shdr->sh_info; >> + >> + shdr->sh_entsize = get_offset(mod, &size, shdr, infosec); get_offset() expects a section index as the last argument, but sh_info is 0 for text sections (SHT_PROGBITS). It's really only used in arch_mod_section_prepend() though, which is only defined for parisc. We could perhaps save the section index in sh_info in the previous for loop (hacky, I have to double check if this is actually safe, but it seems sh_info only has signficance for SHT_REL* and SHT_SYMTAB sections). >> + } >> + >> + kfree(text_list); >> +} >> + >> static int move_module(struct module *mod, struct load_info *info) >> { >> int i; >> @@ -3282,6 +3365,8 @@ static int move_module(struct module *mod, struct load_info *info) >> } else >> mod->init_layout.base = NULL; >> >> + randomize_text(mod, info); Hm, I wonder if we should not incorporate the randomize_text() logic into layout_sections() instead of move_module(), since logically speaking layout_sections() determines all the section offsets and stores them in sh_entsize, whereas move_module() does all the allocations and copying to final destination. Just a thought. Thanks! Jessica