Received: by 2002:ac0:a581:0:0:0:0:0 with SMTP id m1-v6csp287788imm; Mon, 2 Jul 2018 11:30:37 -0700 (PDT) X-Google-Smtp-Source: ADUXVKKB+gZ9yM/JBHung4cK0ykTTteNOL8/StDEGhUgLqzTEiHmLLKlnwJ5fshvkDOd/PmOCwm3 X-Received: by 2002:a17:902:8f94:: with SMTP id z20-v6mr26629257plo.337.1530556237718; Mon, 02 Jul 2018 11:30:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1530556237; cv=none; d=google.com; s=arc-20160816; b=VVKbZ8SuHBMEvW8s9lc0A8xhQfaa40Lt3fTUym8/0W968fPmvzhCremq9yFTRUG+Gf w7tFcBHYApNENMRfVD2LrEZOwCl0dtXHikod/6tgkvwVKPavpXN5Nd8sGX+NAyJMCda/ +yAOkKWYggccLyU+OdagtyxC2bKJkFaHTGwMGiRKKfJSFjUIiw7sxrEluUjdSpdFUJPA iuxJoXKVo/D5nZSypyeq/iBE+47oroHTWgWCC5uCBDKOBQWhFMEkLxvuc231fYeXFQWD jW8FANFrQbIz6x7hJptngwDxd0gapKm9IvTiRY255t57sRNowgi73K1nRaG2scYDphr5 vKxg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature:dkim-signature :arc-authentication-results; bh=DYRu05wnYaDe7opIvzDEfnb2lmho3RndIiX0T8/FYNk=; b=Qa3hFmwYSQ7kkLw+W3J9BXDVG/vb4n3YVD8yh+Hqj6o+yoN39SDE7GDG7hHWVVDdLV MnEi8j75P2w5aBIzzDJDZcuK6m103IlW2lkQa2ANaG74KKSeNfU+urwOh5a1uioOECQS nbFo/m9KdaotCk8dSjmLQ6F/cJwBjso/bKU1MKJCAuCSSWs05qHAuhoLIjq9mY1ksE/q 8PY/n/n1seAMsKwfizIEO4t6X16bhjFqWPz3JVsgl9kb273EKczinTMGfRIyGv9PJ9o/ Fso8s13njssScilhJ9RrAjM7/8CqOTMnRB7ZMqornZ2L5aFIz1Vwbt2APKZHIOUtZje8 1eyw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@google.com header.s=20161025 header.b=q6W5NACQ; dkim=fail header.i=@chromium.org header.s=google header.b="NXd5/xjG"; 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=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 184-v6si6082489pgj.421.2018.07.02.11.30.23; Mon, 02 Jul 2018 11:30:37 -0700 (PDT) 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=fail header.i=@google.com header.s=20161025 header.b=q6W5NACQ; dkim=fail header.i=@chromium.org header.s=google header.b="NXd5/xjG"; 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=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753328AbeGBS2q (ORCPT + 99 others); Mon, 2 Jul 2018 14:28:46 -0400 Received: from mail-yb0-f194.google.com ([209.85.213.194]:39241 "EHLO mail-yb0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753158AbeGBS2o (ORCPT ); Mon, 2 Jul 2018 14:28:44 -0400 Received: by mail-yb0-f194.google.com with SMTP id k127-v6so5439915ybk.6 for ; Mon, 02 Jul 2018 11:28:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=DYRu05wnYaDe7opIvzDEfnb2lmho3RndIiX0T8/FYNk=; b=q6W5NACQ04UqoavYVinBcb9iefNNPjRyL2+Bfg4qHggm1c//FRPrtByvDCmjWkdxIr fMecADD1InrwsySdVzyUBEqyUns0xLLhE7TKP4d/aAPNkkpeVr400usug+WlU3bMTEXj 1nBmhfyBIm807VygLR0JBwyVvPYZTP+msdGT9T/1VNyjv+IcH5wQtw9puRB8c+Vi5gGu T+mp23u543xSjZPAyWqXjk6iJmf+Q5Uvhbt1R4RBbVq1PmUVr+iX4RS80irU1pQUbpES zZFigiG8MA4frKIBEp+vOHmfquC3b5xefkKZtFmAV3eWskA+A4+A5F/cq42bJ+Sg8MHP 0p2g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=DYRu05wnYaDe7opIvzDEfnb2lmho3RndIiX0T8/FYNk=; b=NXd5/xjGygwuvrYdV6MGOUCrG2A0ySwT0eWgGbt4efBwgiQ1vur78bkGLmi3lEY7bN 1xOy+gx/QzbIR8ryZt+xmKKrBlWHd1W+AcrsoFQh9B2K8Rf5B4ri0xc+0rh8MnpeRJYv gwmMWs1NvRijV5fn1NJDqtNuEoUORlFPCEFZw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=DYRu05wnYaDe7opIvzDEfnb2lmho3RndIiX0T8/FYNk=; b=W1yaY6LLjIX8KGhUiQjYyg2oNJv8EnfoBI725/xJnJd9w0tOxn3YCt64CF39PpQqXG iDCSio/pq/IXACc5wbbyrfnl5Dzb9Uf3pomJK2J95o//1CJktAztDwixEpHL5VMEPZy5 pcVykJ9WaS6srXuLUU6Zt4Det7M0ReG7ArzY0wneWRscom1BQwByE9E9RIBFMaI0gMvM Um5Hf1FcQgp/E6P1gdMytQBJkabAjYinlfzTYv10dvA1w67zi1V+0vWh9RQsAfQmZBBg L52kuSbVd5txkP+ZU7wbPmxMcWprLrTNSTm8ctNQ3caxTKzjn2OQ1N2ewiJh203qkPdL WgHA== X-Gm-Message-State: APt69E2np03nGjHFPU9GdaqA8uM1kJlr9OGrHaXTX33MrgOcNnyWvn6G 1bFAmeIyHrl73PVtZpMbi1ySS3b6WqpQIOdSl4Q+mg== X-Received: by 2002:a25:b219:: with SMTP id i25-v6mr14404837ybj.112.1530556123143; Mon, 02 Jul 2018 11:28:43 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a25:5f51:0:0:0:0:0 with HTTP; Mon, 2 Jul 2018 11:28:42 -0700 (PDT) In-Reply-To: <20180702181145.4799-8-ard.biesheuvel@linaro.org> References: <20180702181145.4799-1-ard.biesheuvel@linaro.org> <20180702181145.4799-8-ard.biesheuvel@linaro.org> From: Kees Cook Date: Mon, 2 Jul 2018 11:28:42 -0700 X-Google-Sender-Auth: zPeLIXdIM0rb9Ycc35Bly-c40fA Message-ID: Subject: Re: [PATCH v2 7/8] jump_label: annotate entries that operate on __init code earlier To: Ard Biesheuvel Cc: linux-arm-kernel , LKML , linux-s390 , linux-arch , Arnd Bergmann , Heiko Carstens , Will Deacon , Thomas Gleixner , Catalin Marinas , Ingo Molnar , Steven Rostedt , Martin Schwidefsky , Jessica Yu , Peter Zijlstra Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 2, 2018 at 11:11 AM, Ard Biesheuvel wrote: > Jump table entries are mostly read-only, with the exception of the > init and module loader code that defuses entries that point into init > code when the code being referred to is freed. > > For robustness, it would be better to move these entries into the > ro_after_init section, but clearing the 'code' member of each jump > table entry referring to init code at module load time races with the > module_enable_ro() call that remaps the ro_after_init section read > only, so we'd like to do it earlier. > > So given that whether such an entry refers to init code can be decided > much earlier, we can pull this check forward. Since we may still need > the code entry at this point, let's switch to setting a low bit in the > 'key' member just like we do to annotate the default state of a jump > table entry. > > Signed-off-by: Ard Biesheuvel > --- > include/linux/jump_label.h | 11 ++--- > init/main.c | 1 - > kernel/jump_label.c | 48 ++++++-------------- > 3 files changed, 18 insertions(+), 42 deletions(-) Net reduction in code, too! :) Reviewed-by: Kees Cook -Kees > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 871826fd0c3b..feee8abc96be 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -141,7 +141,7 @@ static inline unsigned long jump_entry_target(const struct jump_entry *entry) > > static inline struct static_key *jump_entry_key(const struct jump_entry *entry) > { > - long offset = entry->key & ~1L; > + long offset = entry->key & ~3L; > > return (struct static_key *)((unsigned long)&entry->key + offset); > } > @@ -160,7 +160,7 @@ static inline unsigned long jump_entry_target(const struct jump_entry *entry) > > static inline struct static_key *jump_entry_key(const struct jump_entry *entry) > { > - return (struct static_key *)((unsigned long)entry->key & ~1UL); > + return (struct static_key *)((unsigned long)entry->key & ~3UL); > } > > #endif > @@ -172,12 +172,12 @@ static inline bool jump_entry_is_branch(const struct jump_entry *entry) > > static inline bool jump_entry_is_init(const struct jump_entry *entry) > { > - return entry->code == 0; > + return (unsigned long)entry->key & 2UL; > } > > static inline void jump_entry_set_init(struct jump_entry *entry) > { > - entry->code = 0; > + entry->key |= 2; > } > > #endif > @@ -213,7 +213,6 @@ extern struct jump_entry __start___jump_table[]; > extern struct jump_entry __stop___jump_table[]; > > extern void jump_label_init(void); > -extern void jump_label_invalidate_initmem(void); > extern void jump_label_lock(void); > extern void jump_label_unlock(void); > extern void arch_jump_label_transform(struct jump_entry *entry, > @@ -261,8 +260,6 @@ static __always_inline void jump_label_init(void) > static_key_initialized = true; > } > > -static inline void jump_label_invalidate_initmem(void) {} > - > static __always_inline bool static_key_false(struct static_key *key) > { > if (unlikely(static_key_count(key) > 0)) > diff --git a/init/main.c b/init/main.c > index e59a01f163d6..d1a6b8a896e5 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1062,7 +1062,6 @@ static int __ref kernel_init(void *unused) > /* need to finish all async __init code before freeing the memory */ > async_synchronize_full(); > ftrace_free_init_mem(); > - jump_label_invalidate_initmem(); > free_initmem(); > mark_readonly(); > system_state = SYSTEM_RUNNING; > diff --git a/kernel/jump_label.c b/kernel/jump_label.c > index d424e1d22d63..7cdd49aeaf6a 100644 > --- a/kernel/jump_label.c > +++ b/kernel/jump_label.c > @@ -373,14 +373,15 @@ static enum jump_label_type jump_label_type(struct jump_entry *entry) > > static void __jump_label_update(struct static_key *key, > struct jump_entry *entry, > - struct jump_entry *stop) > + struct jump_entry *stop, > + bool init) > { > for (; (entry < stop) && (jump_entry_key(entry) == key); entry++) { > /* > * An entry->code of 0 indicates an entry which has been > * disabled because it was in an init text area. > */ > - if (!jump_entry_is_init(entry)) { > + if (init || !jump_entry_is_init(entry)) { > if (kernel_text_address(jump_entry_code(entry))) > arch_jump_label_transform(entry, jump_label_type(entry)); > else > @@ -420,6 +421,9 @@ void __init jump_label_init(void) > if (jump_label_type(iter) == JUMP_LABEL_NOP) > arch_jump_label_transform_static(iter, JUMP_LABEL_NOP); > > + if (init_section_contains((void *)jump_entry_code(iter), 1)) > + jump_entry_set_init(iter); > + > iterk = jump_entry_key(iter); > if (iterk == key) > continue; > @@ -432,19 +436,6 @@ void __init jump_label_init(void) > cpus_read_unlock(); > } > > -/* Disable any jump label entries in __init/__exit code */ > -void __init jump_label_invalidate_initmem(void) > -{ > - struct jump_entry *iter_start = __start___jump_table; > - struct jump_entry *iter_stop = __stop___jump_table; > - struct jump_entry *iter; > - > - for (iter = iter_start; iter < iter_stop; iter++) { > - if (init_section_contains((void *)jump_entry_code(iter), 1)) > - jump_entry_set_init(iter); > - } > -} > - > #ifdef CONFIG_MODULES > > static enum jump_label_type jump_label_init_type(struct jump_entry *entry) > @@ -524,7 +515,8 @@ static void __jump_label_mod_update(struct static_key *key) > stop = __stop___jump_table; > else > stop = m->jump_entries + m->num_jump_entries; > - __jump_label_update(key, mod->entries, stop); > + __jump_label_update(key, mod->entries, stop, > + m->state == MODULE_STATE_COMING); > } > } > > @@ -570,6 +562,9 @@ static int jump_label_add_module(struct module *mod) > for (iter = iter_start; iter < iter_stop; iter++) { > struct static_key *iterk; > > + if (within_module_init(jump_entry_code(iter), mod)) > + jump_entry_set_init(iter); > + > iterk = jump_entry_key(iter); > if (iterk == key) > continue; > @@ -605,7 +600,7 @@ static int jump_label_add_module(struct module *mod) > > /* Only update if we've changed from our initial state */ > if (jump_label_type(iter) != jump_label_init_type(iter)) > - __jump_label_update(key, iter, iter_stop); > + __jump_label_update(key, iter, iter_stop, true); > } > > return 0; > @@ -661,19 +656,6 @@ static void jump_label_del_module(struct module *mod) > } > } > > -/* Disable any jump label entries in module init code */ > -static void jump_label_invalidate_module_init(struct module *mod) > -{ > - struct jump_entry *iter_start = mod->jump_entries; > - struct jump_entry *iter_stop = iter_start + mod->num_jump_entries; > - struct jump_entry *iter; > - > - for (iter = iter_start; iter < iter_stop; iter++) { > - if (within_module_init(jump_entry_code(iter), mod)) > - jump_entry_set_init(iter); > - } > -} > - > static int > jump_label_module_notify(struct notifier_block *self, unsigned long val, > void *data) > @@ -695,9 +677,6 @@ jump_label_module_notify(struct notifier_block *self, unsigned long val, > case MODULE_STATE_GOING: > jump_label_del_module(mod); > break; > - case MODULE_STATE_LIVE: > - jump_label_invalidate_module_init(mod); > - break; > } > > jump_label_unlock(); > @@ -767,7 +746,8 @@ static void jump_label_update(struct static_key *key) > entry = static_key_entries(key); > /* if there are no users, entry can be NULL */ > if (entry) > - __jump_label_update(key, entry, stop); > + __jump_label_update(key, entry, stop, > + system_state < SYSTEM_RUNNING); > } > > #ifdef CONFIG_STATIC_KEYS_SELFTEST > -- > 2.17.1 > -- Kees Cook Pixel Security