Skip to content
Snippets Groups Projects
  1. Jun 22, 2023
  2. Jun 12, 2023
  3. Jun 08, 2023
  4. May 31, 2023
    • Mirsad Goran Todorovac's avatar
      test_firmware: fix the memory leak of the allocated firmware buffer · 48e15602
      Mirsad Goran Todorovac authored
      
      The following kernel memory leak was noticed after running
      tools/testing/selftests/firmware/fw_run_tests.sh:
      
      [root@pc-mtodorov firmware]# cat /sys/kernel/debug/kmemleak
      .
      .
      .
      unreferenced object 0xffff955389bc3400 (size 1024):
        comm "test_firmware-0", pid 5451, jiffies 4294944822 (age 65.652s)
        hex dump (first 32 bytes):
          47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
          [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
          [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
          [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
          [<ffffffff95fd813b>] kthread+0x10b/0x140
          [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
      unreferenced object 0xffff9553c334b400 (size 1024):
        comm "test_firmware-1", pid 5452, jiffies 4294944822 (age 65.652s)
        hex dump (first 32 bytes):
          47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
          [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
          [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
          [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
          [<ffffffff95fd813b>] kthread+0x10b/0x140
          [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
      unreferenced object 0xffff9553c334f000 (size 1024):
        comm "test_firmware-2", pid 5453, jiffies 4294944822 (age 65.652s)
        hex dump (first 32 bytes):
          47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
          [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
          [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
          [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
          [<ffffffff95fd813b>] kthread+0x10b/0x140
          [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
      unreferenced object 0xffff9553c3348400 (size 1024):
        comm "test_firmware-3", pid 5454, jiffies 4294944822 (age 65.652s)
        hex dump (first 32 bytes):
          47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
          [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
          [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
          [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
          [<ffffffff95fd813b>] kthread+0x10b/0x140
          [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
      [root@pc-mtodorov firmware]#
      
      Note that the size 1024 corresponds to the size of the test firmware
      buffer. The actual number of the buffers leaked is around 70-110,
      depending on the test run.
      
      The cause of the leak is the following:
      
      request_partial_firmware_into_buf() and request_firmware_into_buf()
      provided firmware buffer isn't released on release_firmware(), we
      have allocated it and we are responsible for deallocating it manually.
      This is introduced in a number of context where previously only
      release_firmware() was called, which was insufficient.
      
      Reported-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
      Fixes: 7feebfa4 ("test_firmware: add support for request_firmware_into_buf")
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Dan Carpenter <error27@gmail.com>
      Cc: Takashi Iwai <tiwai@suse.de>
      Cc: Luis Chamberlain <mcgrof@kernel.org>
      Cc: Russ Weight <russell.h.weight@intel.com>
      Cc: Tianfei zhang <tianfei.zhang@intel.com>
      Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
      Cc: Zhengchao Shao <shaozhengchao@huawei.com>
      Cc: Colin Ian King <colin.i.king@gmail.com>
      Cc: linux-kernel@vger.kernel.org
      Cc: Kees Cook <keescook@chromium.org>
      Cc: Scott Branden <sbranden@broadcom.com>
      Cc: Luis R. Rodriguez <mcgrof@kernel.org>
      Cc: linux-kselftest@vger.kernel.org
      Cc: stable@vger.kernel.org # v5.4
      Signed-off-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
      Link: https://lore.kernel.org/r/20230509084746.48259-3-mirsad.todorovac@alu.unizg.hr
      
      
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      48e15602
    • Mirsad Goran Todorovac's avatar
      test_firmware: fix a memory leak with reqs buffer · be37bed7
      Mirsad Goran Todorovac authored
      
      Dan Carpenter spotted that test_fw_config->reqs will be leaked if
      trigger_batched_requests_store() is called two or more times.
      The same appears with trigger_batched_requests_async_store().
      
      This bug wasn't trigger by the tests, but observed by Dan's visual
      inspection of the code.
      
      The recommended workaround was to return -EBUSY if test_fw_config->reqs
      is already allocated.
      
      Fixes: 7feebfa4 ("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: 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: default avatarDan Carpenter <error27@gmail.com>
      Suggested-by: default avatarTakashi Iwai <tiwai@suse.de>
      Signed-off-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
      Acked-by: default avatarLuis Chamberlain <mcgrof@kernel.org>
      Link: https://lore.kernel.org/r/20230509084746.48259-2-mirsad.todorovac@alu.unizg.hr
      
      
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      be37bed7
    • Mirsad Goran Todorovac's avatar
      test_firmware: prevent race conditions by a correct implementation of locking · 4acfe3df
      Mirsad Goran Todorovac authored
      
      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: 7feebfa4 ("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: default avatarDan Carpenter <error27@gmail.com>
      Signed-off-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
      Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
      
      
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      4acfe3df
  5. May 22, 2023
  6. May 17, 2023
  7. May 09, 2023
  8. May 03, 2023
    • Kefeng Wang's avatar
      mm: hwpoison: coredump: support recovery from dump_user_range() · 245f0922
      Kefeng Wang authored
      dump_user_range() is used to copy the user page to a coredump file, but if
      a hardware memory error occurred during copy, which called from
      __kernel_write_iter() in dump_user_range(), it crashes,
      
        CPU: 112 PID: 7014 Comm: mca-recover Not tainted 6.3.0-rc2 #425
      
        pc : __memcpy+0x110/0x260
        lr : _copy_from_iter+0x3bc/0x4c8
        ...
        Call trace:
         __memcpy+0x110/0x260
         copy_page_from_iter+0xcc/0x130
         pipe_write+0x164/0x6d8
         __kernel_write_iter+0x9c/0x210
         dump_user_range+0xc8/0x1d8
         elf_core_dump+0x308/0x368
         do_coredump+0x2e8/0xa40
         get_signal+0x59c/0x788
         do_signal+0x118/0x1f8
         do_notify_resume+0xf0/0x280
         el0_da+0x130/0x138
         el0t_64_sync_handler+0x68/0xc0
         el0t_64_sync+0x188/0x190
      
      Generally, the '->write_iter' of file ops will use copy_page_from_iter()
      and copy_page_from_iter_atomic(), change memcpy() to copy_mc_to_kernel()
      in both of them to handle #MC during source read, which stop coredump
      processing and kill the task instead of kernel panic, but the source
      address may not always a user address, so introduce a new copy_mc flag in
      struct iov_iter{} to indicate that the iter could do a safe memory copy,
      also introduce the helpers to set/cleck the flag, for now, it's only used
      in coredump's dump_user_range(), but it could expand to any other
      scenarios to fix the similar issue.
      
      Link: https://lkml.kernel.org/r/20230417045323.11054-1-wangkefeng.wang@huawei.com
      
      
      Signed-off-by: default avatarKefeng Wang <wangkefeng.wang@huawei.com>
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: Christian Brauner <brauner@kernel.org>
      Cc: Miaohe Lin <linmiaohe@huawei.com>
      Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
      Cc: Tong Tiangen <tongtiangen@huawei.com>
      Cc: Jens Axboe <axboe@kernel.dk>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      245f0922
  9. May 02, 2023
  10. Apr 26, 2023
  11. Apr 24, 2023
  12. Apr 21, 2023
  13. Apr 18, 2023
  14. Apr 17, 2023
  15. Apr 16, 2023
  16. Apr 15, 2023
    • Thomas Gleixner's avatar
      debugobject: Prevent init race with static objects · 63a75969
      Thomas Gleixner authored
      
      Statically initialized objects are usually not initialized via the init()
      function of the subsystem. They are special cased and the subsystem
      provides a function to validate whether an object which is not yet tracked
      by debugobjects is statically initialized. This means the object is started
      to be tracked on first use, e.g. activation.
      
      This works perfectly fine, unless there are two concurrent operations on
      that object. Schspa decoded the problem:
      
      T0 	          	    	    T1
      
      debug_object_assert_init(addr)
        lock_hash_bucket()
        obj = lookup_object(addr);
        if (!obj) {
        	unlock_hash_bucket();
      	- > preemption
      			            lock_subsytem_object(addr);
      				      activate_object(addr)
      				      lock_hash_bucket();
      				      obj = lookup_object(addr);
      				      if (!obj) {
      				    	unlock_hash_bucket();
      					if (is_static_object(addr))
      					   init_and_track(addr);
      				      lock_hash_bucket();
      				      obj = lookup_object(addr);
      				      obj->state = ACTIVATED;
      				      unlock_hash_bucket();
      
      				    subsys function modifies content of addr,
      				    so static object detection does
      				    not longer work.
      
      				    unlock_subsytem_object(addr);
      				    
              if (is_static_object(addr)) <- Fails
      
      	  debugobject emits a warning and invokes the fixup function which
      	  reinitializes the already active object in the worst case.
      
      This race exists forever, but was never observed until mod_timer() got a
      debug_object_assert_init() added which is outside of the timer base lock
      held section right at the beginning of the function to cover the lockless
      early exit points too.
      
      Rework the code so that the lookup, the static object check and the
      tracking object association happens atomically under the hash bucket
      lock. This prevents the issue completely as all callers are serialized on
      the hash bucket lock and therefore cannot observe inconsistent state.
      
      Fixes: 3ac7fe5a ("infrastructure to debug (dynamic) objects")
      Reported-by: default avatar <syzbot+5093ba19745994288b53@syzkaller.appspotmail.com>
      Debugged-by: default avatarSchspa Shi <schspa@gmail.com>
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Reviewed-by: default avatarStephen Boyd <swboyd@chromium.org>
      Link: https://syzkaller.appspot.com/bug?id=22c8a5938eab640d1c6bcc0e3dc7be519d878462
      Link: https://lore.kernel.org/lkml/20230303161906.831686-1-schspa@gmail.com
      Link: https://lore.kernel.org/r/87zg7dzgao.ffs@tglx
      63a75969
  17. Apr 13, 2023
  18. Apr 12, 2023
  19. Apr 08, 2023
  20. Apr 06, 2023
    • Lorenzo Stoakes's avatar
      iov_iter: add copy_page_to_iter_nofault() · 4f80818b
      Lorenzo Stoakes authored
      Provide a means to copy a page to user space from an iterator, aborting if
      a page fault would occur.  This supports compound pages, but may be passed
      a tail page with an offset extending further into the compound page, so we
      cannot pass a folio.
      
      This allows for this function to be called from atomic context and _try_
      to user pages if they are faulted in, aborting if not.
      
      The function does not use _copy_to_iter() in order to not specify
      might_fault(), this is similar to copy_page_from_iter_atomic().
      
      This is being added in order that an iteratable form of vread() can be
      implemented while holding spinlocks.
      
      Link: https://lkml.kernel.org/r/19734729defb0f498a76bdec1bef3ac48a3af3e8.1679511146.git.lstoakes@gmail.com
      
      
      Signed-off-by: default avatarLorenzo Stoakes <lstoakes@gmail.com>
      Reviewed-by: default avatarBaoquan He <bhe@redhat.com>
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: David Hildenbrand <david@redhat.com>
      Cc: Jens Axboe <axboe@kernel.dk>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Liu Shixin <liushixin2@huawei.com>
      Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
      Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      4f80818b
    • Peng Zhang's avatar
      maple_tree: fix a potential concurrency bug in RCU mode · c45ea315
      Peng Zhang authored
      There is a concurrency bug that may cause the wrong value to be loaded
      when a CPU is modifying the maple tree.
      
      CPU1:
      mtree_insert_range()
        mas_insert()
          mas_store_root()
            ...
            mas_root_expand()
              ...
              rcu_assign_pointer(mas->tree->ma_root, mte_mk_root(mas->node));
              ma_set_meta(node, maple_leaf_64, 0, slot);    <---IP
      
      CPU2:
      mtree_load()
        mtree_lookup_walk()
          ma_data_end();
      
      When CPU1 is about to execute the instruction pointed to by IP, the
      ma_data_end() executed by CPU2 may return the wrong end position, which
      will cause the value loaded by mtree_load() to be wrong.
      
      An example of triggering the bug:
      
      Add mdelay(100) between rcu_assign_pointer() and ma_set_meta() in
      mas_root_expand().
      
      static DEFINE_MTREE(tree);
      int work(void *p) {
      	unsigned long val;
      	for (int i = 0 ; i< 30; ++i) {
      		val = (unsigned long)mtree_load(&tree, 8);
      		mdelay(5);
      		pr_info("%lu",val);
      	}
      	return 0;
      }
      
      mt_init_flags(&tree, MT_FLAGS_USE_RCU);
      mtree_insert(&tree, 0, (void*)12345, GFP_KERNEL);
      run_thread(work)
      mtree_insert(&tree, 1, (void*)56789, GFP_KERNEL);
      
      In RCU mode, mtree_load() should always return the value before or after
      the data structure is modified, and in this example mtree_load(&tree, 8)
      may return 56789 which is not expected, it should always return NULL.  Fix
      it by put ma_set_meta() before rcu_assign_pointer().
      
      Link: https://lkml.kernel.org/r/20230314124203.91572-4-zhangpeng.00@bytedance.com
      
      
      Fixes: 54a611b6 ("Maple Tree: add new data structure")
      Signed-off-by: default avatarPeng Zhang <zhangpeng.00@bytedance.com>
      Reviewed-by: default avatarLiam R. Howlett <Liam.Howlett@oracle.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      c45ea315
Loading