linux-kernel - Re: module: add an early early_mod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Dec 2017 02:00:53 +0100
From:   Jessica Yu <jeyu@...nel.org>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     rusty@...tcorp.com.au, keescook@...omium.org, tixxdz@...il.com,
        mbenes@...e.cz, atomlin@...hat.com, pmladek@...e.com,
        hare@...e.com, james.l.morris@...cle.com, ebiederm@...ssion.com,
        davem@...emloft.net, akpm@...ux-foundation.org,
        torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: module: add an early early_mod_check()

+++ Luis R. Rodriguez [07/12/17 16:15 -0800]:
>We technically do a bit of sanity check with a local
>struct module with pointers set to passed user data first.
>Only after we do these checks do we embark on allocating
>a struct module based on the passed information.
>
>There's a small set of early checks which help us bail
>out from processing and avoiding memory allocation, we
>currently have these small checks tangled with the allocation
>routine. Split this out so that we can expand on the early
>checks with a local copy first.
>
>This leaves setup_load_info() in an odd place given its
>the one that originally sets up the local struct module
>with pointing to the user data. Just move this out into the
>laod_module() routine to make the early_mod_check() routine
>much simpler.
>
>This change just shuffles code around a bit to make the
>next change easier to read, it technically has no functional
>changes.
>
>Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
>---
> kernel/module.c | 38 ++++++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 8042b8fcbf14..195ef37e242a 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3287,23 +3287,28 @@ static bool blacklisted(const char *module_name)
> }
> core_param(module_blacklist, module_blacklist, charp, 0400);
>
>-static struct module *layout_and_allocate(struct load_info *info, int flags)
>+/* Module within temporary copy, this doesn't do any allocation  */
>+static int early_mod_check(struct load_info *info, int flags,
>+			   struct module *mod)
> {
>-	/* Module within temporary copy. */
>-	struct module *mod;
>-	unsigned int ndx;
> 	int err;
>
>-	mod = setup_load_info(info, flags);
>-	if (IS_ERR(mod))
>-		return mod;
>-
> 	if (blacklisted(info->name))
>-		return ERR_PTR(-EPERM);
>+		return -EPERM;
>
> 	err = check_modinfo(mod, info, flags);
> 	if (err)
>-		return ERR_PTR(err);
>+		return err;
>+
>+	return 0;
>+}
>+
>+/* This starts to process the module and finally allocates a copy */
>+static struct module *layout_and_allocate(struct load_info *info,
>+					  struct module *mod)
>+{
>+	unsigned int ndx;
>+	int err;
>
> 	/* Allow arches to frob section contents and sizes.  */
> 	err = module_frob_arch_sections(info->hdr, info->sechdrs,
>@@ -3654,8 +3659,17 @@ static int load_module(struct load_info *info, const char __user *uargs,
> 	if (err)
> 		goto free_copy;
>
>-	/* Figure out module layout, and allocate all the memory. */
>-	mod = layout_and_allocate(info, flags);
>+	mod = setup_load_info(info, flags);
>+	if (IS_ERR(mod))
>+		return -EINVAL;

We need to goto free_copy here too, and propagate the error returned
by setup_load_info() i.e. propagate PTR_ERR(mod).

>+
>+	/* layout and check */
>+	err = early_mod_check(info, flags, mod);
>+	if (err)
>+		goto free_copy;
>+
>+	/* Layout and allocate all the memory. */
>+	mod = layout_and_allocate(info, mod);
> 	if (IS_ERR(mod)) {
> 		err = PTR_ERR(mod);
> 		goto free_copy;
>-- 
>2.15.0
>