Re: [PATCH] depmod: ignore related modules in depmod_report_cycles

[Mian Yousaf Kaukab <yousaf.kaukab@suse.com>]

On Sat, 2016-11-05 at 21:50 -0200, Lucas De Marchi wrote:
> Hi,
> 
> On Fri, Nov 4, 2016 at 7:33 AM, Mian Yousaf Kaukab
> <yousaf.kaukab@suse.com> wrote:
> > 
> > Only print actual cyclic dependencies. Don't print count of all the
> > modules as it includes other modules which have dependencies but
> > not
> > necessarily cyclic.
> > 
> > Printing related modules causes buffer overflow as m->modnamesz is
> > not
> > included in buffer size calculations (loop == m is never true).
> > This buffer overflow causes kmod to crash.
> > 
> > Reported-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
> > ---
> > As count of modules in cyclic dependency chain is not known at the
> > start of this function, so it is not printed anymore. The output
> > when cyclic dependency is detected is changed as following:
> > 
> > Old:
> > depmod: ERROR: Found 8 modules in dependency cycles!
> > 
> > New:
> > depmod: ERROR: Modules found in dependency cycles!
> 
> I think it would be good to fix this problem, but retaining the
> behavior.
Would it be OK if the modules names are printed first and count is
printed afterward. Something like following: 

DEPMOD  4.9.0-rc4-default
depmod: ERROR: Cycle detected: qcom_wcnss_iris -> qcom_wcnss ->
qcom_wcnss_iris
depmod: ERROR: Found 2 modules in dependency cycles!
Makefile:1227: recipe for target '_modinst_post' failed

In this way modules names can still be printed in the loop and then the
exact count of modules involved in cyclic dependency is printed at the
end of the depmod_report_cycles().

> 
> My first reaction to this is "how do I reproduce the issue?". This
> number so far does tell us how many modules are involved in loops. 
There is more info at the following link:
https://bugzilla.opensuse.org/show_bug.cgi?id=1008186

You can reproduce the issue by enabling CONFIG_QCOM_WCNSS_PIL as module
in v4.9-rc4 for arm64. 
+CONFIG_REMOTEPROC=m
+CONFIG_QCOM_MDT_LOADER=m
-# CONFIG_QCOM_WCNSS_PIL is not set
+CONFIG_QCOM_WCNSS_IRIS=m
+CONFIG_QCOM_WCNSS_PIL=m

Issue will reproduce when depmod is run at the end of modules_install.

> So,
> for example:
> 
> A -> B -> D -> A
> B -> C -> B
C -> X
C -> Y
C -> Z
In this case count becomes 7. Which is misleading.

> This line should point to 4 modules. We don't know which module is
> wrong. But there are 4 modules involved in the loops. D could be the
> culprit.
> 
> The function should show what are the loops present (in "first found
> order" - it could very well report as "B -> D -> A -> B" the line
> above).
> 
> We have a testsuite to check if the loop detection is working in
> depmod. See testsuite/test-depmod.c:depmod_detect_loop().
> Would it be possible to add your use case there? [...]
Yes I can take a look.

> [...] If you tell me what
> are your dependency chain and cycle I can take a look.
> 
> If I misunderstood the issue with the total number, then at least the
> testsuite needs to be fixed, because right now it expects that number
> to be in the output (see
> testsuite/rootfs-pristine/test-depmod/detect-loop/correct.txt)
> 
> 
> thanks
> Lucas De Marchi
> 
BR,
Yousaf