[PATCH v5 1/2] locking/lockdep: Avoid creating new name string literals in lockdep_set_subclass()

[PATCH v5 1/2] locking/lockdep: Avoid creating new name string literals in lockdep_set_subclass()

[Ahmed Ehab]

Syzbot reports a problem that a warning will be triggered while
searching a lock class in look_up_lock_class().

The cause of the issue is that a new name is created and used by
lockdep_set_subclass() instead of using the existing one. This results
in two lock classes with the same key but different name pointers and a
WARN_ONCE() is triggered because of that in look_up_lock_class().

To fix this, change lockdep_set_subclass() to use the existing name
instead of a new one. Hence, no new name will be created by
lockdep_set_subclass(). Hence, the warning is avoided.

Reported-by: <syzbot+7f4a6f7f7051474e40ad@syzkaller.appspotmail.com>
Fixes: de8f5e4f2dc1f ("lockdep: Introduce wait-type checks")
Cc: <stable@vger.kernel.org>
Signed-off-by: Ahmed Ehab <bottaawesome633@gmail.com>
---
v4->v5:
    - Changed the subject
    - Changed the changelog to be more detailed

 include/linux/lockdep.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 08b0d1d9d78b..df8fa5929de7 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -173,7 +173,7 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name,
 			      (lock)->dep_map.lock_type)
 
 #define lockdep_set_subclass(lock, sub)					\
-	lockdep_init_map_type(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
+	lockdep_init_map_type(&(lock)->dep_map, (lock)->dep_map.name, (lock)->dep_map.key, sub,\
 			      (lock)->dep_map.wait_type_inner,		\
 			      (lock)->dep_map.wait_type_outer,		\
 			      (lock)->dep_map.lock_type)
-- 
2.45.2

[PATCH v6 2/2] locking/lockdep: Test no new string literal is created in lockdep_set_subclass()

[Ahmed Ehab]

Add a test case to ensure that no new name string literal will be
created in lockdep_set_subclass(), otherwise a warning will be triggered
in look_up_lock_class(). Add this to catch the problem in the future. 

Signed-off-by: Ahmed Ehab <bottaawesome633@gmail.com>
---
v5->v6:
- Changed the subject and changelog to be in imperative format

 lib/locking-selftest.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 6f6a5fc85b42..2b4650bdf833 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -2710,6 +2710,32 @@ static void local_lock_3B(void)
 
 }
 
+static void lock_class_subclass_X1(void)
+{
+	const char *name_before_setting_subclass = rwsem_X1.dep_map.name;
+	const char *name_after_setting_subclass;
+
+	lockdep_set_subclass(&rwsem_X1, 1);
+	name_after_setting_subclass = rwsem_X1.dep_map.name;
+	DEBUG_LOCKS_WARN_ON(name_before_setting_subclass != name_after_setting_subclass);
+}
+
+/*
+ * after setting the subclass the lockdep_map.name changes
+ * if we initialize a new string literal for the subclass
+ * we will have a new name pointer
+ */
+static void class_subclass_X1_name_test(void)
+{
+	printk("  --------------------------------------------------------------------------\n");
+	printk("  | class and subclass name test|\n");
+	printk("  ---------------------\n");
+
+	print_testname("lock class and subclass same name");
+	dotest(lock_class_subclass_X1, SUCCESS, LOCKTYPE_RWSEM);
+	pr_cont("\n");
+}
+
 static void local_lock_tests(void)
 {
 	printk("  --------------------------------------------------------------------------\n");
@@ -2920,6 +2946,8 @@ void locking_selftest(void)
 	dotest(hardirq_deadlock_softirq_not_deadlock, FAILURE, LOCKTYPE_SPECIAL);
 	pr_cont("\n");
 
+	class_subclass_X1_name_test();
+
 	if (unexpected_testcase_failures) {
 		printk("-----------------------------------------------------------------\n");
 		debug_locks = 0;
-- 
2.45.2

Re: [PATCH v6 2/2] locking/lockdep: Test no new string literal is created in lockdep_set_subclass()

[kernel test robot]

Hi Ahmed,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/locking/core]
[also build test ERROR on tip/master arm-perf/for-next/perf linus/master tip/auto-latest v6.11-rc5 next-20240826]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ahmed-Ehab/locking-lockdep-Test-no-new-string-literal-is-created-in-lockdep_set_subclass/20240826-145215
base:   tip/locking/core
patch link:    https://lore.kernel.org/r/20240824221031.7751-2-bottaawesome633%40gmail.com
patch subject: [PATCH v6 2/2] locking/lockdep: Test no new string literal is created in lockdep_set_subclass()
config: i386-buildonly-randconfig-003-20240827 (https://download.01.org/0day-ci/archive/20240827/202408270559.rym5UAv9-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240827/202408270559.rym5UAv9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408270559.rym5UAv9-lkp@intel.com/

All errors (new ones prefixed by >>):

   lib/locking-selftest.c: In function 'lock_class_subclass_X1':
>> lib/locking-selftest.c:2715:60: error: 'struct rw_semaphore' has no member named 'dep_map'
    2715 |         const char *name_before_setting_subclass = rwsem_X1.dep_map.name;
         |                                                            ^
   lib/locking-selftest.c:2719:47: error: 'struct rw_semaphore' has no member named 'dep_map'
    2719 |         name_after_setting_subclass = rwsem_X1.dep_map.name;
         |                                               ^


vim +2715 lib/locking-selftest.c

  2712	
  2713	static void lock_class_subclass_X1(void)
  2714	{
> 2715		const char *name_before_setting_subclass = rwsem_X1.dep_map.name;
  2716		const char *name_after_setting_subclass;
  2717	
  2718		lockdep_set_subclass(&rwsem_X1, 1);
  2719		name_after_setting_subclass = rwsem_X1.dep_map.name;
  2720		DEBUG_LOCKS_WARN_ON(name_before_setting_subclass != name_after_setting_subclass);
  2721	}
  2722	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v6 2/2] locking/lockdep: Test no new string literal is created in lockdep_set_subclass()

[Boqun Feng]

On Tue, Aug 27, 2024 at 01:04:15PM +0300, ahmed Ehab wrote:
> On Tue, Aug 27, 2024 at 1:08 AM kernel test robot <lkp@intel.com> wrote:
> 
> > Hi Ahmed,
> >
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on tip/locking/core]
> > [also build test ERROR on tip/master arm-perf/for-next/perf linus/master
> > tip/auto-latest v6.11-rc5 next-20240826]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:
> > https://github.com/intel-lab-lkp/linux/commits/Ahmed-Ehab/locking-lockdep-Test-no-new-string-literal-is-created-in-lockdep_set_subclass/20240826-145215
> > base:   tip/locking/core
> > patch link:
> > https://lore.kernel.org/r/20240824221031.7751-2-bottaawesome633%40gmail.com
> > patch subject: [PATCH v6 2/2] locking/lockdep: Test no new string literal
> > is created in lockdep_set_subclass()
> > config: i386-buildonly-randconfig-003-20240827 (
> > https://download.01.org/0day-ci/archive/20240827/202408270559.rym5UAv9-lkp@intel.com/config
> > )
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (
> > https://download.01.org/0day-ci/archive/20240827/202408270559.rym5UAv9-lkp@intel.com/reproduce
> > )
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new
> > version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes:
> > https://lore.kernel.org/oe-kbuild-all/202408270559.rym5UAv9-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> >    lib/locking-selftest.c: In function 'lock_class_subclass_X1':
> > >> lib/locking-selftest.c:2715:60: error: 'struct rw_semaphore' has no
> > member named 'dep_map'
> >     2715 |         const char *name_before_setting_subclass =
> > rwsem_X1.dep_map.name;
> >          |                                                            ^
> >    lib/locking-selftest.c:2719:47: error: 'struct rw_semaphore' has no
> > member named 'dep_map'
> >     2719 |         name_after_setting_subclass = rwsem_X1.dep_map.name;
> >          |                                               ^
> >
> These failures will only happen if CONFIG_DEBUG_LOCK_ALLOC is not set,
> which is required for lockdep I think.

Oh yes, this test can be compiled with LOCKDEP=n.

> Should I consider these errors by checking CONFIG_DEBUG_LOCK_ALLOC before
> calling my test method or is it
> irrelevant?
> 

Could you introduce a helper function here?

	#if CONFIG_DEBUG_LOCK_ALLOC
	static inline char* rw_semaphore_lockdep_name(struct rw_semaphore *rwsem)
	{
		return rwsem.dep_map.name;
	}
	#else
	static inline char* rw_semaphore_lockdep_name(struct rw_semaphore *rwsem)
	{
		return NULL;
	}
	#endif

and use it in your test case (you put the function definitio right
before the test case.

Regards,
Boqun


> >
> >
> > vim +2715 lib/locking-selftest.c
> >
> >   2712
> >   2713  static void lock_class_subclass_X1(void)
> >   2714  {
> > > 2715          const char *name_before_setting_subclass =
> > rwsem_X1.dep_map.name;
> >   2716          const char *name_after_setting_subclass;
> >   2717
> >   2718          lockdep_set_subclass(&rwsem_X1, 1);
> >   2719          name_after_setting_subclass = rwsem_X1.dep_map.name;
> >   2720          DEBUG_LOCKS_WARN_ON(name_before_setting_subclass !=
> > name_after_setting_subclass);
> >   2721  }
> >   2722
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
> 
> 
> Regards,
> Ahmed