Received: by 2002:a05:6a10:2726:0:0:0:0 with SMTP id ib38csp693341pxb; Tue, 5 Apr 2022 19:03:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx0kuDBzi8lMG40L8TQRp/cyG31A5OAL4tu/NFSYkK07O8me3qyUKR+kGsRwIfXrjtUfRtN X-Received: by 2002:a17:902:e5ce:b0:156:bb45:92a2 with SMTP id u14-20020a170902e5ce00b00156bb4592a2mr6323932plf.34.1649210591129; Tue, 05 Apr 2022 19:03:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1649210591; cv=none; d=google.com; s=arc-20160816; b=QBTb0rKKZm/QSjZl6/HcUUJXLv/5A6bmmFbcaIUx9JXDNJc59QsLohpqS/hW/0ioJO QOfLYwnIZ39j2WHmYfd2awR3FHMknXxL1x9lMvxsG0YaV0fP7KMsqI+Y3uvh0AD3KIhC p827ugTlJ6yyoG7dI7XzJnzDoRARJBtx/z+IkK/m+/W74ts0n4jO/eG/e9iqTTxeXnyn 47Z8OgsZSC9cwwhkT2l296QCX7XZvIxZofV7jFVNUhrHPGwdEfiltpSuFbbjMpTdmQop cWcJKNDKCyPgb+Acxk/6OkO4zKARj216VQeyajWiEFehGBkedSfM2bbgFrDV/n1hkgNR HDQQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=DZH1aI+sxjiyNB26q9On50lqEX7/WjGD/28hNF58PAw=; b=cWW/OY/zX7+XgqWj8UvNmhzbQVoWIa+IfBAUtf4gMLq5mx5zTgWjqBbCzxlNyh++of LWGkZnFXlKkWXXxEEAIxKN0XtgE6SIRDsvMxO6kTc5zI6CIUA8KWaHwqC3UR0DjdxWAO WmmFOYrXUpkzPCc24pX0qQQCoryLU7UXZkiTJQlEhsRC17pYN7zwuVMkCYEIi/JFZRYe RSNPJ9ZEnjrZECZFTGPEdqLViwxHq3vAL+JNfbVX23WTVxq3UyLN43Reb4X+eGszNlLi xu4Q6MSJIGkJTWmhthk/jhqS8N18dOtHwV0BIGrJf/eXwPgnec1KCxKCEc9x3kdGo1di yGJw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=DKhnuJez; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q1-20020a63ae01000000b00381f044e11fsi14702927pgf.550.2022.04.05.19.02.53; Tue, 05 Apr 2022 19:03:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=DKhnuJez; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232808AbiDEH4V (ORCPT + 99 others); Tue, 5 Apr 2022 03:56:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233565AbiDEHr6 (ORCPT ); Tue, 5 Apr 2022 03:47:58 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 19F1DBB; Tue, 5 Apr 2022 00:45:45 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AA3BF616C4; Tue, 5 Apr 2022 07:45:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B0DB4C340EE; Tue, 5 Apr 2022 07:45:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1649144744; bh=toUIKSG7Y9lYiTObnBI3exiCjRwsy9pdblqnPZcepWY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DKhnuJezIRO73IKInc2pryqh6X5HVmW0G3yQMdIUXI8zY/G9iy9yAMdk5KYCJ90Y1 ZQyT7yjdpTk00lAPpaHgRBTenlqadgDyoiLChietgtUII1pBsoZCF9PV4NOvhxqz8H 4//ea2VPjvOsJPBUC7nfQoJbxtlee3CxpkQ3SVfM= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Sebastian Andrzej Siewior , Jann Horn , Kees Cook Subject: [PATCH 5.17 0156/1126] pstore: Dont use semaphores in always-atomic-context code Date: Tue, 5 Apr 2022 09:15:03 +0200 Message-Id: <20220405070412.167026517@linuxfoundation.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220405070407.513532867@linuxfoundation.org> References: <20220405070407.513532867@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Jann Horn commit 8126b1c73108bc691f5643df19071a59a69d0bc6 upstream. pstore_dump() is *always* invoked in atomic context (nowadays in an RCU read-side critical section, before that under a spinlock). It doesn't make sense to try to use semaphores here. This is mostly a revert of commit ea84b580b955 ("pstore: Convert buf_lock to semaphore"), except that two parts aren't restored back exactly as they were: - keep the lock initialization in pstore_register - in efi_pstore_write(), always set the "block" flag to false - omit "is_locked", that was unnecessary since commit 959217c84c27 ("pstore: Actually give up during locking failure") - fix the bailout message The actual problem that the buggy commit was trying to address may have been that the use of preemptible() in efi_pstore_write() was wrong - it only looks at preempt_count() and the state of IRQs, but __rcu_read_lock() doesn't touch either of those under CONFIG_PREEMPT_RCU. (Sidenote: CONFIG_PREEMPT_RCU means that the scheduler can preempt tasks in RCU read-side critical sections, but you're not allowed to actively block/reschedule.) Lockdep probably never caught the problem because it's very rare that you actually hit the contended case, so lockdep always just sees the down_trylock(), not the down_interruptible(), and so it can't tell that there's a problem. Fixes: ea84b580b955 ("pstore: Convert buf_lock to semaphore") Cc: stable@vger.kernel.org Acked-by: Sebastian Andrzej Siewior Signed-off-by: Jann Horn Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20220314185953.2068993-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/efi/efi-pstore.c | 2 +- fs/pstore/platform.c | 38 ++++++++++++++++++-------------------- include/linux/pstore.h | 6 +++--- 3 files changed, 22 insertions(+), 24 deletions(-) --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -266,7 +266,7 @@ static int efi_pstore_write(struct pstor efi_name[i] = name[i]; ret = efivar_entry_set_safe(efi_name, vendor, PSTORE_EFI_ATTRIBUTES, - preemptible(), record->size, record->psi->buf); + false, record->size, record->psi->buf); if (record->reason == KMSG_DUMP_OOPS && try_module_get(THIS_MODULE)) if (!schedule_work(&efivar_work)) --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -143,21 +143,22 @@ static void pstore_timer_kick(void) mod_timer(&pstore_timer, jiffies + msecs_to_jiffies(pstore_update_ms)); } -/* - * Should pstore_dump() wait for a concurrent pstore_dump()? If - * not, the current pstore_dump() will report a failure to dump - * and return. - */ -static bool pstore_cannot_wait(enum kmsg_dump_reason reason) +static bool pstore_cannot_block_path(enum kmsg_dump_reason reason) { - /* In NMI path, pstore shouldn't block regardless of reason. */ + /* + * In case of NMI path, pstore shouldn't be blocked + * regardless of reason. + */ if (in_nmi()) return true; switch (reason) { /* In panic case, other cpus are stopped by smp_send_stop(). */ case KMSG_DUMP_PANIC: - /* Emergency restart shouldn't be blocked. */ + /* + * Emergency restart shouldn't be blocked by spinning on + * pstore_info::buf_lock. + */ case KMSG_DUMP_EMERG: return true; default: @@ -389,21 +390,19 @@ static void pstore_dump(struct kmsg_dump unsigned long total = 0; const char *why; unsigned int part = 1; + unsigned long flags = 0; int ret; why = kmsg_dump_reason_str(reason); - if (down_trylock(&psinfo->buf_lock)) { - /* Failed to acquire lock: give up if we cannot wait. */ - if (pstore_cannot_wait(reason)) { - pr_err("dump skipped in %s path: may corrupt error record\n", - in_nmi() ? "NMI" : why); - return; - } - if (down_interruptible(&psinfo->buf_lock)) { - pr_err("could not grab semaphore?!\n"); + if (pstore_cannot_block_path(reason)) { + if (!spin_trylock_irqsave(&psinfo->buf_lock, flags)) { + pr_err("dump skipped in %s path because of concurrent dump\n", + in_nmi() ? "NMI" : why); return; } + } else { + spin_lock_irqsave(&psinfo->buf_lock, flags); } kmsg_dump_rewind(&iter); @@ -467,8 +466,7 @@ static void pstore_dump(struct kmsg_dump total += record.size; part++; } - - up(&psinfo->buf_lock); + spin_unlock_irqrestore(&psinfo->buf_lock, flags); } static struct kmsg_dumper pstore_dumper = { @@ -594,7 +592,7 @@ int pstore_register(struct pstore_info * psi->write_user = pstore_write_user_compat; psinfo = psi; mutex_init(&psinfo->read_mutex); - sema_init(&psinfo->buf_lock, 1); + spin_lock_init(&psinfo->buf_lock); if (psi->flags & PSTORE_FLAGS_DMESG) allocate_buf_for_compression(); --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include @@ -87,7 +87,7 @@ struct pstore_record { * @owner: module which is responsible for this backend driver * @name: name of the backend driver * - * @buf_lock: semaphore to serialize access to @buf + * @buf_lock: spinlock to serialize access to @buf * @buf: preallocated crash dump buffer * @bufsize: size of @buf available for crash dump bytes (must match * smallest number of bytes available for writing to a @@ -178,7 +178,7 @@ struct pstore_info { struct module *owner; const char *name; - struct semaphore buf_lock; + spinlock_t buf_lock; char *buf; size_t bufsize;