All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Branden <scott.branden@broadcom.com>
To: Luis Chamberlain <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Shuah Khan <shuah@kernel.org>,
	bjorn.andersson@linaro.org
Cc: "Rafael J . Wysocki" <rafael@kernel.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,
	Olof Johansson <olof@lixom.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Colin Ian King <colin.king@canonical.com>,
	Kees Cook <keescook@chromium.org>, Takashi Iwai <tiwai@suse.de>,
	linux-kselftest@vger.kernel.org,
	Scott Branden <scott.branden@broadcom.com>
Subject: [PATCH 3/3] firmware: add mutex fw_lock_fallback for race condition
Date: Thu, 15 Aug 2019 17:09:45 -0700	[thread overview]
Message-ID: <20190816000945.29810-4-scott.branden@broadcom.com> (raw)
In-Reply-To: <20190816000945.29810-1-scott.branden@broadcom.com>

A race condition exists between _request_firmware_prepare checking
if firmware is assigned and firmware_fallback_sysfs creating a sysfs
entry (kernel trace below).  To avoid such condition add a mutex
fw_lock_fallback to protect against such condition.

misc test_firmware: Falling back to sysfs fallback for: nope-test-firmware.bin
sysfs: cannot create duplicate filename '/devices/virtual/misc/test_firmware/nope-test-firmware.bin'
CPU: 4 PID: 2059 Comm: test_firmware-3 Not tainted 5.3.0-rc4 #1
Hardware name: Dell Inc. OptiPlex 7010/0KRC95, BIOS A13 03/25/2013
Call Trace:
 dump_stack+0x67/0x90
 sysfs_warn_dup.cold+0x17/0x24
 sysfs_create_dir_ns+0xb3/0xd0
 kobject_add_internal+0xa6/0x2a0
 kobject_add+0x7e/0xb0
 ? _cond_resched+0x15/0x30
 device_add+0x121/0x670
 firmware_fallback_sysfs+0x15c/0x3c9
 _request_firmware+0x432/0x5a0
 ? devres_find+0x63/0xc0
 request_firmware_into_buf+0x63/0x80
 test_fw_run_batch_request+0x96/0xe0
 kthread+0xfb/0x130
 ? reset_store+0x30/0x30
 ? kthread_park+0x80/0x80
 ret_from_fork+0x3a/0x50
kobject_add_internal failed for nope-test-firmware.bin with -EEXIST, don't try to register things with the same name in the same directory.

Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/base/firmware_loader/main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index bf44c79beae9..ce9896e3b782 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -88,6 +88,7 @@ static inline struct fw_priv *to_fw_priv(struct kref *ref)
 /* fw_lock could be moved to 'struct fw_sysfs' but since it is just
  * guarding for corner cases a global lock should be OK */
 DEFINE_MUTEX(fw_lock);
+DEFINE_MUTEX(fw_lock_fallback);
 
 static struct firmware_cache fw_cache;
 
@@ -758,6 +759,17 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	if (!firmware_p)
 		return -EINVAL;
 
+	/*
+	 * There is a race condition between _request_firmware_prepare checking
+	 * if firmware is assigned and firmware_fallback_sysfs creating sysfs
+	 * entries with duplicate names.
+	 * Yet, with this lock the firmware_test locks up with cache enabled
+	 * and no event used during firmware test.
+	 * This points to some very racy code I don't know how to entirely fix.
+	 */
+	if (opt_flags & FW_OPT_NOCACHE)
+		mutex_lock(&fw_lock_fallback);
+
 	if (!name || name[0] == '\0') {
 		ret = -EINVAL;
 		goto out;
@@ -791,6 +803,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		fw = NULL;
 	}
 
+	if (opt_flags & FW_OPT_NOCACHE)
+		mutex_unlock(&fw_lock_fallback);
+
 	*firmware_p = fw;
 	return ret;
 }
-- 
2.17.1


  parent reply	other threads:[~2019-08-16  0:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16  0:09 [PATCH 0/3] firmware: selftest for request_firmware_into_buf Scott Branden
2019-08-16  0:09 ` [PATCH 1/3] test_firmware: add support " Scott Branden
2019-08-19  5:24   ` Luis Chamberlain
2019-08-19 20:27     ` shuah
2019-08-16  0:09 ` [PATCH 2/3] selftest: firmware: Add request_firmware_into_buf tests Scott Branden
2019-08-19  5:24   ` Luis Chamberlain
2019-08-19 20:27     ` shuah
2019-08-16  0:09 ` Scott Branden [this message]
2019-08-19  5:39   ` [PATCH 3/3] firmware: add mutex fw_lock_fallback for race condition Luis Chamberlain
2019-08-19 16:19     ` Scott Branden
2019-08-20  1:26       ` Luis Chamberlain
2019-08-20 15:54         ` Scott Branden
2019-08-23 10:31         ` Takashi Iwai
2019-08-23 15:43           ` Luis Chamberlain
2019-08-23 19:56             ` Scott Branden
2019-08-23 19:48           ` Scott Branden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190816000945.29810-4-scott.branden@broadcom.com \
    --to=scott.branden@broadcom.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy.gross@linaro.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=colin.king@canonical.com \
    --cc=dan.carpenter@oracle.com \
    --cc=david.brown@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=olof@lixom.net \
    --cc=rafael@kernel.org \
    --cc=shuah@kernel.org \
    --cc=tiwai@suse.de \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.