Re: [PATCH] debugfs: fix potential infinite loop in debugfs_remove_recursive

[yukuai (C)]

On 2019/11/14 10:43, Steven Rostedt wrote:
> On Thu, 14 Nov 2019 10:01:23 +0800
> "yukuai (C)" <yukuai3@xxxxxxxxxx> wrote:
>
>
> > Do you agree with that list_empty(&chile->d_subdirs) here is not
> > appropriate? Since it can't skip the subdirs that is not
> > simple_positive(simple_positive() will return false), which is the
> > reason of infinite loop.
>
> I do agree that simple_empty() is wrong, for the reasons you pointed out.
>
> > > > +		if (!simple_empty(child)) {
> > >
> > > Have you tried this with lockdep enabled? I'm thinking that you might
> > > get a splat with holding parent->d_lock and simple_empty(child) taking
> > > the child->d_lock.
> > The locks are taken and released in the right order:
> > take parent->d_lock
> > 	take child->d_lock
> > 		list_for_each_entry(c, &child->d_sundirs, d_child)
> > 			take c->d_lock
> > 			release c->d_lock
> > 	release child->d_lock
> > release parent->d_lock
> > I don't see anything wrong, am I missing something?
>
> It should be fine, my worry is that we may be missing a lockdep
> annotation, that might confuse lockdep, as lockdep may see this as the
> same type of lock being taken, and wont know the order.
>
> Have you tried this patch with lockdep enabled and tried to hit this
> code path?
I haven't tried yet. I'll try soon and show the result.
Thanks
Yu Kuai
> -- Steve
>
> .