| Message ID | 20240714051427.114044-2-bottaawesome633@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] ext4: Forcing subclasses to have same name pointer as their parent class | expand |
On Sun, Jul 14, 2024 at 08:14:27AM +0300, botta633 wrote: First, the subsystem tag also needs to be "locking/lockdep" or "lockdep". > Checking if the lockdep_map->name will change when setting the subclass. > It shouldn't change so that the lock class and subclass will have the same name > Could you make the commit log wrapped at 75 columns? > > Reported-by: <syzbot+7f4a6f7f7051474e40ad@syzkaller.appspotmail.com> > Fixes: fd5e3f5fe27 > Cc: <stable@vger.kernel.org> Since this is only adding test for a bug fix, you don't need to put these tags here. > Signed-off-by: botta633 <bottaawesome633@gmail.com> Again, could you please put your name here? Also seems that you send two patch #2, one is with the proper version number "v2", but not in-reply-to the patch #1, the other doesn't have the correct version number but has the correct "in-reply-to" field. Could you use the correct version number and "in-reply-to" next time? > --- > lib/locking-selftest.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c > index 6f6a5fc85b42..1d7885205f36 100644 > --- a/lib/locking-selftest.c > +++ b/lib/locking-selftest.c > @@ -2710,12 +2710,24 @@ static void local_lock_3B(void) > > } > > +static void class_subclass_X1_name(void) > +{ > + const char *name_before_subclass = rwsem_X1.dep_map.name; > + const char *name_after_subclass; > + > + WARN_ON(!rwsem_X1.dep_map.name); > + lockdep_set_subclass(&rwsem_X1, 1); > + WARN_ON(name_before_subclass != name_after_subclass); Could you add some comments here explaining your test? For example, where name_after_subclass gets set? > +} > + > static void local_lock_tests(void) > { Please don't add this test into another test, you could directly call your class_subclass_X1_name() (maybe rename it to *_test()) in lockding_selftest() function. Besides, make sure you run the test with and without your modification in patch #1, and confirm this is the test that could verify your fix. Regards, Boqun > printk(" --------------------------------------------------------------------------\n"); > printk(" | local_lock tests |\n"); > printk(" ---------------------\n"); > > + init_class_X(&lock_X1, &rwlock_X1, &mutex_X1, &rwsem_X1); > + > print_testname("local_lock inversion 2"); > dotest(local_lock_2, SUCCESS, LOCKTYPE_LL); > pr_cont("\n"); > @@ -2727,6 +2739,10 @@ static void local_lock_tests(void) > print_testname("local_lock inversion 3B"); > dotest(local_lock_3B, FAILURE, LOCKTYPE_LL); > pr_cont("\n"); > + > + print_testname("Class and subclass"); > + dotest(class_subclass_X1_name, SUCCESS, LOCKTYPE_RWSEM); > + pr_cont("\n"); > } > > static void hardirq_deadlock_softirq_not_deadlock(void) > -- > 2.45.2 >
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c index 6f6a5fc85b42..1d7885205f36 100644 --- a/lib/locking-selftest.c +++ b/lib/locking-selftest.c @@ -2710,12 +2710,24 @@ static void local_lock_3B(void) } +static void class_subclass_X1_name(void) +{ + const char *name_before_subclass = rwsem_X1.dep_map.name; + const char *name_after_subclass; + + WARN_ON(!rwsem_X1.dep_map.name); + lockdep_set_subclass(&rwsem_X1, 1); + WARN_ON(name_before_subclass != name_after_subclass); +} + static void local_lock_tests(void) { printk(" --------------------------------------------------------------------------\n"); printk(" | local_lock tests |\n"); printk(" ---------------------\n"); + init_class_X(&lock_X1, &rwlock_X1, &mutex_X1, &rwsem_X1); + print_testname("local_lock inversion 2"); dotest(local_lock_2, SUCCESS, LOCKTYPE_LL); pr_cont("\n"); @@ -2727,6 +2739,10 @@ static void local_lock_tests(void) print_testname("local_lock inversion 3B"); dotest(local_lock_3B, FAILURE, LOCKTYPE_LL); pr_cont("\n"); + + print_testname("Class and subclass"); + dotest(class_subclass_X1_name, SUCCESS, LOCKTYPE_RWSEM); + pr_cont("\n"); } static void hardirq_deadlock_softirq_not_deadlock(void)
Checking if the lockdep_map->name will change when setting the subclass. It shouldn't change so that the lock class and subclass will have the same name Reported-by: <syzbot+7f4a6f7f7051474e40ad@syzkaller.appspotmail.com> Fixes: fd5e3f5fe27 Cc: <stable@vger.kernel.org> Signed-off-by: botta633 <bottaawesome633@gmail.com> --- lib/locking-selftest.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)