From patchwork Wed Oct 13 17:57:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 12556503 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC502C433EF for ; Wed, 13 Oct 2021 17:57:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 986A060174 for ; Wed, 13 Oct 2021 17:57:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238329AbhJMR77 (ORCPT ); Wed, 13 Oct 2021 13:59:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42612 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238310AbhJMR75 (ORCPT ); Wed, 13 Oct 2021 13:59:57 -0400 Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A2CBC061746 for ; Wed, 13 Oct 2021 10:57:53 -0700 (PDT) Received: by mail-pf1-x429.google.com with SMTP id y7so3165521pfg.8 for ; Wed, 13 Oct 2021 10:57:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Kcjz/jZcxE+mB3F2AkM0clWB2nKEsktmPdDaqY7pNLM=; b=L75cOX6+P2Yg2PCtJBc3rDxsjCw7N/NVtlafkSSqqUGzYVqCBBPEt9zMKsJ+BBeqTQ XSfl2ZFSn+uvEQ2K00+x27I4dsgfBWLotrBBXGLMS7tKuUf8Tj6WaQSomgkFtKyLA4W2 lCeFO9tpk6CbOVBc2TLZDwUMCfoxm5xa5u7L0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Kcjz/jZcxE+mB3F2AkM0clWB2nKEsktmPdDaqY7pNLM=; b=Lo+Frj9CYGXpMDGKgZIvlMDOT188LzsfJSw7JDR4eiMZ+IoqaQPaZYsRLUr4bp9C+L HEEQxjon1iy12cDIIuATPVvqEfj8Y98AxYoNHXSUhONJOYJ/s5rJhx9kiQYaoxeMSr4t 9/1hQHhkggh9RWp+lUQLMD69Ur0Qs0UD3iqIa31cPFkWKesHVpfJIjIsPO3XqHrq0Lhp OToI2aY36VRpRtK3V193eOFlnI6diR7JE/32yIxSxHPZoYWuLGivdW2XWv0nShP6zmRq oPIDa3Uuf0N9eooyuWOI7M8qSUUqCotcWNjiJ1gp+TYmhjTQciwMaRII05Nj5NjuCHRh pMWA== X-Gm-Message-State: AOAM530JEQQ1mabSzaIT9a3GybCdsu++iu8QvhXu2oZhBdhjhBBFazLY dRltkzrxbcWc8oe20I/u80U1tg== X-Google-Smtp-Source: ABdhPJx3hCeP0/qXzBriQG9RAyOIBhOSahtHSc2nigzi8GBlJyOW45fh2MS5OSF1B4QStfEMj11pPA== X-Received: by 2002:a63:2acc:: with SMTP id q195mr450989pgq.45.1634147872623; Wed, 13 Oct 2021 10:57:52 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id d9sm162132pgn.64.2021.10.13.10.57.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Oct 2021 10:57:52 -0700 (PDT) From: Kees Cook To: Borislav Petkov Cc: Kees Cook , Alexander Lobakin , Josh Poimboeuf , "H. Peter Anvin" , Kristen Carlson Accardi , Tony Luck , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Arnd Bergmann , Joerg Roedel , Arvind Sankar , Jing Yangyang , Abaci Robot , Jiapeng Chong , Nathan Chancellor , Nick Desaulniers , Vincenzo Frascino , Andrey Konovalov , Miroslav Benes , "H. Nikolaus Schaller" , Fangrui Song , linux-kernel@vger.kernel.org, x86@kernel.org, linux-arch@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH 3/4] x86/boot/compressed: Avoid duplicate malloc() implementations Date: Wed, 13 Oct 2021 10:57:41 -0700 Message-Id: <20211013175742.1197608-4-keescook@chromium.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211013175742.1197608-1-keescook@chromium.org> References: <20211013175742.1197608-1-keescook@chromium.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4454; h=from:subject; bh=hYF77CNx8NXydRq0Sh94JwlUksK5cSbvI41FvGg5UbQ=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBhZx4VrL7HD4WV4zwnJZdT6sN2jDPwltje0/KQButA e0RjcEKJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCYWceFQAKCRCJcvTf3G3AJjoGD/ sGuSJ5GjwOngN6Kzcn20C3dlHYxuKIeUYkhzF7JVQ1n9+cPfhqDVzDkbO+NJHYfvK7zrlGgsa6wtI2 fi7EeRT/FVXdNE356vGVAKnwm8+JqAf8+fMAZCnQAtpMjwF6Wgs9uBhPvRxnKb2AKTXjWqEiwhfTBS QabEnkh5Xo6Dy2QFIJVGPob91BrYShwcIClCE2qTZiAIGbKxS4ia3bHtBPTDXVLNgutIo9Lemw9Htd 70YgUsSCEj0YBclhomTZ4ZMh/hN6yNgXDVo8vnUGrLqtWKsrqAec1NSe11qDqnG2ih+vcoYkivRemG c6TCd9u6BDGU8EIATYPTf577U7csmm4Mdfi29fsUhG58Fg0z9Os2/cZd4LKvAJFZmDmyIQvrnw9Ymt jHGrMQz2/qdikk4WfndV7Pw+tJ7c68L2hHXynezzXebe1LdW2fcuVaeeME1cyF0kjZ9x1lIBw3MBEv Q+OqsZCZ0mSjrL2zFAovH6+vnhA5M1wv34ML2bYsgn/kDHg7SgmIEfzCvi/0p4rk9guAkF+VSMhNQn s/AEh3Knh2JP4VvkKq4gR1of3+iCI8cr5aPhhwppSOvqqJdjozgCHojH4ZZG/m1yiprZBQBWEDVT1n Ax/1g3/3igDo4LbXRABYBefMmZOEFhGay99YNxHgk0tkgX2EXq8bucbu/1Hg== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Precedence: bulk List-ID: X-Mailing-List: linux-hardening@vger.kernel.org The early malloc() and free() implementation in include/linux/decompress/mm.h (which is also included by the static decompressors) is static. This is fine when the only thing interested in using malloc() is the decompression code, but the x86 early boot environment may use malloc() in a couple places, leading to a potential collision when the static copies of the available memory region ("malloc_ptr") gets reset to the global "free_mem_ptr" value. As it happened, the existing usage pattern was accidentally safe because each user did 1 malloc() and 1 free() before returning and were not nested: extract_kernel() (misc.c) choose_random_location() (kaslr.c) mem_avoid_init() handle_mem_options() malloc() ... free() ... parse_elf() (misc.c) malloc() ... free() Once the future FGKASLR series is added, however, it will insert additional malloc() calls local to fgkaslr.c in the middle of parse_elf()'s malloc()/free() pair: parse_elf() (misc.c) malloc() if (...) { layout_randomized_image(output, &ehdr, phdrs); malloc() <- boom ... else layout_image(output, &ehdr, phdrs); free() To avoid collisions, there must be a single implementation of malloc(). Adjust include/linux/decompress/mm.h so that visibility can be controlled, provide prototypes in misc.h, and implement the functions in misc.c. This also results in a small size savings: $ size vmlinux.before vmlinux.after text data bss dec hex filename 8842314 468 178320 9021102 89a6ae vmlinux.before 8842240 468 178320 9021028 89a664 vmlinux.after Fixed-by: Alexander Lobakin Signed-off-by: Kees Cook --- arch/x86/boot/compressed/kaslr.c | 4 ---- arch/x86/boot/compressed/misc.c | 3 +++ arch/x86/boot/compressed/misc.h | 2 ++ include/linux/decompress/mm.h | 12 ++++++++++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 67c3208b668a..411b268bc0a2 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -32,10 +32,6 @@ #include #include -/* Macros used by the included decompressor code below. */ -#define STATIC -#include - #define _SETUP #include /* For COMMAND_LINE_SIZE */ #undef _SETUP diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index 743f13ea25c1..a4339cb2d247 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -28,6 +28,9 @@ /* Macros used by the included decompressor code below. */ #define STATIC static +/* Define an externally visible malloc()/free(). */ +#define MALLOC_VISIBLE +#include /* * Provide definitions of memzero and memmove as some of the decompressors will diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 31139256859f..975ef4ae7395 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -44,6 +44,8 @@ extern char _head[], _end[]; /* misc.c */ extern memptr free_mem_ptr; extern memptr free_mem_end_ptr; +void *malloc(int size); +void free(void *where); extern struct boot_params *boot_params; void __putstr(const char *s); void __puthex(unsigned long value); diff --git a/include/linux/decompress/mm.h b/include/linux/decompress/mm.h index 868e9eacd69e..9192986b1a73 100644 --- a/include/linux/decompress/mm.h +++ b/include/linux/decompress/mm.h @@ -25,13 +25,21 @@ #define STATIC_RW_DATA static #endif +/* + * When an architecture needs to share the malloc()/free() implementation + * between compilation units, it needs to have non-local visibility. + */ +#ifndef MALLOC_VISIBLE +#define MALLOC_VISIBLE static +#endif + /* A trivial malloc implementation, adapted from * malloc by Hannu Savolainen 1993 and Matthias Urlichs 1994 */ STATIC_RW_DATA unsigned long malloc_ptr; STATIC_RW_DATA int malloc_count; -static void *malloc(int size) +MALLOC_VISIBLE void *malloc(int size) { void *p; @@ -52,7 +60,7 @@ static void *malloc(int size) return p; } -static void free(void *where) +MALLOC_VISIBLE void free(void *where) { malloc_count--; if (!malloc_count)