Linux-Modules Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH RFC 0/3] Proposal for cycles handling
  2016-11-11 11:43         ` [PATCH RFC 0/3] Proposal for cycles handling Yauheni Kaliuta
@ 2017-02-13  8:32 Lucas De Marchi
  2017-02-20 14:18 ` [PATCH RFC v2 0/2] " Yauheni Kaliuta
  2 siblings, 1 reply; 23+ messages in thread
From: Lucas De Marchi @ 2017-02-13  8:32 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: linux-modules, Mian Yousaf Kaukab, bjorn.andersson, Jessica Yu

On Fri, Nov 11, 2016 at 3:43 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Back to the discussion, does it make any sense?
>
> This is an RFC proposal of dependency loops reporting.
>
> Yauheni Kaliuta (3):
>   testsuite: depmod: check netsted loops reporting
>   libkmod: list: export list handling functions
>   depmod: handle nested loops

I added some comments. It's looking good and I'd like to get this in
to do a release this week.

thanks for fixing this.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH RFC 2/3] libkmod: list: export list handling functions
  2017-02-20 14:22               ` Yauheni Kaliuta
@ 2017-02-22  5:26 Lucas De Marchi
  2017-02-22  9:41 ` [PATCH v3 0/2] Proposal for cycles handling Yauheni Kaliuta
  -1 siblings, 1 reply; 23+ messages in thread
From: Lucas De Marchi @ 2017-02-22  5:26 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: linux-modules, Mian Yousaf Kaukab, bjorn.andersson

On Mon, Feb 20, 2017 at 6:22 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Hi, Lucas!
>
>>>>>> On Mon, 13 Feb 2017 00:05:07 -0800, Lucas De Marchi  wrote:
>
>  > On Fri, Nov 11, 2016 at 3:43 AM, Yauheni Kaliuta
>  > <yauheni.kaliuta@redhat.com> wrote:
>  >> The library uses list functions to create lists internally and
>  >> provides to the clients immutable lists and only functions to
>  >> traverse them.
>  >>
>  >> It may be useful to create own lists in the kmod utilities, so
>  >> export functions for lists creation as well (as it's done for
>  >> arrays). The following functions affected (needed for the
>  >> following depmod modifications):
>  >>
>  >> kmod_list_append()
>  >> kmod_list_remove()
>  >> kmod_list_remove_data()
>  >>
>  >> The patch also adds kmod_list_data() accessor to keep the
>  >> struct kmod_list opaque.
>
>  > I don't see a use for these functions outside of the tools we provide.
>  > We don't need to export them (to external users) in order to use for
>  > our tools.  You can just make the tools include libkmod-internal.h and
>  > use that.
>
>  > In fact since you didn't update the .sym file what will happen is that
>  > external users will see a function in the header but won't be able to
>  > use it since it will not be part of the .so.
>
> Hmm, log macros redefined in this case:
>
>   CC       tools/depmod.o
> In file included from tools/kmod.h:42:0,
>                  from tools/depmod.c:42:
> tools/log.h:31:0: warning: "ERR" redefined
>  #define ERR(...) log_printf(LOG_ERR, __VA_ARGS__)
>
> In file included from tools/depmod.c:40:0:
> ./libkmod/libkmod-internal.h:29:0: note: this is the location of the previous definition
>  #  define ERR(ctx, arg...) kmod_log_cond(ctx, LOG_ERR, ## arg)
>
>
> Would include libkmod-internal.h, then undef then, then include kmod.h be
> acceptable?

This is the approach taken by testsuite. So, yes, it's fine for now.

Since we always include with order system headers, lib headers, local
headers, we could even make the undef on tools/log.h, so inside the
tools we use our local logging macros.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH v1 1/2] testsuite: depmod: add module dependency outside cyclic chain
@ 2016-11-08 16:45 Mian Yousaf Kaukab
  2016-11-08 16:45 ` [PATCH v1 2/2] depmod: ignore related modules in depmod_report_cycles Mian Yousaf Kaukab
  2016-11-09  0:29 ` [PATCH v1 1/2] testsuite: depmod: add module dependency outside cyclic chain Lucas De Marchi
  0 siblings, 2 replies; 23+ messages in thread
From: Mian Yousaf Kaukab @ 2016-11-08 16:45 UTC (permalink / raw)
  To: linux-modules, lucas.demarchi
  Cc: bjorn.andersson, afaerber, Mian Yousaf Kaukab

Check that depmod do not report modules outside cyclic chain

Two modules f and g are added which do not have any dependency.
modules a and b are made dependent on f and g.

Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
---
Here is the output of loop dependency check test after adding this
patch:

ESTSUITE: ERR: wrong:
depmod: ERROR: Found 7 modules in dependency cycles!
depmod: ERROR: Cycle detected: mod_loop_d -> mod_loop_e -> mod_loop_d 
depmod: ERROR: Cycle detected: mod_loop_b -> mod_loop_c -> mod_loop_a -> mod_loop_b 
depmod: ERROR: Cycle detected: mod_loop_b -> mod_loop_c -> mod_loop_a -> mod_loop_g 
depmod: ERROR: Cycle detected: mod_loop_b -> mod_loop_c -> mod_loop_a -> mod_loop_f 

Buffer overflow occurs in the loop when last two lines are printed.
43 bytes buffer is allocated and 53 bytes are used.

 testsuite/module-playground/Makefile     |  6 +++++-
 testsuite/module-playground/mod-loop-a.c |  2 ++
 testsuite/module-playground/mod-loop-b.c |  2 ++
 testsuite/module-playground/mod-loop-f.c | 24 ++++++++++++++++++++++++
 testsuite/module-playground/mod-loop-g.c | 24 ++++++++++++++++++++++++
 testsuite/module-playground/mod-loop.h   |  2 ++
 testsuite/populate-modules.sh            |  2 ++
 7 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 testsuite/module-playground/mod-loop-f.c
 create mode 100644 testsuite/module-playground/mod-loop-g.c

diff --git a/testsuite/module-playground/Makefile b/testsuite/module-playground/Makefile
index a5f142f..bf364a9 100644
--- a/testsuite/module-playground/Makefile
+++ b/testsuite/module-playground/Makefile
@@ -12,13 +12,17 @@ obj-m += mod-foo-c.o
 obj-m += mod-foo.o
 
 # mod-loop: create loops in dependencies:
-# 1) mod-loop-a -> mod-loop-b -> mod-loop-c -> mod-loop-a
+# 1) mod-loop-a  ->  mod-loop-b -> mod-loop-c -> mod-loop-a
+#     |-> mod-loop-f    |-> mod-loop-f
+#     \-> mod-loop-g    \-> mod-loop-g
 # 2) mod-loop-d -> mod-loop-e -> mod-loop-d
 obj-m += mod-loop-a.o
 obj-m += mod-loop-b.o
 obj-m += mod-loop-c.o
 obj-m += mod-loop-d.o
 obj-m += mod-loop-e.o
+obj-m += mod-loop-f.o
+obj-m += mod-loop-g.o
 
 # mod-fake-*: fake the respective modules in kernel with these aliases. Aliases
 # list was taken from 3.5.4
diff --git a/testsuite/module-playground/mod-loop-a.c b/testsuite/module-playground/mod-loop-a.c
index e1fd0ce..e5adb49 100644
--- a/testsuite/module-playground/mod-loop-a.c
+++ b/testsuite/module-playground/mod-loop-a.c
@@ -10,6 +10,8 @@ static int __init test_module_init(void)
 {
 	printA();
 	printB();
+	printF();
+	printG();
 
 	return 0;
 }
diff --git a/testsuite/module-playground/mod-loop-b.c b/testsuite/module-playground/mod-loop-b.c
index f4490b7..26232ea 100644
--- a/testsuite/module-playground/mod-loop-b.c
+++ b/testsuite/module-playground/mod-loop-b.c
@@ -10,6 +10,8 @@ static int __init test_module_init(void)
 {
 	printB();
 	printC();
+	printF();
+	printG();
 
 	return 0;
 }
diff --git a/testsuite/module-playground/mod-loop-f.c b/testsuite/module-playground/mod-loop-f.c
new file mode 100644
index 0000000..0abb161
--- /dev/null
+++ b/testsuite/module-playground/mod-loop-f.c
@@ -0,0 +1,24 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+
+#include "mod-loop.h"
+
+static int __init test_module_init(void)
+{
+	printF();
+
+	return 0;
+}
+module_init(test_module_init);
+
+void printF(void)
+{
+	pr_warn("Hello, world F\n");
+}
+EXPORT_SYMBOL(printF);
+
+MODULE_AUTHOR("Lucas De Marchi <lucas.demarchi@intel.com>");
+MODULE_LICENSE("LGPL");
diff --git a/testsuite/module-playground/mod-loop-g.c b/testsuite/module-playground/mod-loop-g.c
new file mode 100644
index 0000000..0965d76
--- /dev/null
+++ b/testsuite/module-playground/mod-loop-g.c
@@ -0,0 +1,24 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+
+#include "mod-loop.h"
+
+static int __init test_module_init(void)
+{
+	printG();
+
+	return 0;
+}
+module_init(test_module_init);
+
+void printG(void)
+{
+	pr_warn("Hello, world G\n");
+}
+EXPORT_SYMBOL(printG);
+
+MODULE_AUTHOR("Lucas De Marchi <lucas.demarchi@intel.com>");
+MODULE_LICENSE("LGPL");
diff --git a/testsuite/module-playground/mod-loop.h b/testsuite/module-playground/mod-loop.h
index 3244ad9..4da3e67 100644
--- a/testsuite/module-playground/mod-loop.h
+++ b/testsuite/module-playground/mod-loop.h
@@ -5,3 +5,5 @@ void printB(void);
 void printC(void);
 void printD(void);
 void printE(void);
+void printF(void);
+void printG(void);
diff --git a/testsuite/populate-modules.sh b/testsuite/populate-modules.sh
index 409a6de..ba1f842 100755
--- a/testsuite/populate-modules.sh
+++ b/testsuite/populate-modules.sh
@@ -16,6 +16,8 @@ map=(
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-c.ko"]="mod-loop-c.ko"
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-d.ko"]="mod-loop-d.ko"
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-e.ko"]="mod-loop-e.ko"
+    ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-f.ko"]="mod-loop-f.ko"
+    ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-g.ko"]="mod-loop-g.ko"
     ["test-dependencies/lib/modules/4.0.20-kmod/kernel/fs/foo/"]="mod-foo-b.ko"
     ["test-dependencies/lib/modules/4.0.20-kmod/kernel/"]="mod-foo-c.ko"
     ["test-dependencies/lib/modules/4.0.20-kmod/kernel/lib/"]="mod-foo-a.ko"
-- 
1.8.5.6


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, back to index

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13  8:32 [PATCH RFC 0/3] Proposal for cycles handling Lucas De Marchi
2017-02-20 14:18 ` [PATCH RFC v2 0/2] " Yauheni Kaliuta
2017-02-20 14:18   ` [PATCH RFC v2 1/2] testsuite: depmod: check netsted loops reporting Yauheni Kaliuta
2017-02-20 14:19   ` [PATCH RFC v2 2/2] depmod: handle nested loops Yauheni Kaliuta
  -- strict thread matches above, loose matches on Subject: below --
2017-02-22  5:26 [PATCH RFC 2/3] libkmod: list: export list handling functions Lucas De Marchi
2017-02-22  9:41 ` [PATCH v3 0/2] Proposal for cycles handling Yauheni Kaliuta
2017-02-22  9:41   ` [PATCH v3 1/2] testsuite: depmod: check netsted loops reporting Yauheni Kaliuta
2017-02-22  9:41   ` [PATCH v3 2/2] depmod: handle nested loops Yauheni Kaliuta
2017-02-23 22:30     ` Lucas De Marchi
2016-11-08 16:45 [PATCH v1 1/2] testsuite: depmod: add module dependency outside cyclic chain Mian Yousaf Kaukab
2016-11-08 16:45 ` [PATCH v1 2/2] depmod: ignore related modules in depmod_report_cycles Mian Yousaf Kaukab
2016-11-09  0:40   ` Lucas De Marchi
2016-11-09  2:59     ` Yauheni Kaliuta
2016-11-09  9:17       ` Mian Yousaf Kaukab
2016-11-09 11:23         ` Yauheni Kaliuta
2016-11-11 11:43         ` [PATCH RFC 0/3] Proposal for cycles handling Yauheni Kaliuta
2016-11-11 11:43           ` [PATCH RFC 2/3] libkmod: list: export list handling functions Yauheni Kaliuta
2017-02-13  8:05             ` Lucas De Marchi
2017-02-20 14:22               ` Yauheni Kaliuta
2016-11-11 11:43           ` [PATCH RFC 3/3] depmod: handle nested loops Yauheni Kaliuta
2017-02-13  8:30             ` Lucas De Marchi
2017-02-20 14:16               ` Yauheni Kaliuta
2017-02-13  8:16           ` [PATCH RFC 0/3] Proposal for cycles handling Lucas De Marchi
2017-02-13  9:56             ` Yauheni Kaliuta
2016-11-09  0:29 ` [PATCH v1 1/2] testsuite: depmod: add module dependency outside cyclic chain Lucas De Marchi

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox