linux-kernel - Re: [PATCH v2] audit: fix potential null dereference 'context->module.name'

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20180725183124.zstlfonw7n3rxkvq@madcap2.tricolour.ca>
Date:   Wed, 25 Jul 2018 14:31:24 -0400
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Yi Wang <wang.yi59@....com.cn>
Cc:     paul@...l-moore.com, zhong.weidong@....com.cn,
        linux-kernel@...r.kernel.org, linux-audit@...hat.com,
        jiang.biao2@....com.cn
Subject: Re: [PATCH v2] audit: fix potential null dereference
 'context->module.name'

On 2018-07-25 10:26, Yi Wang wrote:
> The variable 'context->module.name' may be null pointer when
> kmalloc return null, so it's better to check it before using
> to avoid null dereference.
> Another one more thing this patch does is using kstrdup instead
> of (kmalloc + strcpy), and signal a lost record via audit_log_lost.
> 
> Signed-off-by: Yi Wang <wang.yi59@....com.cn>
> Reviewed-by: Jiang Biao <jiang.biao2@....com.cn>

Reviewed-by: Richard Guy Briggs <rgb@...hat.com>

> ---
> v2: use kstrdup instead of kmalloc + strcpy, and signal a lost
> record. Thanks to Eric and Paul.
> 
>  kernel/auditsc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index e80459f..713386a 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1272,8 +1272,12 @@ static void show_special(struct audit_context *context, int *call_panic)
>  		break;
>  	case AUDIT_KERN_MODULE:
>  		audit_log_format(ab, "name=");
> -		audit_log_untrustedstring(ab, context->module.name);
> -		kfree(context->module.name);
> +		if (context->module.name) {
> +			audit_log_untrustedstring(ab, context->module.name);
> +			kfree(context->module.name);
> +		} else
> +			audit_log_format(ab, "(null)");
> +
>  		break;
>  	}
>  	audit_log_end(ab);
> @@ -2408,8 +2412,9 @@ void __audit_log_kern_module(char *name)
>  {
>  	struct audit_context *context = current->audit_context;
>  
> -	context->module.name = kmalloc(strlen(name) + 1, GFP_KERNEL);
> -	strcpy(context->module.name, name);
> +	context->module.name = kstrdup(name, GFP_KERNEL);
> +	if (!context->module.name)
> +		audit_log_lost("out of memory in __audit_log_kern_module");
>  	context->type = AUDIT_KERN_MODULE;
>  }
>  

- RGB

--
Richard Guy Briggs <rgb@...hat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635