[PATCH] modprobe: prefer -ENODATA over -ENOENT if no section found

[PATCH] modprobe: prefer -ENODATA over -ENOENT if no section found

[Dmitry Antipov]

When the module is definitely present but CONFIG_MODVERSIONS is
disabled, the following error message may be somewhat confusing:

modprobe --dump-modversions /path/to/module.ko.xz
modprobe: FATAL: could not get modversions of /path/to/module/ko.xz: No such file or directory

Choosing among the convenient errno values, I would suggest to use ENODATA
when the module lacks a particular ELF section (and vermagic as well).

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 libkmod/libkmod-elf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index ef4a8a3..fb2e3d9 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -392,7 +392,7 @@ static int elf_find_section(const struct kmod_elf *elf, const char *section)
 		return i;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const void **buf, uint64_t *buf_size)
@@ -422,7 +422,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const
 		return 0;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 /* array will be allocated with strings in a single malloc, just free *array */
@@ -653,7 +653,7 @@ int kmod_elf_strip_vermagic(struct kmod_elf *elf)
 	}
 
 	ELFDBG(elf, "no vermagic found in .modinfo\n");
-	return -ENOENT;
+	return -ENODATA;
 }
 
 
-- 
2.40.1

Re: [PATCH] modprobe: prefer -ENODATA over -ENOENT if no section found

[Christophe Leroy]

Le 18/05/2023 à 07:47, Dmitry Antipov a écrit :
> [Vous ne recevez pas souvent de courriers de dmantipov@yandex.ru. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> When the module is definitely present but CONFIG_MODVERSIONS is
> disabled, the following error message may be somewhat confusing:
> 
> modprobe --dump-modversions /path/to/module.ko.xz
> modprobe: FATAL: could not get modversions of /path/to/module/ko.xz: No such file or directory
> 
> Choosing among the convenient errno values, I would suggest to use ENODATA
> when the module lacks a particular ELF section (and vermagic as well).

Fine, but can you show the text message modprobe provides with your 
change, so that we can see if the change provides a somewhat better less 
confusing message ?


> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>   libkmod/libkmod-elf.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
> index ef4a8a3..fb2e3d9 100644
> --- a/libkmod/libkmod-elf.c
> +++ b/libkmod/libkmod-elf.c
> @@ -392,7 +392,7 @@ static int elf_find_section(const struct kmod_elf *elf, const char *section)
>                  return i;
>          }
> 
> -       return -ENOENT;
> +       return -ENODATA;
>   }
> 
>   int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const void **buf, uint64_t *buf_size)
> @@ -422,7 +422,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const
>                  return 0;
>          }
> 
> -       return -ENOENT;
> +       return -ENODATA;
>   }
> 
>   /* array will be allocated with strings in a single malloc, just free *array */
> @@ -653,7 +653,7 @@ int kmod_elf_strip_vermagic(struct kmod_elf *elf)
>          }
> 
>          ELFDBG(elf, "no vermagic found in .modinfo\n");
> -       return -ENOENT;
> +       return -ENODATA;
>   }
> 
> 
> --
> 2.40.1
> 

Re: [PATCH] modprobe: prefer -ENODATA over -ENOENT if no section found

[Dmitry Antipov]

On 5/19/23 09:52, Christophe Leroy wrote:

> Fine, but can you show the text message modprobe provides with your
> change, so that we can see if the change provides a somewhat better less
> confusing message ?

Sure, now it is expected to be:

$ modprobe --dump-modversions /lib/modules/6.2.15-300.fc38.x86_64/extra/crystalhd/crystalhd.ko.xz
modprobe: FATAL: could not get modversions of /lib/modules/6.2.15-300.fc38.x86_64/extra/crystalhd/crystalhd.ko.xz: No data available

I'm not insisting on -ENODATA, but IMO -ENOENT is not suitable in this case at all.

Dmitry

Re: [PATCH] modprobe: prefer -ENODATA over -ENOENT if no section found

[Christophe Leroy]

Le 19/05/2023 à 09:03, Dmitry Antipov a écrit :
> On 5/19/23 09:52, Christophe Leroy wrote:
> 
>> Fine, but can you show the text message modprobe provides with your
>> change, so that we can see if the change provides a somewhat better less
>> confusing message ?
> 
> Sure, now it is expected to be:
> 
> $ modprobe --dump-modversions 
> /lib/modules/6.2.15-300.fc38.x86_64/extra/crystalhd/crystalhd.ko.xz
> modprobe: FATAL: could not get modversions of 
> /lib/modules/6.2.15-300.fc38.x86_64/extra/crystalhd/crystalhd.ko.xz: No 
> data available
> 
> I'm not insisting on -ENODATA, but IMO -ENOENT is not suitable in this 
> case at all.
> 

I'm fine with that, just wanted know what the message is now.

Can you had the information in the commit message ?

With that added:

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

[PATCH] libkmod: prefer -ENODATA over -ENOENT if no section found

[Dmitry Antipov]

When the module is definitely present but CONFIG_MODVERSIONS is
disabled, the following error message may be somewhat confusing:

modprobe --dump-modversions /path/to/module.ko.xz
modprobe: FATAL: could not get modversions of /path/to/module.ko.xz: No such file or directory

Choosing among the convenient errno values, I would suggest to use
ENODATA when the module lacks a particular ELF section (and vermagic
as well). So now it is expected to be:

modprobe: FATAL: could not get modversions of /path/to/module.ko.xz: No data available

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 libkmod/libkmod-elf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index ef4a8a3..fb2e3d9 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -392,7 +392,7 @@ static int elf_find_section(const struct kmod_elf *elf, const char *section)
 		return i;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const void **buf, uint64_t *buf_size)
@@ -422,7 +422,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const
 		return 0;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 /* array will be allocated with strings in a single malloc, just free *array */
@@ -653,7 +653,7 @@ int kmod_elf_strip_vermagic(struct kmod_elf *elf)
 	}
 
 	ELFDBG(elf, "no vermagic found in .modinfo\n");
-	return -ENOENT;
+	return -ENODATA;
 }
 
 
-- 
2.40.1

[PATCH] libkmod, depmod: prefer -ENODATA over -ENOENT if no section found

[Dmitry Antipov]

When the module is definitely present but CONFIG_MODVERSIONS is
disabled, the following error message may be somewhat confusing:

modprobe --dump-modversions /path/to/module.ko.xz
modprobe: FATAL: could not get modversions of /path/to/module.ko.xz: No such file or directory

Choosing among the convenient errno values, I would suggest to use
ENODATA when the module lacks a particular ELF section (and vermagic
as well). So now it is expected to be:

modprobe: FATAL: could not get modversions of /path/to/module.ko.xz: No data available

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 libkmod/libkmod-elf.c | 6 +++---
 tools/depmod.c        | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index ef4a8a3..fb2e3d9 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -392,7 +392,7 @@ static int elf_find_section(const struct kmod_elf *elf, const char *section)
 		return i;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const void **buf, uint64_t *buf_size)
@@ -422,7 +422,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const
 		return 0;
 	}
 
-	return -ENOENT;
+	return -ENODATA;
 }
 
 /* array will be allocated with strings in a single malloc, just free *array */
@@ -653,7 +653,7 @@ int kmod_elf_strip_vermagic(struct kmod_elf *elf)
 	}
 
 	ELFDBG(elf, "no vermagic found in .modinfo\n");
-	return -ENOENT;
+	return -ENODATA;
 }
 
 
diff --git a/tools/depmod.c b/tools/depmod.c
index a2c39b3..1d1d41d 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1586,7 +1586,7 @@ static int depmod_load_modules(struct depmod *depmod)
 		struct kmod_list *l, *list = NULL;
 		int err = kmod_module_get_symbols(mod->kmod, &list);
 		if (err < 0) {
-			if (err == -ENOENT)
+			if (err == -ENODATA)
 				DBG("ignoring %s: no symbols\n", mod->path);
 			else
 				ERR("failed to load symbols from %s: %s\n",
-- 
2.40.1

Re: [PATCH] libkmod, depmod: prefer -ENODATA over -ENOENT if no section found

[Lucas De Marchi]

On Fri, 19 May 2023 12:36:30 +0300, Dmitry Antipov wrote:
> When the module is definitely present but CONFIG_MODVERSIONS is
> disabled, the following error message may be somewhat confusing:
> 
> modprobe --dump-modversions /path/to/module.ko.xz
> modprobe: FATAL: could not get modversions of /path/to/module.ko.xz: No such file or directory
> 
> Choosing among the convenient errno values, I would suggest to use
> ENODATA when the module lacks a particular ELF section (and vermagic
> as well). So now it is expected to be:
> 
> [...]

Applied, thanks!

[1/1] libkmod, depmod: prefer -ENODATA over -ENOENT if no section found
      commit: 5c004af29daf38119cc472dc8f1f080f10da6d82

Best regards,
-- 
Lucas De Marchi <lucas.de.marchi@gmail.com>