5.4-stable patches

added patches:
	test_firmware-prevent-race-conditions-by-a-correct-implementation-of-locking.patch
	test_firmware-return-enomem-instead-of-enospc-on-failed-memory-allocation.patch

diff --git a/queue-5.4/series b/queue-5.4/series
index 7c19a016811c1ee65408b8292e4e12595cfc786d..f2b024bf726da70cc44f14681628feaa2162cc91 100644 (file)
--- a/queue-5.4/series
+++ b/queue-5.4/series
@@ -131,3 +131,5 @@ bluetooth-l2cap-fix-use-after-free-in-l2cap_sock_ready_cb.patch
 net-usbnet-fix-warning-in-usbnet_start_xmit-usb_submit_urb.patch
 fs-protect-reconfiguration-of-sb-read-write-from-racing-writes.patch
 ext2-drop-fragment-support.patch
+test_firmware-prevent-race-conditions-by-a-correct-implementation-of-locking.patch
+test_firmware-return-enomem-instead-of-enospc-on-failed-memory-allocation.patch

diff --git a/queue-5.4/test_firmware-prevent-race-conditions-by-a-correct-implementation-of-locking.patch b/queue-5.4/test_firmware-prevent-race-conditions-by-a-correct-implementation-of-locking.patch
new file mode 100644 (file)
index 0000000..0f478ba
--- /dev/null
+++ b/queue-5.4/test_firmware-prevent-race-conditions-by-a-correct-implementation-of-locking.patch
@@ -0,0 +1,205 @@
+From 4acfe3dfde685a5a9eaec5555351918e2d7266a1 Mon Sep 17 00:00:00 2001
+From: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+Date: Tue, 9 May 2023 10:47:45 +0200
+Subject: test_firmware: prevent race conditions by a correct implementation of locking
+
+From: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+
+commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 upstream.
+
+Dan Carpenter spotted a race condition in a couple of situations like
+these in the test_firmware driver:
+
+static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
+{
+        u8 val;
+        int ret;
+
+        ret = kstrtou8(buf, 10, &val);
+        if (ret)
+                return ret;
+
+        mutex_lock(&test_fw_mutex);
+        *(u8 *)cfg = val;
+        mutex_unlock(&test_fw_mutex);
+
+        /* Always return full write size even if we didn't consume all */
+        return size;
+}
+
+static ssize_t config_num_requests_store(struct device *dev,
+                                         struct device_attribute *attr,
+                                         const char *buf, size_t count)
+{
+        int rc;
+
+        mutex_lock(&test_fw_mutex);
+        if (test_fw_config->reqs) {
+                pr_err("Must call release_all_firmware prior to changing config\n");
+                rc = -EINVAL;
+                mutex_unlock(&test_fw_mutex);
+                goto out;
+        }
+        mutex_unlock(&test_fw_mutex);
+
+        rc = test_dev_config_update_u8(buf, count,
+                                       &test_fw_config->num_requests);
+
+out:
+        return rc;
+}
+
+static ssize_t config_read_fw_idx_store(struct device *dev,
+                                        struct device_attribute *attr,
+                                        const char *buf, size_t count)
+{
+        return test_dev_config_update_u8(buf, count,
+                                         &test_fw_config->read_fw_idx);
+}
+
+The function test_dev_config_update_u8() is called from both the locked
+and the unlocked context, function config_num_requests_store() and
+config_read_fw_idx_store() which can both be called asynchronously as
+they are driver's methods, while test_dev_config_update_u8() and siblings
+change their argument pointed to by u8 *cfg or similar pointer.
+
+To avoid deadlock on test_fw_mutex, the lock is dropped before calling
+test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
+itself, but alas this creates a race condition.
+
+Having two locks wouldn't assure a race-proof mutual exclusion.
+
+This situation is best avoided by the introduction of a new, unlocked
+function __test_dev_config_update_u8() which can be called from the locked
+context and reducing test_dev_config_update_u8() to:
+
+static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
+{
+        int ret;
+
+        mutex_lock(&test_fw_mutex);
+        ret = __test_dev_config_update_u8(buf, size, cfg);
+        mutex_unlock(&test_fw_mutex);
+
+        return ret;
+}
+
+doing the locking and calling the unlocked primitive, which enables both
+locked and unlocked versions without duplication of code.
+
+The similar approach was applied to all functions called from the locked
+and the unlocked context, which safely mitigates both deadlocks and race
+conditions in the driver.
+
+__test_dev_config_update_bool(), __test_dev_config_update_u8() and
+__test_dev_config_update_size_t() unlocked versions of the functions
+were introduced to be called from the locked contexts as a workaround
+without releasing the main driver's lock and thereof causing a race
+condition.
+
+The test_dev_config_update_bool(), test_dev_config_update_u8() and
+test_dev_config_update_size_t() locked versions of the functions
+are being called from driver methods without the unnecessary multiplying
+of the locking and unlocking code for each method, and complicating
+the code with saving of the return value across lock.
+
+Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
+Cc: Luis Chamberlain <mcgrof@kernel.org>
+Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Cc: Russ Weight <russell.h.weight@intel.com>
+Cc: Takashi Iwai <tiwai@suse.de>
+Cc: Tianfei Zhang <tianfei.zhang@intel.com>
+Cc: Shuah Khan <shuah@kernel.org>
+Cc: Colin Ian King <colin.i.king@gmail.com>
+Cc: Randy Dunlap <rdunlap@infradead.org>
+Cc: linux-kselftest@vger.kernel.org
+Cc: stable@vger.kernel.org # v5.4
+Suggested-by: Dan Carpenter <error27@gmail.com>
+Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ lib/test_firmware.c |   37 ++++++++++++++++++++++++++++---------
+ 1 file changed, 28 insertions(+), 9 deletions(-)
+
+--- a/lib/test_firmware.c
++++ b/lib/test_firmware.c
+@@ -301,16 +301,26 @@ static ssize_t config_test_show_str(char
+       return len;
+ }
+ 
+-static int test_dev_config_update_bool(const char *buf, size_t size,
+-                                     bool *cfg)
++static inline int __test_dev_config_update_bool(const char *buf, size_t size,
++                                              bool *cfg)
+ {
+       int ret;
+ 
+-      mutex_lock(&test_fw_mutex);
+       if (strtobool(buf, cfg) < 0)
+               ret = -EINVAL;
+       else
+               ret = size;
++
++      return ret;
++}
++
++static int test_dev_config_update_bool(const char *buf, size_t size,
++                                     bool *cfg)
++{
++      int ret;
++
++      mutex_lock(&test_fw_mutex);
++      ret = __test_dev_config_update_bool(buf, size, cfg);
+       mutex_unlock(&test_fw_mutex);
+ 
+       return ret;
+@@ -340,7 +350,7 @@ static ssize_t test_dev_config_show_int(
+       return snprintf(buf, PAGE_SIZE, "%d\n", val);
+ }
+ 
+-static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
++static inline int __test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
+ {
+       int ret;
+       long new;
+@@ -352,14 +362,23 @@ static int test_dev_config_update_u8(con
+       if (new > U8_MAX)
+               return -EINVAL;
+ 
+-      mutex_lock(&test_fw_mutex);
+       *(u8 *)cfg = new;
+-      mutex_unlock(&test_fw_mutex);
+ 
+       /* Always return full write size even if we didn't consume all */
+       return size;
+ }
+ 
++static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
++{
++      int ret;
++
++      mutex_lock(&test_fw_mutex);
++      ret = __test_dev_config_update_u8(buf, size, cfg);
++      mutex_unlock(&test_fw_mutex);
++
++      return ret;
++}
++
+ static ssize_t test_dev_config_show_u8(char *buf, u8 cfg)
+ {
+       u8 val;
+@@ -392,10 +411,10 @@ static ssize_t config_num_requests_store
+               mutex_unlock(&test_fw_mutex);
+               goto out;
+       }
+-      mutex_unlock(&test_fw_mutex);
+ 
+-      rc = test_dev_config_update_u8(buf, count,
+-                                     &test_fw_config->num_requests);
++      rc = __test_dev_config_update_u8(buf, count,
++                                       &test_fw_config->num_requests);
++      mutex_unlock(&test_fw_mutex);
+ 
+ out:
+       return rc;

diff --git a/queue-5.4/test_firmware-return-enomem-instead-of-enospc-on-failed-memory-allocation.patch b/queue-5.4/test_firmware-return-enomem-instead-of-enospc-on-failed-memory-allocation.patch
new file mode 100644 (file)
index 0000000..052879b
--- /dev/null
+++ b/queue-5.4/test_firmware-return-enomem-instead-of-enospc-on-failed-memory-allocation.patch
@@ -0,0 +1,95 @@
+From 7dae593cd226a0bca61201cf85ceb9335cf63682 Mon Sep 17 00:00:00 2001
+From: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+Date: Tue, 6 Jun 2023 09:08:10 +0200
+Subject: test_firmware: return ENOMEM instead of ENOSPC on failed memory allocation
+
+From: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+
+commit 7dae593cd226a0bca61201cf85ceb9335cf63682 upstream.
+
+In a couple of situations like
+
+       name = kstrndup(buf, count, GFP_KERNEL);
+       if (!name)
+               return -ENOSPC;
+
+the error is not actually "No space left on device", but "Out of memory".
+
+It is semantically correct to return -ENOMEM in all failed kstrndup()
+and kzalloc() cases in this driver, as it is not a problem with disk
+space, but with kernel memory allocator failing allocation.
+
+The semantically correct should be:
+
+        name = kstrndup(buf, count, GFP_KERNEL);
+        if (!name)
+                return -ENOMEM;
+
+Cc: Dan Carpenter <error27@gmail.com>
+Cc: Takashi Iwai <tiwai@suse.de>
+Cc: Kees Cook <keescook@chromium.org>
+Cc: "Luis R. Rodriguez" <mcgrof@ruslug.rutgers.edu>
+Cc: Scott Branden <sbranden@broadcom.com>
+Cc: Hans de Goede <hdegoede@redhat.com>
+Cc: Brian Norris <briannorris@chromium.org>
+Fixes: c92316bf8e948 ("test_firmware: add batched firmware tests")
+Fixes: 0a8adf584759c ("test: add firmware_class loader test")
+Fixes: 548193cba2a7d ("test_firmware: add support for firmware_request_platform")
+Fixes: eb910947c82f9 ("test: firmware_class: add asynchronous request trigger")
+Fixes: 061132d2b9c95 ("test_firmware: add test custom fallback trigger")
+Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
+Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
+Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
+Message-ID: <20230606070808.9300-1-mirsad.todorovac@alu.unizg.hr>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ lib/test_firmware.c |   10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+--- a/lib/test_firmware.c
++++ b/lib/test_firmware.c
+@@ -173,7 +173,7 @@ static int __kstrncpy(char **dst, const
+ {
+       *dst = kstrndup(name, count, gfp);
+       if (!*dst)
+-              return -ENOSPC;
++              return -ENOMEM;
+       return count;
+ }
+ 
+@@ -509,7 +509,7 @@ static ssize_t trigger_request_store(str
+ 
+       name = kstrndup(buf, count, GFP_KERNEL);
+       if (!name)
+-              return -ENOSPC;
++              return -ENOMEM;
+ 
+       pr_info("loading '%s'\n", name);
+ 
+@@ -552,7 +552,7 @@ static ssize_t trigger_async_request_sto
+ 
+       name = kstrndup(buf, count, GFP_KERNEL);
+       if (!name)
+-              return -ENOSPC;
++              return -ENOMEM;
+ 
+       pr_info("loading '%s'\n", name);
+ 
+@@ -597,7 +597,7 @@ static ssize_t trigger_custom_fallback_s
+ 
+       name = kstrndup(buf, count, GFP_KERNEL);
+       if (!name)
+-              return -ENOSPC;
++              return -ENOMEM;
+ 
+       pr_info("loading '%s' using custom fallback mechanism\n", name);
+ 
+@@ -648,7 +648,7 @@ static int test_fw_run_batch_request(voi
+ 
+               test_buf = kzalloc(TEST_FIRMWARE_BUF_SIZE, GFP_KERNEL);
+               if (!test_buf)
+-                      return -ENOSPC;
++                      return -ENOMEM;
+ 
+               req->rc = request_firmware_into_buf(&req->fw,
+                                                   req->name,