[PATCH RFC 0/3] depmod: implement external directories support

[PATCH RFC 0/3] depmod: implement external directories support

[Yauheni Kaliuta]

Would it be possible to implement something like external
directories support in depmod (see PATCH 3 description).

I'll copy here the part:

This is a pretty simple extention of existing logic, since now
depmod already is able to:

a) scan modules with full path from command line without -a
switch;
b) detects broken symbol dependencies and broken modversions,
what assumes, that modules are already are not built for the
existing kernel.

Yauheni Kaliuta (3):
  depmod: create depmod dir independent search function
  depmod: search key: move builtin detection under the add function
  depmod: implement external directories support

 tools/depmod.c | 209 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 175 insertions(+), 34 deletions(-)

-- 
2.9.3

[PATCH RFC 1/3] depmod: create depmod dir independent search function

[Yauheni Kaliuta]

Prepare to implement external directories support.

The patch splits depmod_modules_search() function to two
functions: depmod_modules_search(), called by the high level with
intention to search all possible modules, and
depmod_module_search_path(), which takes path as a parameter and
scans modules under the path only. Initially it is used to scan
the same depmod->cfg->dirname path only.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index f2b370f146bb..a96501d1cbfb 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1186,29 +1186,42 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 	return err;
 }
 
-static int depmod_modules_search(struct depmod *depmod)
+static int depmod_modules_search_path(struct depmod *depmod,
+				      const char *path)
 {
-	char path[PATH_MAX];
-	DIR *d = opendir(depmod->cfg->dirname);
+	char path_buf[PATH_MAX];
+	DIR *d;
 	size_t baselen;
 	int err;
+
+	d = opendir(path);
 	if (d == NULL) {
 		err = -errno;
-		ERR("could not open directory %s: %m\n", depmod->cfg->dirname);
+		ERR("could not open directory %s: %m\n", path);
 		return err;
 	}
 
-	baselen = depmod->cfg->dirnamelen;
-	memcpy(path, depmod->cfg->dirname, baselen);
-	path[baselen] = '/';
+	baselen = strlen(path);
+	memcpy(path_buf, path, baselen);
+	path_buf[baselen] = '/';
 	baselen++;
-	path[baselen] = '\0';
+	path_buf[baselen] = '\0';
 
-	err = depmod_modules_search_dir(depmod, d, baselen, path);
+	err = depmod_modules_search_dir(depmod, d, baselen, path_buf);
 	closedir(d);
 	return err;
 }
 
+static int depmod_modules_search(struct depmod *depmod)
+{
+	int err;
+
+	err = depmod_modules_search_path(depmod, depmod->cfg->dirname);
+	if (err < 0)
+		return err;
+	return 0;
+}
+
 static int mod_cmp(const void *pa, const void *pb) {
 	const struct mod *a = *(const struct mod **)pa;
 	const struct mod *b = *(const struct mod **)pb;
-- 
2.9.3

Re: [PATCH RFC 1/3] depmod: create depmod dir independent search function

[Lucas De Marchi]

On Wed, Nov 23, 2016 at 7:23 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Prepare to implement external directories support.
>
> The patch splits depmod_modules_search() function to two
> functions: depmod_modules_search(), called by the high level with
> intention to search all possible modules, and
> depmod_module_search_path(), which takes path as a parameter and
> scans modules under the path only. Initially it is used to scan
> the same depmod->cfg->dirname path only.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
>  tools/depmod.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/tools/depmod.c b/tools/depmod.c
> index f2b370f146bb..a96501d1cbfb 100644
> --- a/tools/depmod.c
> +++ b/tools/depmod.c
> @@ -1186,29 +1186,42 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
>         return err;
>  }
>
> -static int depmod_modules_search(struct depmod *depmod)
> +static int depmod_modules_search_path(struct depmod *depmod,
> +                                     const char *path)
>  {
> -       char path[PATH_MAX];
> -       DIR *d = opendir(depmod->cfg->dirname);
> +       char path_buf[PATH_MAX];

not required for this, but since now the path can be anywhere, this is
more prone to failures and could be a good candidate to use
scratchbuf.


> +       DIR *d;
>         size_t baselen;
>         int err;
> +
> +       d = opendir(path);
>         if (d == NULL) {
>                 err = -errno;
> -               ERR("could not open directory %s: %m\n", depmod->cfg->dirname);
> +               ERR("could not open directory %s: %m\n", path);
>                 return err;
>         }
>
> -       baselen = depmod->cfg->dirnamelen;
> -       memcpy(path, depmod->cfg->dirname, baselen);
> -       path[baselen] = '/';
> +       baselen = strlen(path);
> +       memcpy(path_buf, path, baselen);
> +       path_buf[baselen] = '/';
>         baselen++;
> -       path[baselen] = '\0';
> +       path_buf[baselen] = '\0';
>
> -       err = depmod_modules_search_dir(depmod, d, baselen, path);
> +       err = depmod_modules_search_dir(depmod, d, baselen, path_buf);
>         closedir(d);
>         return err;
>  }
>
> +static int depmod_modules_search(struct depmod *depmod)
> +{
> +       int err;
> +
> +       err = depmod_modules_search_path(depmod, depmod->cfg->dirname);
> +       if (err < 0)
> +               return err;
> +       return 0;
> +}

LGTM


Lucas De Marchi

[PATCH RFC 2/3] depmod: search key: move builtin detection under the add function

[Yauheni Kaliuta]

Prepare to implement external directories support.

It's better to isolated behaviour difference under the
cfg_search_add() call, then make the client code aware of it.

In case of external modules/directories support, there will be
one more keyword added, so making the clients aware of it makes
even less sense.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index a96501d1cbfb..81360a651811 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -452,10 +452,11 @@ struct cfg {
 	struct cfg_search *searches;
 };
 
-static int cfg_search_add(struct cfg *cfg, const char *path, uint8_t builtin)
+static int cfg_search_add(struct cfg *cfg, const char *path)
 {
 	struct cfg_search *s;
 	size_t len;
+	uint8_t builtin = streq(path, CFG_BUILTIN_KEY);
 
 	if (builtin)
 		len = 0;
@@ -568,8 +569,7 @@ static int cfg_file_parse(struct cfg *cfg, const char *filename)
 		if (streq(cmd, "search")) {
 			const char *sp;
 			while ((sp = strtok_r(NULL, "\t ", &saveptr)) != NULL) {
-				uint8_t builtin = streq(sp, CFG_BUILTIN_KEY);
-				cfg_search_add(cfg, sp, builtin);
+				cfg_search_add(cfg, sp);
 			}
 		} else if (streq(cmd, "override")) {
 			const char *modname = strtok_r(NULL, "\t ", &saveptr);
@@ -763,7 +763,7 @@ static int cfg_load(struct cfg *cfg, const char * const *cfg_paths)
 	 * list here. But only if there was no "search" option specified.
 	 */
 	if (cfg->searches == NULL)
-		cfg_search_add(cfg, "updates", 0);
+		cfg_search_add(cfg, "updates");
 
 	return 0;
 }
-- 
2.9.3

Re: [PATCH RFC 2/3] depmod: search key: move builtin detection under the add function

[Lucas De Marchi]

On Wed, Nov 23, 2016 at 7:23 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Prepare to implement external directories support.
>
> It's better to isolated behaviour difference under the
> cfg_search_add() call, then make the client code aware of it.
>
> In case of external modules/directories support, there will be
> one more keyword added, so making the clients aware of it makes
> even less sense.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---


Applied, thanks.


Lucas De Marchi

[PATCH RFC 3/3] depmod: implement external directories support

[Yauheni Kaliuta]

The idea is to add a configuration keyword, external, which
will list directories for scanning for particular kernel version
mask:

external 4.10 /the/modules/dir /second/modules/dir

And extend "search" keyword to set it's priority with pseudo dir
"external" (as it's done for built-in):

search subdir external subdir2 built-in subdir3

(actually, the version is the same as for override keyword: * or
posix regexp, so example above is a bit incorrect).

All other logic left the same: if there are duplicates, only one
is under consideration and it is unloadable if it is bad.

The resulting modules.dep will contain entries a-la:

/the/modules/dir/module1.ko:
kernel/module2.ko: /the/modules/dir/module1.ko

(here /lib/modules/$(uname -r)/kernel/module2.ko depends of
symbols, provided by /the/modules/dir/module1.ko and external has
higher priority).

modprobe and modinfo understand it out of box.

This is a pretty simple extention of existing logic, since now
depmod already is able to:

a) scan modules with full path from command line without -a
switch;
b) detects broken symbol dependencies and broken modversions,
what assumes, that modules are already are not built for the
existing kernel.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 150 insertions(+), 22 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index 81360a651811..5eb22032ead7 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -45,6 +45,7 @@
 static int verbose = DEFAULT_VERBOSE;
 
 static const char CFG_BUILTIN_KEY[] = "built-in";
+static const char CFG_EXTERNAL_KEY[] = "external";
 static const char *default_cfg_paths[] = {
 	"/run/depmod.d",
 	SYSCONFDIR "/depmod.d",
@@ -433,9 +434,21 @@ struct cfg_override {
 	char path[];
 };
 
+enum search_type {
+	SEARCH_PATH,
+	SEARCH_BUILTIN,
+	SEARCH_EXTERNAL
+};
+
 struct cfg_search {
 	struct cfg_search *next;
-	uint8_t builtin;
+	enum search_type type;
+	size_t len;
+	char path[];
+};
+
+struct cfg_external {
+	struct cfg_external *next;
 	size_t len;
 	char path[];
 };
@@ -450,15 +463,27 @@ struct cfg {
 	uint8_t warn_dups;
 	struct cfg_override *overrides;
 	struct cfg_search *searches;
+	struct cfg_external *externals;
 };
 
+static enum search_type cfg_define_search_type(const char *path)
+{
+	if (streq(path, CFG_BUILTIN_KEY))
+		return SEARCH_BUILTIN;
+	if (streq(path, CFG_EXTERNAL_KEY))
+		return SEARCH_EXTERNAL;
+	return SEARCH_PATH;
+}
+
 static int cfg_search_add(struct cfg *cfg, const char *path)
 {
 	struct cfg_search *s;
 	size_t len;
-	uint8_t builtin = streq(path, CFG_BUILTIN_KEY);
+	enum search_type type;
+
+	type = cfg_define_search_type(path);
 
-	if (builtin)
+	if (type != SEARCH_PATH)
 		len = 0;
 	else
 		len = strlen(path) + 1;
@@ -468,15 +493,15 @@ static int cfg_search_add(struct cfg *cfg, const char *path)
 		ERR("search add: out of memory\n");
 		return -ENOMEM;
 	}
-	s->builtin = builtin;
-	if (builtin)
+	s->type = type;
+	if (type != SEARCH_PATH)
 		s->len = 0;
 	else {
 		s->len = len - 1;
 		memcpy(s->path, path, len);
 	}
 
-	DBG("search add: %s, builtin=%hhu\n", path, builtin);
+	DBG("search add: %s, search type=%hhu\n", path, type);
 
 	s->next = cfg->searches;
 	cfg->searches = s;
@@ -524,6 +549,32 @@ static void cfg_override_free(struct cfg_override *o)
 	free(o);
 }
 
+static int cfg_external_add(struct cfg *cfg, const char *path)
+{
+	struct cfg_external *ext;
+	size_t len = strlen(path);
+
+	ext = malloc(sizeof(struct cfg_external) + len + 1);
+	if (ext == NULL) {
+		ERR("external add: out of memory\n");
+		return -ENOMEM;
+	}
+
+	strcpy(ext->path, path);
+	ext->len = len;
+
+	DBG("external add: %s\n", ext->path);
+
+	ext->next = cfg->externals;
+	cfg->externals = ext;
+	return 0;
+}
+
+static void cfg_external_free(struct cfg_external *ext)
+{
+	free(ext);
+}
+
 static int cfg_kernel_matches(const struct cfg *cfg, const char *pattern)
 {
 	regex_t re;
@@ -587,6 +638,20 @@ static int cfg_file_parse(struct cfg *cfg, const char *filename)
 			}
 
 			cfg_override_add(cfg, modname, subdir);
+		} else if (streq(cmd, "external")) {
+			const char *version = strtok_r(NULL, "\t ", &saveptr);
+			const char *dir = strtok_r(NULL, "\t ", &saveptr);
+
+			if (version == NULL || dir == NULL)
+				goto syntax_error;
+
+			if (!cfg_kernel_matches(cfg, version)) {
+				INF("%s:%u: external directory did not match %s\n",
+				    filename, linenum, version);
+				goto done_next;
+			}
+
+			cfg_external_add(cfg, dir);
 		} else if (streq(cmd, "include")
 				|| streq(cmd, "make_map_files")) {
 			INF("%s:%u: command %s not implemented yet\n",
@@ -781,6 +846,12 @@ static void cfg_free(struct cfg *cfg)
 		cfg->searches = cfg->searches->next;
 		cfg_search_free(tmp);
 	}
+
+	while (cfg->externals) {
+		struct cfg_external *tmp = cfg->externals;
+		cfg->externals = cfg->externals->next;
+		cfg_external_free(tmp);
+	}
 }
 
 
@@ -988,6 +1059,33 @@ static int depmod_module_del(struct depmod *depmod, struct mod *mod)
 	return 0;
 }
 
+static const char *search_to_string(const struct cfg_search *s)
+{
+	switch(s->type) {
+	case SEARCH_EXTERNAL:
+		return "external";
+	case SEARCH_BUILTIN:
+		return "built-in";
+	default:
+		return s->path;
+	}
+}
+
+static bool depmod_is_path_starts_with(const char *path,
+				       size_t pathlen,
+				       const char *prefix,
+				       size_t prefix_len)
+{
+	if (pathlen <= prefix_len)
+		return false;
+	if (path[prefix_len] != '/')
+		return false;
+	if (memcmp(path, prefix, prefix_len) != 0)
+		return false;
+
+	return true;
+}
+
 /* returns if existing module @mod is higher priority than newpath.
  * note this is the inverse of module-init-tools is_higher_priority()
  */
@@ -996,6 +1094,7 @@ static int depmod_module_is_higher_priority(const struct depmod *depmod, const s
 	const struct cfg *cfg = depmod->cfg;
 	const struct cfg_override *ov;
 	const struct cfg_search *se;
+	const struct cfg_external *ext;
 
 	/* baselen includes the last '/' and mod->baselen doesn't. So it's
 	 * actually correct to use modnamelen in the first and modnamesz in
@@ -1004,35 +1103,55 @@ static int depmod_module_is_higher_priority(const struct depmod *depmod, const s
 	size_t oldlen = mod->baselen + mod->modnamesz;
 	const char *oldpath = mod->path;
 	int i, bprio = -1, oldprio = -1, newprio = -1;
-
-	assert(strncmp(newpath, cfg->dirname, cfg->dirnamelen) == 0);
-	assert(strncmp(oldpath, cfg->dirname, cfg->dirnamelen) == 0);
-
-	newpath += cfg->dirnamelen + 1;
-	newlen -= cfg->dirnamelen + 1;
-	oldpath += cfg->dirnamelen + 1;
-	oldlen -= cfg->dirnamelen + 1;
+	size_t relnewlen = 0;
+	size_t reloldlen = 0;
+	const char *relnewpath = NULL;
+	const char *reloldpath = NULL;
 
 	DBG("comparing priorities of %s and %s\n",
 	    oldpath, newpath);
 
+	if (strncmp(newpath, cfg->dirname, cfg->dirnamelen) == 0) {
+		relnewpath = newpath + cfg->dirnamelen + 1;
+		relnewlen = newlen - cfg->dirnamelen + 1;
+	}
+	if (strncmp(oldpath, cfg->dirname, cfg->dirnamelen) == 0) {
+		reloldpath = oldpath + cfg->dirnamelen + 1;
+		reloldlen = oldlen - cfg->dirnamelen + 1;
+	}
+
 	for (ov = cfg->overrides; ov != NULL; ov = ov->next) {
 		DBG("override %s\n", ov->path);
-		if (newlen == ov->len && memcmp(ov->path, newpath, newlen) == 0)
+		if (relnewlen == ov->len &&
+		    memcmp(ov->path, relnewpath, relnewlen) == 0)
 			return 0;
-		if (oldlen == ov->len && memcmp(ov->path, oldpath, oldlen) == 0)
+		if (reloldlen == ov->len &&
+		    memcmp(ov->path, reloldpath, reloldlen) == 0)
 			return 1;
 	}
 
 	for (i = 0, se = cfg->searches; se != NULL; se = se->next, i++) {
-		DBG("search %s\n", se->builtin ? "built-in" : se->path);
-		if (se->builtin)
+		DBG("search %s\n", search_to_string(se));
+		if (se->type == SEARCH_BUILTIN)
 			bprio = i;
-		else if (newlen > se->len && newpath[se->len] == '/' &&
-			 memcmp(se->path, newpath, se->len) == 0)
+		else if (se->type == SEARCH_EXTERNAL) {
+			for (ext = cfg->externals; ext != NULL; ext = ext->next, i++) {
+				if (depmod_is_path_starts_with(newpath,
+							       newlen,
+							       ext->path,
+							       ext->len))
+					newprio = i;
+				if (depmod_is_path_starts_with(oldpath,
+							       oldlen,
+							       ext->path,
+							       ext->len))
+					oldprio = i;
+			}
+		} else if (relnewlen > se->len && relnewpath[se->len] == '/' &&
+			 memcmp(se->path, relnewpath, se->len) == 0)
 			newprio = i;
-		else if (oldlen > se->len && oldpath[se->len] == '/' &&
-			 memcmp(se->path, oldpath, se->len) == 0)
+		else if (reloldlen > se->len && reloldpath[se->len] == '/' &&
+			 memcmp(se->path, reloldpath, se->len) == 0)
 			oldprio = i;
 	}
 
@@ -1215,10 +1334,19 @@ static int depmod_modules_search_path(struct depmod *depmod,
 static int depmod_modules_search(struct depmod *depmod)
 {
 	int err;
+	struct cfg_external *ext;
 
 	err = depmod_modules_search_path(depmod, depmod->cfg->dirname);
 	if (err < 0)
 		return err;
+
+	for (ext = depmod->cfg->externals; ext != NULL; ext = ext->next) {
+		err = depmod_modules_search_path(depmod, ext->path);
+		if (err < 0 && err == -ENOENT)
+			/* ignore external dir absense */
+			continue;
+	}
+
 	return 0;
 }
 
-- 
2.9.3

Re: [PATCH RFC 0/3] depmod: implement external directories support

[Yauheni Kaliuta]

Hi, Lucas!

Any feedback on that?

>>>>> On Wed, 23 Nov 2016 17:23:36 +0200, Yauheni Kaliuta  wrote:

 > Would it be possible to implement something like external
 > directories support in depmod (see PATCH 3 description).

 > I'll copy here the part:

 > This is a pretty simple extention of existing logic, since now
 > depmod already is able to:

 > a) scan modules with full path from command line without -a
 > switch;
 > b) detects broken symbol dependencies and broken modversions,
 > what assumes, that modules are already are not built for the
 > existing kernel.

 > Yauheni Kaliuta (3):
 >   depmod: create depmod dir independent search function
 >   depmod: search key: move builtin detection under the add function
 >   depmod: implement external directories support

 >  tools/depmod.c | 209 +++++++++++++++++++++++++++++++++++++++++++++++----------
 >  1 file changed, 175 insertions(+), 34 deletions(-)

 > -- 
 > 2.9.3


-- 
WBR,
Yauheni Kaliuta

Re: [PATCH RFC 0/3] depmod: implement external directories support

[Lucas De Marchi]

On Sat, Mar 18, 2017 at 2:46 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Hi, Lucas!
>
> Any feedback on that?

I gave it a quick look and it looks good to me. I applied the second patch.

For first patch would be good to use scratchbuf

For this one... I miss some tests in the testsuite to be confident
this a) is working as expected
and b) isn't break the current behavior.


Sorry for the delay.


Lucas De Marchi

Re: [PATCH RFC 0/3] depmod: implement external directories support

[Yauheni Kaliuta]

Hi, Lucas!

>>>>> On Tue, 9 May 2017 00:51:43 -0700, Lucas De Marchi  wrote:

 > On Sat, Mar 18, 2017 at 2:46 PM, Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> Hi, Lucas!
 >> 
 >> Any feedback on that?

 > I gave it a quick look and it looks good to me. I applied the second patch.

 > For first patch would be good to use scratchbuf

Ok.

 > For this one... I miss some tests in the testsuite to be confident
 > this a) is working as expected
 > and b) isn't break the current behavior.

Thanks a lot! I'll think about the testsuite.

-- 
WBR,
Yauheni Kaliuta

[PATCHv2 0/4] depmod: implement external directories support

[Yauheni Kaliuta]

This is a pretty simple extention of existing logic, since now
depmod already is able to:

a) scan modules with full path from command line without -a
switch;
b) detects broken symbol dependencies and broken modversions,
what assumes, that modules are already are not built for the
existing kernel.

See PATCH3 description.


V1->V2:

- rewrite directory scanning code to use scratchbuf;
- add tests.

Yauheni Kaliuta (4):
  depmod: create depmod dir independent search function
  depmod: rewrite depmod modules search with scratchbuf
  depmod: implement external directories support
  testsuite: add tests for external directory support

 testsuite/populate-modules.sh                      |   8 +
 .../etc/depmod.d/external.conf                     |   1 +
 .../etc/depmod.d/search.conf                       |   1 +
 .../lib/modules/4.4.4/correct-modules.dep          |   1 +
 .../etc/depmod.d/external.conf                     |   1 +
 .../etc/depmod.d/search.conf                       |   1 +
 .../lib/modules/4.4.4/correct-modules.dep          |   1 +
 .../test-modinfo/correct-external.txt              |   1 +
 .../external/lib/modules/4.4.4/modules.alias       |   1 +
 .../external/lib/modules/4.4.4/modules.alias.bin   | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.builtin.bin |   0
 .../external/lib/modules/4.4.4/modules.dep         |   1 +
 .../external/lib/modules/4.4.4/modules.dep.bin     | Bin 0 -> 73 bytes
 .../external/lib/modules/4.4.4/modules.devname     |   0
 .../external/lib/modules/4.4.4/modules.softdep     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols.bin | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.alias       |   1 +
 .../external/lib/modules/4.4.4/modules.alias.bin   | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.builtin.bin |   0
 .../external/lib/modules/4.4.4/modules.dep         |   1 +
 .../external/lib/modules/4.4.4/modules.dep.bin     | Bin 0 -> 73 bytes
 .../external/lib/modules/4.4.4/modules.devname     |   0
 .../external/lib/modules/4.4.4/modules.softdep     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols.bin | Bin 0 -> 12 bytes
 .../test-modprobe/external/proc/modules            |   0
 testsuite/test-depmod.c                            |  52 +++++
 testsuite/test-modinfo.c                           |  21 ++
 testsuite/test-modprobe.c                          |  22 ++
 tools/depmod.c                                     | 232 +++++++++++++++++----
 31 files changed, 309 insertions(+), 41 deletions(-)
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/external.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/search.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/lib/modules/4.4.4/correct-modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/external.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/search.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/lib/modules/4.4.4/correct-modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/correct-external.txt
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.builtin.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.devname
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.softdep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.builtin.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.devname
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.softdep
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/proc/modules

-- 
2.12.2.639.g584f8975d2d9

[PATCHv2 1/4] depmod: create depmod dir independent search function

[Yauheni Kaliuta]

Prepare to implement external directories support.

The patch splits depmod_modules_search() function to two
functions: depmod_modules_search(), called by the high level with
intention to search all possible modules, and
depmod_module_search_path(), which takes path as a parameter and
scans modules under the path only. Initially it is used to scan
the same depmod->cfg->dirname path only.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index ded64500a50f..cfed864ff6c1 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1191,29 +1191,42 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 	return err;
 }
 
-static int depmod_modules_search(struct depmod *depmod)
+static int depmod_modules_search_path(struct depmod *depmod,
+				      const char *path)
 {
-	char path[PATH_MAX];
-	DIR *d = opendir(depmod->cfg->dirname);
+	char path_buf[PATH_MAX];
+	DIR *d;
 	size_t baselen;
 	int err;
+
+	d = opendir(path);
 	if (d == NULL) {
 		err = -errno;
-		ERR("could not open directory %s: %m\n", depmod->cfg->dirname);
+		ERR("could not open directory %s: %m\n", path);
 		return err;
 	}
 
-	baselen = depmod->cfg->dirnamelen;
-	memcpy(path, depmod->cfg->dirname, baselen);
-	path[baselen] = '/';
+	baselen = strlen(path);
+	memcpy(path_buf, path, baselen);
+	path_buf[baselen] = '/';
 	baselen++;
-	path[baselen] = '\0';
+	path_buf[baselen] = '\0';
 
-	err = depmod_modules_search_dir(depmod, d, baselen, path);
+	err = depmod_modules_search_dir(depmod, d, baselen, path_buf);
 	closedir(d);
 	return err;
 }
 
+static int depmod_modules_search(struct depmod *depmod)
+{
+	int err;
+
+	err = depmod_modules_search_path(depmod, depmod->cfg->dirname);
+	if (err < 0)
+		return err;
+	return 0;
+}
+
 static int mod_cmp(const void *pa, const void *pb) {
 	const struct mod *a = *(const struct mod **)pa;
 	const struct mod *b = *(const struct mod **)pb;
-- 
2.12.2.639.g584f8975d2d9

Re: [PATCHv2 1/4] depmod: create depmod dir independent search function

[Lucas De Marchi]

On Tue, May 9, 2017 at 12:09 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Prepare to implement external directories support.
>
> The patch splits depmod_modules_search() function to two
> functions: depmod_modules_search(), called by the high level with
> intention to search all possible modules, and
> depmod_module_search_path(), which takes path as a parameter and
> scans modules under the path only. Initially it is used to scan
> the same depmod->cfg->dirname path only.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---

Applied, thanks.

Lucas De Marchi

[PATCHv2 2/4] depmod: rewrite depmod modules search with scratchbuf

[Yauheni Kaliuta]

The recursive search code used used pretty big, PATH_MAX,
automatic storage buffer for the module directory scanning. Some
time ago there was scratchbuf implemented, which dynamically
reallocates its buffer on demand. The patch takes it in use for
the scanning code also. The initial size is hardcoded to 256
bytes which sounds good enough for most usecases so there should
be not many reallocations.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index cfed864ff6c1..a9a02bbb1aa7 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1108,10 +1108,11 @@ add:
 	return 0;
 }
 
-static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t baselen, char *path)
+static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t baselen, struct scratchbuf *s_path)
 {
 	struct dirent *de;
 	int err = 0, dfd = dirfd(d);
+	char *path;
 
 	while ((de = readdir(d)) != NULL) {
 		const char *name = de->d_name;
@@ -1124,11 +1125,13 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 		if (streq(name, "build") || streq(name, "source"))
 			continue;
 		namelen = strlen(name);
-		if (baselen + namelen + 2 >= PATH_MAX) {
-			path[baselen] = '\0';
-			ERR("path is too long %s%s\n", path, name);
+		if (scratchbuf_alloc(s_path, baselen + namelen + 2) < 0) {
+			err = -ENOMEM;
+			ERR("No memory\n");
 			continue;
 		}
+
+		path = scratchbuf_str(s_path);
 		memcpy(path + baselen, name, namelen + 1);
 
 		if (de->d_type == DT_REG)
@@ -1154,10 +1157,6 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 		if (is_dir) {
 			int fd;
 			DIR *subdir;
-			if (baselen + namelen + 2 + NAME_MAX >= PATH_MAX) {
-				ERR("directory path is too long %s\n", path);
-				continue;
-			}
 			fd = openat(dfd, name, O_RDONLY);
 			if (fd < 0) {
 				ERR("openat(%d, %s, O_RDONLY): %m\n",
@@ -1174,7 +1173,7 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 			path[baselen + namelen + 1] = '\0';
 			err = depmod_modules_search_dir(depmod, subdir,
 							baselen + namelen + 1,
-							path);
+							s_path);
 			closedir(subdir);
 		} else {
 			err = depmod_modules_search_file(depmod, baselen,
@@ -1187,14 +1186,16 @@ static int depmod_modules_search_dir(struct depmod *depmod, DIR *d, size_t basel
 			err = 0; /* ignore errors */
 		}
 	}
-
 	return err;
 }
 
 static int depmod_modules_search_path(struct depmod *depmod,
 				      const char *path)
 {
-	char path_buf[PATH_MAX];
+	char buf[256];
+	_cleanup_(scratchbuf_release) struct scratchbuf s_path_buf =
+		SCRATCHBUF_INITIALIZER(buf);
+	char *path_buf;
 	DIR *d;
 	size_t baselen;
 	int err;
@@ -1207,12 +1208,20 @@ static int depmod_modules_search_path(struct depmod *depmod,
 	}
 
 	baselen = strlen(path);
+
+	if (scratchbuf_alloc(&s_path_buf, baselen + 2) < 0) {
+		err = -ENOMEM;
+		goto out;
+	}
+	path_buf = scratchbuf_str(&s_path_buf);
+
 	memcpy(path_buf, path, baselen);
 	path_buf[baselen] = '/';
 	baselen++;
 	path_buf[baselen] = '\0';
 
-	err = depmod_modules_search_dir(depmod, d, baselen, path_buf);
+	err = depmod_modules_search_dir(depmod, d, baselen, &s_path_buf);
+out:
 	closedir(d);
 	return err;
 }
-- 
2.12.2.639.g584f8975d2d9

Re: [PATCHv2 2/4] depmod: rewrite depmod modules search with scratchbuf

[Lucas De Marchi]

On Tue, May 9, 2017 at 12:09 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> The recursive search code used used pretty big, PATH_MAX,
> automatic storage buffer for the module directory scanning. Some
> time ago there was scratchbuf implemented, which dynamically
> reallocates its buffer on demand. The patch takes it in use for
> the scanning code also. The initial size is hardcoded to 256
> bytes which sounds good enough for most usecases so there should
> be not many reallocations.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---

Applied, thanks.

Lucas De Marchi

[PATCHv2 3/4] depmod: implement external directories support

[Yauheni Kaliuta]

The idea is to add a configuration keyword, external, which
will list directories for scanning for particular kernel version
mask:

external 4.10 /the/modules/dir /second/modules/dir

And extend "search" keyword to set it's priority with pseudo dir
"external" (as it's done for built-in):

search subdir external subdir2 built-in subdir3

(actually, the version is the same as for override keyword: * or
posix regexp, so example above is a bit incorrect).

All other logic left the same: if there are duplicates, only one
is under consideration and it is unloadable if it is bad.

The resulting modules.dep will contain entries a-la:

/the/modules/dir/module1.ko:
kernel/module2.ko: /the/modules/dir/module1.ko

(here /lib/modules/$(uname -r)/kernel/module2.ko depends of
symbols, provided by /the/modules/dir/module1.ko and external has
higher priority).

modprobe and modinfo understand it out of box.

This is a pretty simple extention of existing logic, since now
depmod already is able to:

a) scan modules with full path from command line without -a
switch;
b) detects broken symbol dependencies and broken modversions,
what assumes, that modules are already are not built for the
existing kernel.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 150 insertions(+), 22 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index a9a02bbb1aa7..7ff3e9ed191e 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -48,6 +48,7 @@
 static int verbose = DEFAULT_VERBOSE;
 
 static const char CFG_BUILTIN_KEY[] = "built-in";
+static const char CFG_EXTERNAL_KEY[] = "external";
 static const char *default_cfg_paths[] = {
 	"/run/depmod.d",
 	SYSCONFDIR "/depmod.d",
@@ -436,9 +437,21 @@ struct cfg_override {
 	char path[];
 };
 
+enum search_type {
+	SEARCH_PATH,
+	SEARCH_BUILTIN,
+	SEARCH_EXTERNAL
+};
+
 struct cfg_search {
 	struct cfg_search *next;
-	uint8_t builtin;
+	enum search_type type;
+	size_t len;
+	char path[];
+};
+
+struct cfg_external {
+	struct cfg_external *next;
 	size_t len;
 	char path[];
 };
@@ -453,15 +466,27 @@ struct cfg {
 	uint8_t warn_dups;
 	struct cfg_override *overrides;
 	struct cfg_search *searches;
+	struct cfg_external *externals;
 };
 
+static enum search_type cfg_define_search_type(const char *path)
+{
+	if (streq(path, CFG_BUILTIN_KEY))
+		return SEARCH_BUILTIN;
+	if (streq(path, CFG_EXTERNAL_KEY))
+		return SEARCH_EXTERNAL;
+	return SEARCH_PATH;
+}
+
 static int cfg_search_add(struct cfg *cfg, const char *path)
 {
 	struct cfg_search *s;
 	size_t len;
-	uint8_t builtin = streq(path, CFG_BUILTIN_KEY);
+	enum search_type type;
+
+	type = cfg_define_search_type(path);
 
-	if (builtin)
+	if (type != SEARCH_PATH)
 		len = 0;
 	else
 		len = strlen(path) + 1;
@@ -471,15 +496,15 @@ static int cfg_search_add(struct cfg *cfg, const char *path)
 		ERR("search add: out of memory\n");
 		return -ENOMEM;
 	}
-	s->builtin = builtin;
-	if (builtin)
+	s->type = type;
+	if (type != SEARCH_PATH)
 		s->len = 0;
 	else {
 		s->len = len - 1;
 		memcpy(s->path, path, len);
 	}
 
-	DBG("search add: %s, builtin=%hhu\n", path, builtin);
+	DBG("search add: %s, search type=%hhu\n", path, type);
 
 	s->next = cfg->searches;
 	cfg->searches = s;
@@ -527,6 +552,32 @@ static void cfg_override_free(struct cfg_override *o)
 	free(o);
 }
 
+static int cfg_external_add(struct cfg *cfg, const char *path)
+{
+	struct cfg_external *ext;
+	size_t len = strlen(path);
+
+	ext = malloc(sizeof(struct cfg_external) + len + 1);
+	if (ext == NULL) {
+		ERR("external add: out of memory\n");
+		return -ENOMEM;
+	}
+
+	strcpy(ext->path, path);
+	ext->len = len;
+
+	DBG("external add: %s\n", ext->path);
+
+	ext->next = cfg->externals;
+	cfg->externals = ext;
+	return 0;
+}
+
+static void cfg_external_free(struct cfg_external *ext)
+{
+	free(ext);
+}
+
 static int cfg_kernel_matches(const struct cfg *cfg, const char *pattern)
 {
 	regex_t re;
@@ -590,6 +641,20 @@ static int cfg_file_parse(struct cfg *cfg, const char *filename)
 			}
 
 			cfg_override_add(cfg, modname, subdir);
+		} else if (streq(cmd, "external")) {
+			const char *version = strtok_r(NULL, "\t ", &saveptr);
+			const char *dir = strtok_r(NULL, "\t ", &saveptr);
+
+			if (version == NULL || dir == NULL)
+				goto syntax_error;
+
+			if (!cfg_kernel_matches(cfg, version)) {
+				INF("%s:%u: external directory did not match %s\n",
+				    filename, linenum, version);
+				goto done_next;
+			}
+
+			cfg_external_add(cfg, dir);
 		} else if (streq(cmd, "include")
 				|| streq(cmd, "make_map_files")) {
 			INF("%s:%u: command %s not implemented yet\n",
@@ -784,6 +849,12 @@ static void cfg_free(struct cfg *cfg)
 		cfg->searches = cfg->searches->next;
 		cfg_search_free(tmp);
 	}
+
+	while (cfg->externals) {
+		struct cfg_external *tmp = cfg->externals;
+		cfg->externals = cfg->externals->next;
+		cfg_external_free(tmp);
+	}
 }
 
 
@@ -993,6 +1064,33 @@ static int depmod_module_del(struct depmod *depmod, struct mod *mod)
 	return 0;
 }
 
+static const char *search_to_string(const struct cfg_search *s)
+{
+	switch(s->type) {
+	case SEARCH_EXTERNAL:
+		return "external";
+	case SEARCH_BUILTIN:
+		return "built-in";
+	default:
+		return s->path;
+	}
+}
+
+static bool depmod_is_path_starts_with(const char *path,
+				       size_t pathlen,
+				       const char *prefix,
+				       size_t prefix_len)
+{
+	if (pathlen <= prefix_len)
+		return false;
+	if (path[prefix_len] != '/')
+		return false;
+	if (memcmp(path, prefix, prefix_len) != 0)
+		return false;
+
+	return true;
+}
+
 /* returns if existing module @mod is higher priority than newpath.
  * note this is the inverse of module-init-tools is_higher_priority()
  */
@@ -1001,6 +1099,7 @@ static int depmod_module_is_higher_priority(const struct depmod *depmod, const s
 	const struct cfg *cfg = depmod->cfg;
 	const struct cfg_override *ov;
 	const struct cfg_search *se;
+	const struct cfg_external *ext;
 
 	/* baselen includes the last '/' and mod->baselen doesn't. So it's
 	 * actually correct to use modnamelen in the first and modnamesz in
@@ -1009,35 +1108,55 @@ static int depmod_module_is_higher_priority(const struct depmod *depmod, const s
 	size_t oldlen = mod->baselen + mod->modnamesz;
 	const char *oldpath = mod->path;
 	int i, bprio = -1, oldprio = -1, newprio = -1;
-
-	assert(strncmp(newpath, cfg->dirname, cfg->dirnamelen) == 0);
-	assert(strncmp(oldpath, cfg->dirname, cfg->dirnamelen) == 0);
-
-	newpath += cfg->dirnamelen + 1;
-	newlen -= cfg->dirnamelen + 1;
-	oldpath += cfg->dirnamelen + 1;
-	oldlen -= cfg->dirnamelen + 1;
+	size_t relnewlen = 0;
+	size_t reloldlen = 0;
+	const char *relnewpath = NULL;
+	const char *reloldpath = NULL;
 
 	DBG("comparing priorities of %s and %s\n",
 	    oldpath, newpath);
 
+	if (strncmp(newpath, cfg->dirname, cfg->dirnamelen) == 0) {
+		relnewpath = newpath + cfg->dirnamelen + 1;
+		relnewlen = newlen - cfg->dirnamelen + 1;
+	}
+	if (strncmp(oldpath, cfg->dirname, cfg->dirnamelen) == 0) {
+		reloldpath = oldpath + cfg->dirnamelen + 1;
+		reloldlen = oldlen - cfg->dirnamelen + 1;
+	}
+
 	for (ov = cfg->overrides; ov != NULL; ov = ov->next) {
 		DBG("override %s\n", ov->path);
-		if (newlen == ov->len && memcmp(ov->path, newpath, newlen) == 0)
+		if (relnewlen == ov->len &&
+		    memcmp(ov->path, relnewpath, relnewlen) == 0)
 			return 0;
-		if (oldlen == ov->len && memcmp(ov->path, oldpath, oldlen) == 0)
+		if (reloldlen == ov->len &&
+		    memcmp(ov->path, reloldpath, reloldlen) == 0)
 			return 1;
 	}
 
 	for (i = 0, se = cfg->searches; se != NULL; se = se->next, i++) {
-		DBG("search %s\n", se->builtin ? "built-in" : se->path);
-		if (se->builtin)
+		DBG("search %s\n", search_to_string(se));
+		if (se->type == SEARCH_BUILTIN)
 			bprio = i;
-		else if (newlen > se->len && newpath[se->len] == '/' &&
-			 memcmp(se->path, newpath, se->len) == 0)
+		else if (se->type == SEARCH_EXTERNAL) {
+			for (ext = cfg->externals; ext != NULL; ext = ext->next, i++) {
+				if (depmod_is_path_starts_with(newpath,
+							       newlen,
+							       ext->path,
+							       ext->len))
+					newprio = i;
+				if (depmod_is_path_starts_with(oldpath,
+							       oldlen,
+							       ext->path,
+							       ext->len))
+					oldprio = i;
+			}
+		} else if (relnewlen > se->len && relnewpath[se->len] == '/' &&
+			 memcmp(se->path, relnewpath, se->len) == 0)
 			newprio = i;
-		else if (oldlen > se->len && oldpath[se->len] == '/' &&
-			 memcmp(se->path, oldpath, se->len) == 0)
+		else if (reloldlen > se->len && reloldpath[se->len] == '/' &&
+			 memcmp(se->path, reloldpath, se->len) == 0)
 			oldprio = i;
 	}
 
@@ -1229,10 +1348,19 @@ out:
 static int depmod_modules_search(struct depmod *depmod)
 {
 	int err;
+	struct cfg_external *ext;
 
 	err = depmod_modules_search_path(depmod, depmod->cfg->dirname);
 	if (err < 0)
 		return err;
+
+	for (ext = depmod->cfg->externals; ext != NULL; ext = ext->next) {
+		err = depmod_modules_search_path(depmod, ext->path);
+		if (err < 0 && err == -ENOENT)
+			/* ignore external dir absense */
+			continue;
+	}
+
 	return 0;
 }
 
-- 
2.12.2.639.g584f8975d2d9

Re: [PATCHv2 3/4] depmod: implement external directories support

[Lucas De Marchi]

On Tue, May 9, 2017 at 12:09 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> The idea is to add a configuration keyword, external, which
> will list directories for scanning for particular kernel version
> mask:
>
> external 4.10 /the/modules/dir /second/modules/dir
>
> And extend "search" keyword to set it's priority with pseudo dir
> "external" (as it's done for built-in):
>
> search subdir external subdir2 built-in subdir3
>
> (actually, the version is the same as for override keyword: * or
> posix regexp, so example above is a bit incorrect).
>
> All other logic left the same: if there are duplicates, only one
> is under consideration and it is unloadable if it is bad.
>
> The resulting modules.dep will contain entries a-la:
>
> /the/modules/dir/module1.ko:
> kernel/module2.ko: /the/modules/dir/module1.ko
>
> (here /lib/modules/$(uname -r)/kernel/module2.ko depends of
> symbols, provided by /the/modules/dir/module1.ko and external has
> higher priority).
>
> modprobe and modinfo understand it out of box.
>
> This is a pretty simple extention of existing logic, since now
> depmod already is able to:
>
> a) scan modules with full path from command line without -a
> switch;
> b) detects broken symbol dependencies and broken modversions,
> what assumes, that modules are already are not built for the
> existing kernel.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---


Thanks, applied!

Lucas De Marchi

[PATCHv2 4/4] testsuite: add tests for external directory support

[Yauheni Kaliuta]

The following tests added:

- depmod_search_order_external_first -- checks if external module
  is taken in use when it has higher priority;
- depmod_search_order_external_last -- checks if external module
  is skipped when it has lower priority;
- test_modinfo_external -- checks if modinfo is able to look up
  correct external module;
- modprobe_external -- checks if modprobe is able to look up
  correct external module and loads it.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 testsuite/populate-modules.sh                      |   8 ++++
 .../etc/depmod.d/external.conf                     |   1 +
 .../etc/depmod.d/search.conf                       |   1 +
 .../lib/modules/4.4.4/correct-modules.dep          |   1 +
 .../etc/depmod.d/external.conf                     |   1 +
 .../etc/depmod.d/search.conf                       |   1 +
 .../lib/modules/4.4.4/correct-modules.dep          |   1 +
 .../test-modinfo/correct-external.txt              |   1 +
 .../external/lib/modules/4.4.4/modules.alias       |   1 +
 .../external/lib/modules/4.4.4/modules.alias.bin   | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.builtin.bin |   0
 .../external/lib/modules/4.4.4/modules.dep         |   1 +
 .../external/lib/modules/4.4.4/modules.dep.bin     | Bin 0 -> 73 bytes
 .../external/lib/modules/4.4.4/modules.devname     |   0
 .../external/lib/modules/4.4.4/modules.softdep     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols.bin | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.alias       |   1 +
 .../external/lib/modules/4.4.4/modules.alias.bin   | Bin 0 -> 12 bytes
 .../external/lib/modules/4.4.4/modules.builtin.bin |   0
 .../external/lib/modules/4.4.4/modules.dep         |   1 +
 .../external/lib/modules/4.4.4/modules.dep.bin     | Bin 0 -> 73 bytes
 .../external/lib/modules/4.4.4/modules.devname     |   0
 .../external/lib/modules/4.4.4/modules.softdep     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols     |   1 +
 .../external/lib/modules/4.4.4/modules.symbols.bin | Bin 0 -> 12 bytes
 .../test-modprobe/external/proc/modules            |   0
 testsuite/test-depmod.c                            |  52 +++++++++++++++++++++
 testsuite/test-modinfo.c                           |  21 +++++++++
 testsuite/test-modprobe.c                          |  22 +++++++++
 30 files changed, 118 insertions(+)
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/external.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/search.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-first/lib/modules/4.4.4/correct-modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/external.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/search.conf
 create mode 100644 testsuite/rootfs-pristine/test-depmod/search-order-external-last/lib/modules/4.4.4/correct-modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/correct-external.txt
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.builtin.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep.bin
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.devname
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.softdep
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols
 create mode 100644 testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.builtin.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.devname
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.softdep
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols.bin
 create mode 100644 testsuite/rootfs-pristine/test-modprobe/external/proc/modules

diff --git a/testsuite/populate-modules.sh b/testsuite/populate-modules.sh
index a7e503603b95..3ac92ee8030d 100755
--- a/testsuite/populate-modules.sh
+++ b/testsuite/populate-modules.sh
@@ -22,6 +22,12 @@ map=(
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-i.ko"]="mod-loop-i.ko"
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-j.ko"]="mod-loop-j.ko"
     ["test-depmod/detect-loop/lib/modules/4.4.4/kernel/mod-loop-k.ko"]="mod-loop-k.ko"
+    ["test-depmod/search-order-external-first/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+    ["test-depmod/search-order-external-first/lib/modules/4.4.4/foobar/"]="mod-simple.ko"
+    ["test-depmod/search-order-external-first/lib/modules/external/"]="mod-simple.ko"
+    ["test-depmod/search-order-external-last/lib/modules/4.4.4/foo/"]="mod-simple.ko"
+    ["test-depmod/search-order-external-last/lib/modules/4.4.4/foobar/"]="mod-simple.ko"
+    ["test-depmod/search-order-external-last/lib/modules/external/"]="mod-simple.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"
@@ -40,6 +46,7 @@ map=(
     ["test-modprobe/oldkernel-force/lib/modules/3.3.3/kernel/"]="mod-simple.ko"
     ["test-modprobe/alias-to-none/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
     ["test-modprobe/module-param-kcmdline/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
+    ["test-modprobe/external/lib/modules/external/"]="mod-simple.ko"
     ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko"
     ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko"
     ["test-depmod/modules-order-compressed/lib/modules/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko"
@@ -48,6 +55,7 @@ map=(
     ["test-modinfo/mod-simple-sparc64.ko"]="mod-simple-sparc64.ko"
     ["test-modinfo/mod-simple-sha1.ko"]="mod-simple.ko"
     ["test-modinfo/mod-simple-sha256.ko"]="mod-simple.ko"
+    ["test-modinfo/external/lib/modules/external/mod-simple.ko"]="mod-simple.ko"
     ["test-tools/insert/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
     ["test-tools/remove/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
 )
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/external.conf b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/external.conf
new file mode 100644
index 000000000000..59f46ae91d2d
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/external.conf
@@ -0,0 +1 @@
+external 4\.4\..* /lib/modules/external
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/search.conf b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/search.conf
new file mode 100644
index 000000000000..642e497f5e63
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/etc/depmod.d/search.conf
@@ -0,0 +1 @@
+search external foobar foo built-in
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-first/lib/modules/4.4.4/correct-modules.dep b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/lib/modules/4.4.4/correct-modules.dep
new file mode 100644
index 000000000000..e612900c5de7
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-first/lib/modules/4.4.4/correct-modules.dep
@@ -0,0 +1 @@
+/lib/modules/external/mod-simple.ko:
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/external.conf b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/external.conf
new file mode 100644
index 000000000000..59f46ae91d2d
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/external.conf
@@ -0,0 +1 @@
+external 4\.4\..* /lib/modules/external
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/search.conf b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/search.conf
new file mode 100644
index 000000000000..5fdb81236788
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/etc/depmod.d/search.conf
@@ -0,0 +1 @@
+search foobar foo built-in external
diff --git a/testsuite/rootfs-pristine/test-depmod/search-order-external-last/lib/modules/4.4.4/correct-modules.dep b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/lib/modules/4.4.4/correct-modules.dep
new file mode 100644
index 000000000000..eab3bb05305d
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-depmod/search-order-external-last/lib/modules/4.4.4/correct-modules.dep
@@ -0,0 +1 @@
+foobar/mod-simple.ko:
diff --git a/testsuite/rootfs-pristine/test-modinfo/correct-external.txt b/testsuite/rootfs-pristine/test-modinfo/correct-external.txt
new file mode 100644
index 000000000000..a094abec2650
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modinfo/correct-external.txt
@@ -0,0 +1 @@
+/lib/modules/external/mod-simple.ko
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias
new file mode 100644
index 000000000000..ba76e1815af0
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias
@@ -0,0 +1 @@
+# Aliases extracted from modules themselves.
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias.bin b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.alias.bin
new file mode 100644
index 0000000000000000000000000000000000000000..7075435f6268c4d815aec093d61e26647666ba76
GIT binary patch
literal 12
TcmdnM{w17&iGh)Ufq@4A6;A>Z

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.builtin.bin b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.builtin.bin
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep
new file mode 100644
index 000000000000..e612900c5de7
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep
@@ -0,0 +1 @@
+/lib/modules/external/mod-simple.ko:
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep.bin b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.dep.bin
new file mode 100644
index 0000000000000000000000000000000000000000..556e3c8142d5d85dba5b557474907f9f9dd99dcb
GIT binary patch
literal 73
zcmdnM{w17&iGfjpfx$UHCB8T_w;(5#0SFjDgnmwDl74P}N@-4Nv3_brNorAEVh%_^
S7ot!vJKu^SH}?Po0}lY-ZWUAj

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.devname b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.devname
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.softdep b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.softdep
new file mode 100644
index 000000000000..5554ccca7f9e
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.softdep
@@ -0,0 +1 @@
+# Soft dependencies extracted from modules themselves.
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols
new file mode 100644
index 000000000000..618c345f7e93
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols
@@ -0,0 +1 @@
+# Aliases for symbols, used by symbol_request().
diff --git a/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols.bin b/testsuite/rootfs-pristine/test-modinfo/external/lib/modules/4.4.4/modules.symbols.bin
new file mode 100644
index 0000000000000000000000000000000000000000..7075435f6268c4d815aec093d61e26647666ba76
GIT binary patch
literal 12
TcmdnM{w17&iGh)Ufq@4A6;A>Z

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias
new file mode 100644
index 000000000000..ba76e1815af0
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias
@@ -0,0 +1 @@
+# Aliases extracted from modules themselves.
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias.bin b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.alias.bin
new file mode 100644
index 0000000000000000000000000000000000000000..7075435f6268c4d815aec093d61e26647666ba76
GIT binary patch
literal 12
TcmdnM{w17&iGh)Ufq@4A6;A>Z

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.builtin.bin b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.builtin.bin
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep
new file mode 100644
index 000000000000..e612900c5de7
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep
@@ -0,0 +1 @@
+/lib/modules/external/mod-simple.ko:
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep.bin b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.dep.bin
new file mode 100644
index 0000000000000000000000000000000000000000..556e3c8142d5d85dba5b557474907f9f9dd99dcb
GIT binary patch
literal 73
zcmdnM{w17&iGfjpfx$UHCB8T_w;(5#0SFjDgnmwDl74P}N@-4Nv3_brNorAEVh%_^
S7ot!vJKu^SH}?Po0}lY-ZWUAj

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.devname b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.devname
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.softdep b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.softdep
new file mode 100644
index 000000000000..5554ccca7f9e
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.softdep
@@ -0,0 +1 @@
+# Soft dependencies extracted from modules themselves.
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols
new file mode 100644
index 000000000000..618c345f7e93
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols
@@ -0,0 +1 @@
+# Aliases for symbols, used by symbol_request().
diff --git a/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols.bin b/testsuite/rootfs-pristine/test-modprobe/external/lib/modules/4.4.4/modules.symbols.bin
new file mode 100644
index 0000000000000000000000000000000000000000..7075435f6268c4d815aec093d61e26647666ba76
GIT binary patch
literal 12
TcmdnM{w17&iGh)Ufq@4A6;A>Z

literal 0
HcmV?d00001

diff --git a/testsuite/rootfs-pristine/test-modprobe/external/proc/modules b/testsuite/rootfs-pristine/test-modprobe/external/proc/modules
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/testsuite/test-depmod.c b/testsuite/test-depmod.c
index 732a9d0ad001..6530442ce002 100644
--- a/testsuite/test-depmod.c
+++ b/testsuite/test-depmod.c
@@ -131,4 +131,56 @@ DEFINE_TEST(depmod_detect_loop,
 		.err = DETECT_LOOP_ROOTFS "/correct.txt",
 	});
 
+#define SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-external-first"
+static noreturn int depmod_search_order_external_first(const struct test *t)
+{
+	const char *progname = ABS_TOP_BUILDDIR "/tools/depmod";
+	const char *const args[] = {
+		progname,
+		NULL,
+	};
+
+	test_spawn_prog(progname, args);
+	exit(EXIT_FAILURE);
+}
+DEFINE_TEST(depmod_search_order_external_first,
+	.description = "check if depmod honor external keyword with higher priority",
+	.config = {
+		[TC_UNAME_R] = "4.4.4",
+		[TC_ROOTFS] = SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS,
+	},
+	.output = {
+		.files = (const struct keyval[]) {
+			{ SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS "/lib/modules/4.4.4/correct-modules.dep",
+			  SEARCH_ORDER_EXTERNAL_FIRST_ROOTFS "/lib/modules/4.4.4/modules.dep" },
+			{ }
+		},
+	});
+
+#define SEARCH_ORDER_EXTERNAL_LAST_ROOTFS TESTSUITE_ROOTFS "test-depmod/search-order-external-last"
+static noreturn int depmod_search_order_external_last(const struct test *t)
+{
+	const char *progname = ABS_TOP_BUILDDIR "/tools/depmod";
+	const char *const args[] = {
+		progname,
+		NULL,
+	};
+
+	test_spawn_prog(progname, args);
+	exit(EXIT_FAILURE);
+}
+DEFINE_TEST(depmod_search_order_external_last,
+	.description = "check if depmod honor external keyword with lower priority",
+	.config = {
+		[TC_UNAME_R] = "4.4.4",
+		[TC_ROOTFS] = SEARCH_ORDER_EXTERNAL_LAST_ROOTFS,
+	},
+	.output = {
+		.files = (const struct keyval[]) {
+			{ SEARCH_ORDER_EXTERNAL_LAST_ROOTFS "/lib/modules/4.4.4/correct-modules.dep",
+			  SEARCH_ORDER_EXTERNAL_LAST_ROOTFS "/lib/modules/4.4.4/modules.dep" },
+			{ }
+		},
+	});
+
 TESTSUITE_MAIN();
diff --git a/testsuite/test-modinfo.c b/testsuite/test-modinfo.c
index 487750207d05..8fdfe35ef4e6 100644
--- a/testsuite/test-modinfo.c
+++ b/testsuite/test-modinfo.c
@@ -89,4 +89,25 @@ DEFINE_TEST(test_modinfo_signature,
 		.out = TESTSUITE_ROOTFS "test-modinfo/correct.txt",
 	});
 #endif
+
+static noreturn int test_modinfo_external(const struct test *t)
+{
+	const char *const args[] = {
+		progname, "-F", "filename",
+		"mod-simple",
+		NULL,
+	};
+	test_spawn_prog(progname, args);
+	exit(EXIT_FAILURE);
+}
+DEFINE_TEST(test_modinfo_external,
+	.description = "check if modinfo finds external module",
+	.config = {
+		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modinfo/external",
+		[TC_UNAME_R] = "4.4.4",
+	},
+	.output = {
+		.out = TESTSUITE_ROOTFS "test-modinfo/correct-external.txt",
+	})
+
 TESTSUITE_MAIN();
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index e0dd1992a784..ee9d82d487c7 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -371,4 +371,26 @@ DEFINE_TEST(modprobe_oldkernel_force,
 	.modules_loaded = "mod-simple",
 	);
 
+static noreturn int modprobe_external(const struct test *t)
+{
+	const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
+	const char *const args[] = {
+		progname,
+		"mod-simple",
+		NULL,
+	};
+
+	test_spawn_prog(progname, args);
+	exit(EXIT_FAILURE);
+}
+DEFINE_TEST(modprobe_external,
+	.description = "check modprobe able to load external module",
+	.config = {
+		[TC_UNAME_R] = "4.4.4",
+		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/external",
+		[TC_INIT_MODULE_RETCODES] = "",
+	},
+	.modules_loaded = "mod-simple",
+	);
+
 TESTSUITE_MAIN();
-- 
2.12.2.639.g584f8975d2d9

Re: [PATCHv2 4/4] testsuite: add tests for external directory support

[Lucas De Marchi]

On Tue, May 9, 2017 at 12:09 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> The following tests added:
>
> - depmod_search_order_external_first -- checks if external module
>   is taken in use when it has higher priority;
> - depmod_search_order_external_last -- checks if external module
>   is skipped when it has lower priority;
> - test_modinfo_external -- checks if modinfo is able to look up
>   correct external module;
> - modprobe_external -- checks if modprobe is able to look up
>   correct external module and loads it.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>

Thanks, applied!

Lucas De Marchi

Re: [PATCHv2 0/4] depmod: implement external directories support

[Lucas De Marchi]

Hi,

On Tue, May 9, 2017 at 12:09 PM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> This is a pretty simple extention of existing logic, since now
> depmod already is able to:
>
> a) scan modules with full path from command line without -a
> switch;
> b) detects broken symbol dependencies and broken modversions,
> what assumes, that modules are already are not built for the
> existing kernel.
>
> See PATCH3 description.
>
>
> V1->V2:
>
> - rewrite directory scanning code to use scratchbuf;
> - add tests.
>
> Yauheni Kaliuta (4):
>   depmod: create depmod dir independent search function
>   depmod: rewrite depmod modules search with scratchbuf
>   depmod: implement external directories support
>   testsuite: add tests for external directory support

Thanks a lot. All 4 patches applied and this new feature will appear
on v25 soon. Sorry for the delay in reviewing and applying this.

Lucas De Marchi

Re: [PATCHv2 0/4] depmod: implement external directories support

[Yauheni Kaliuta]

Hi!

>>>>> On Tue,  9 May 2017 22:09:20 +0300, Yauheni Kaliuta  wrote:

 > This is a pretty simple extention of existing logic, since now
 > depmod already is able to:

 > a) scan modules with full path from command line without -a
 > switch;
 > b) detects broken symbol dependencies and broken modversions,
 > what assumes, that modules are already are not built for the
 > existing kernel.

[...]


I've heared a concern about the feature, that it may make sense to limit
the possible external directories to some subdirectory(s). The idea is that
3rd party vendor packages can pollute filesystem with its modules and a
system administrator may like to be sure that they are in a more defined
place.

What do you think?

Of course, it is not security concern, just about unintentional
pollution. If there is the intention, in most cases from the package
maintainer scipts it's possible to install symbolic link under the
permitted directory, for example, with the file anywere.

-- 
WBR,
Yauheni Kaliuta

Re: [PATCHv2 0/4] depmod: implement external directories support

[Lucas De Marchi]

On Tue, Jun 20, 2017 at 2:11 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Hi!
>
>>>>>> On Tue,  9 May 2017 22:09:20 +0300, Yauheni Kaliuta  wrote:
>
>  > This is a pretty simple extention of existing logic, since now
>  > depmod already is able to:
>
>  > a) scan modules with full path from command line without -a
>  > switch;
>  > b) detects broken symbol dependencies and broken modversions,
>  > what assumes, that modules are already are not built for the
>  > existing kernel.
>
> [...]
>
>
> I've heared a concern about the feature, that it may make sense to limit
> the possible external directories to some subdirectory(s). The idea is that
> 3rd party vendor packages can pollute filesystem with its modules and a
> system administrator may like to be sure that they are in a more defined
> place.
>
> What do you think?

Humn... doesn't that completely defeats the purpose of using it for development?

That just reminded me we missed the changes to the man page. Could you
take care of that?

> Of course, it is not security concern, just about unintentional
> pollution. If there is the intention, in most cases from the package
> maintainer scipts it's possible to install symbolic link under the
> permitted directory, for example, with the file anywere.

Not sure if kmod is the right place to restrict the directories.
Maybe it's a distro policy thing?
What would you restrict it to?


-- 
Lucas De Marchi

Re: [PATCHv2 0/4] depmod: implement external directories support

[Yauheni Kaliuta]

Hi, Lucas!

>>>>> On Wed, 19 Jul 2017 11:07:11 -0700, Lucas De Marchi  wrote:

 > On Tue, Jun 20, 2017 at 2:11 AM, Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> Hi!
 >> 
 >>>>>>> On Tue,  9 May 2017 22:09:20 +0300, Yauheni Kaliuta  wrote:
 >> 
 >> > This is a pretty simple extention of existing logic, since now
 >> > depmod already is able to:
 >> 
 >> > a) scan modules with full path from command line without -a
 >> > switch;
 >> > b) detects broken symbol dependencies and broken modversions,
 >> > what assumes, that modules are already are not built for the
 >> > existing kernel.
 >> 
 >> [...]
 >> 
 >> 
 >> I've heared a concern about the feature, that it may make sense to limit
 >> the possible external directories to some subdirectory(s). The idea is that
 >> 3rd party vendor packages can pollute filesystem with its modules and a
 >> system administrator may like to be sure that they are in a more defined
 >> place.
 >> 
 >> What do you think?

 > Humn... doesn't that completely defeats the purpose of using it for
 > development?

 > That just reminded me we missed the changes to the man page. Could you
 > take care of that?

Oh yes, sure. Just may be after vacations (next two weeks).

 >> Of course, it is not security concern, just about unintentional
 >> pollution. If there is the intention, in most cases from the package
 >> maintainer scipts it's possible to install symbolic link under the
 >> permitted directory, for example, with the file anywere.

 > Not sure if kmod is the right place to restrict the directories.
 > Maybe it's a distro policy thing?

I think the same. But it is about 3rd party modules.

 > What would you restrict it to?

I was thinking about that a bit.

What if I implement some configuration keyword, "restrict_external" for
example, with a directory prefix (up to the distribution, /lib/modules or
whatever) which is impossible then to override (by the additional configs
from the 3rd party module package) and if depmod finds such external
configuration, it ignores it with a warning?

-- 
WBR,
Yauheni Kaliuta