Here are 3 patches against the 'modules-next' branch of Jessica Yu's 'linux.git' repo.
I'm doing some refactoring in module_sig_check()...
[1/3] module: merge repetitive strings in module_sig_check()
[2/3] module: avoid *goto*s in module_sig_check()
[3/3] module: only handle errors with the *switch* statement in module_sig_check()
The 'reason' variable in module_sig_check() points to 3 strings across
the *switch* statement, all needlessly starting with the same text.
Let's put the starting text into the pr_notice() call -- it saves 21
bytes of the object code (x86 gcc 10.2.1).
Suggested-by: Joe Perches <[email protected]>
Signed-off-by: Sergey Shtylyov <[email protected]>
---
Changes in version 2:
- put less starting text into the pr_notice() call;
- updated the patch description accordingly, added the "Suggested-by:" tag.
kernel/module.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -2907,16 +2907,17 @@ static int module_sig_check(struct load_
* enforcing, certain errors are non-fatal.
*/
case -ENODATA:
- reason = "Loading of unsigned module";
+ reason = "unsigned module";
goto decide;
case -ENOPKG:
- reason = "Loading of module with unsupported crypto";
+ reason = "module with unsupported crypto";
goto decide;
case -ENOKEY:
- reason = "Loading of module with unavailable key";
+ reason = "module with unavailable key";
decide:
if (is_module_sig_enforced()) {
- pr_notice("%s: %s is rejected\n", info->name, reason);
+ pr_notice("%s: loading of %s is rejected\n",
+ info->name, reason);
return -EKEYREJECTED;
}
Let's move the common handling of the non-fatal errors after the *switch*
statement -- this avoids *goto*s inside that *switch*...
Suggested-by: Joe Perches <[email protected]>
Signed-off-by: Sergey Shtylyov <[email protected]>
---
Changes in version 2:
- new patch.
kernel/module.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -2908,20 +2908,13 @@ static int module_sig_check(struct load_
*/
case -ENODATA:
reason = "unsigned module";
- goto decide;
+ break;
case -ENOPKG:
reason = "module with unsupported crypto";
- goto decide;
+ break;
case -ENOKEY:
reason = "module with unavailable key";
- decide:
- if (is_module_sig_enforced()) {
- pr_notice("%s: loading of %s is rejected\n",
- info->name, reason);
- return -EKEYREJECTED;
- }
-
- return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+ break;
/* All other errors are fatal, including nomem, unparseable
* signatures and signature check failures - even if signatures
@@ -2930,6 +2923,13 @@ static int module_sig_check(struct load_
default:
return err;
}
+
+ if (is_module_sig_enforced()) {
+ pr_notice("%s: loading of %s is rejected\n", info->name, reason);
+ return -EKEYREJECTED;
+ }
+
+ return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
}
#else /* !CONFIG_MODULE_SIG */
static int module_sig_check(struct load_info *info, int flags)
Let's handle the successful call of mod_verify_sig() right after that call,
making the *switch* statement only handle the real errors, and then move
the comment from the first *case* before *switch* itself and the comment
before *default* after it. Fix the comment style, add article/comma/dash,
spell out "nomem" as "lack of memory" in these comments, while at it...
Suggested-by: Joe Perches <[email protected]>
Signed-off-by: Sergey Shtylyov <[email protected]>
---
Changes in version 2:
- new patch.
kernel/module.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -2895,17 +2895,18 @@ static int module_sig_check(struct load_
/* We truncate the module to discard the signature */
info->len -= markerlen;
err = mod_verify_sig(mod, info);
+ if (!err) {
+ info->sig_ok = true;
+ return 0;
+ }
}
+ /*
+ * We don't permit modules to be loaded into the trusted kernels
+ * without a valid signature on them, but if we're not enforcing,
+ * certain errors are non-fatal.
+ */
switch (err) {
- case 0:
- info->sig_ok = true;
- return 0;
-
- /* We don't permit modules to be loaded into trusted kernels
- * without a valid signature on them, but if we're not
- * enforcing, certain errors are non-fatal.
- */
case -ENODATA:
reason = "unsigned module";
break;
@@ -2916,11 +2917,12 @@ static int module_sig_check(struct load_
reason = "module with unavailable key";
break;
- /* All other errors are fatal, including nomem, unparseable
- * signatures and signature check failures - even if signatures
- * aren't required.
- */
default:
+ /*
+ * All other errors are fatal, including lack of memory,
+ * unparseable signatures, and signature check failures --
+ * even if signatures aren't required.
+ */
return err;
}
On Sat, 31 Oct 2020, Sergey Shtylyov wrote:
> Here are 3 patches against the 'modules-next' branch of Jessica Yu's 'linux.git' repo.
> I'm doing some refactoring in module_sig_check()...
>
> [1/3] module: merge repetitive strings in module_sig_check()
> [2/3] module: avoid *goto*s in module_sig_check()
> [3/3] module: only handle errors with the *switch* statement in module_sig_check()
Reviewed-by: Miroslav Benes <[email protected]>
M
+++ Sergey Shtylyov [31/10/20 23:05 +0300]:
>Here are 3 patches against the 'modules-next' branch of Jessica Yu's 'linux.git' repo.
>I'm doing some refactoring in module_sig_check()...
>
>[1/3] module: merge repetitive strings in module_sig_check()
>[2/3] module: avoid *goto*s in module_sig_check()
>[3/3] module: only handle errors with the *switch* statement in module_sig_check()
Applied to modules-next. Thanks Sergey for the cleanups!
Jessica