On Tue, Jul 09, 2024 at 02:26:05AM +0300, ahmed Ehab wrote: > Hi, > thanks for the great feedback. > > > > > * Last but not the least, could you try to add a test case in > lib/locking-selftest.c to ensure the issue you fixed won't happen > again? This could be tricky, since you will need to fight against > the compiler to generate two string literals with the same content.* > I added a test case, but I am not sure how to run this test file as it > doesn't seem to be part of the kselftests. I compiled it successfully by > setting the lock debugging option but couldn't get it to run. > My usual approach is: set CONFIG_DEBUG_LOCKING_API_SELFTESTS=y, compile a kernel, and boot it in qemu, the test result will show in serial log of the VM. Hope that helps. Regards, Boqun > Regards, > Ahmed > > On Thu, Jul 4, 2024 at 1:46 AM Boqun Feng <boqun.feng@xxxxxxxxx> wrote: > > > Hi, > > > > On Thu, Jul 04, 2024 at 03:32:24AM +0300, botta633 wrote: > > > Preventing Lockdep_set_subclass from creating a new instance of the > > > string literal. Hence, we will always have the class->name. This > > > prevents kernel panics when locking up a lock class while comparing > > > class locks and class names. > > > > Good catch! Thanks. > > > > > > > > > > > > Please remove the extra blank line here. > > > > > Signed-off-by: botta633 <bottaawesome633@xxxxxxxxx> > > > > Do you mind putting your real name here? Besides, IIUC, this is fixing: > > > > https://syzkaller.appspot.com/bug?extid=7f4a6f7f7051474e40ad > > > > > > , right? If so, there are some more things: > > > > * Copy ext4 and syzkaller people, so that you could get more > > tests. > > > > * Since this is a bug fix, could you please figure out which > > commit introduces the issue, so that you can put a correct > > "Fixes:" tag along with your signed-off-by? > > > > * Since the issue was reported by syzkaller, you should put their > > "Reported-by" tag, they have an example in the website I paste > > above. > > > > * Please also Cc stable mail list so that the fix can be > > backported, you can find the information on "Cc: stable" tag at: > > > > https://docs.kernel.org/process/stable-kernel-rules.html > > > > * Last but not the least, could you try to add a test case in > > lib/locking-selftest.c to ensure the issue you fixed won't > > happen again? This could be tricky, since you will need to fight > > against the compiler to generate two string literals with the > > same content. > > > > [Cc ext4 and syzkaller] > > > > Regards, > > Boqun > > > > > --- > > > 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 > > > > >
