--- /dev/null
+From adb9743d6a08778b78d62d16b4230346d3508986 Mon Sep 17 00:00:00 2001
+From: Qi Zheng <zhengqi.arch@bytedance.com>
+Date: Sun, 25 Jun 2023 15:49:37 +0000
+Subject: binder: fix memory leak in binder_init()
+
+From: Qi Zheng <zhengqi.arch@bytedance.com>
+
+commit adb9743d6a08778b78d62d16b4230346d3508986 upstream.
+
+In binder_init(), the destruction of binder_alloc_shrinker_init() is not
+performed in the wrong path, which will cause memory leaks. So this commit
+introduces binder_alloc_shrinker_exit() and calls it in the wrong path to
+fix that.
+
+Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
+Acked-by: Carlos Llamas <cmllamas@google.com>
+Fixes: f2517eb76f1f ("android: binder: Add global lru shrinker to binder")
+Cc: stable <stable@kernel.org>
+Link: https://lore.kernel.org/r/20230625154937.64316-1-qi.zheng@linux.dev
+[cmllamas: resolved trivial merge conflicts]
+Signed-off-by: Carlos Llamas <cmllamas@google.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/android/binder.c | 1 +
+ drivers/android/binder_alloc.c | 6 ++++++
+ drivers/android/binder_alloc.h | 1 +
+ 3 files changed, 8 insertions(+)
+
+--- a/drivers/android/binder.c
++++ b/drivers/android/binder.c
+@@ -5658,6 +5658,7 @@ err_init_binder_device_failed:
+
+ err_alloc_device_names_failed:
+ debugfs_remove_recursive(binder_debugfs_dir_entry_root);
++ binder_alloc_shrinker_exit();
+
+ return ret;
+ }
+--- a/drivers/android/binder_alloc.c
++++ b/drivers/android/binder_alloc.c
+@@ -1033,3 +1033,9 @@ void binder_alloc_shrinker_init(void)
+ list_lru_init(&binder_alloc_lru);
+ register_shrinker(&binder_shrinker);
+ }
++
++void binder_alloc_shrinker_exit(void)
++{
++ unregister_shrinker(&binder_shrinker);
++ list_lru_destroy(&binder_alloc_lru);
++}
+--- a/drivers/android/binder_alloc.h
++++ b/drivers/android/binder_alloc.h
+@@ -128,6 +128,7 @@ extern struct binder_buffer *binder_allo
+ int is_async);
+ extern void binder_alloc_init(struct binder_alloc *alloc);
+ void binder_alloc_shrinker_init(void);
++extern void binder_alloc_shrinker_exit(void);
+ extern void binder_alloc_vma_close(struct binder_alloc *alloc);
+ extern struct binder_buffer *
+ binder_alloc_prepare_to_free(struct binder_alloc *alloc,
--- /dev/null
+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
+@@ -283,16 +283,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;
+@@ -322,7 +332,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;
+@@ -334,14 +344,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;
+@@ -374,10 +393,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;
projects / thirdparty / kernel / stable-queue.git / commitdiff
