netdev - Re: [PATCH] net: avoid put_cmsg() possible copy longer data than input

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] [day] [month] [year] [list]

Date:   Wed, 28 Dec 2016 14:48:40 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     cugyly@....com
Cc:     netdev@...r.kernel.org, Linyu.Yuan@...atel-sbell.com.cn
Subject: Re: [PATCH] net: avoid put_cmsg() possible copy longer data than
 input

From: yuan linyu <cugyly@....com>
Date: Wed, 28 Dec 2016 22:34:23 +0800

> From: yuan linyu <Linyu.Yuan@...atel-sbell.com.cn>
> 
> if CMSG_ALIGN(sizeof(struct cmsghdr)) > sizeof(struct cmsghdr),
> original (cmlen - sizeof(struct cmsghdr)) may greater than
> input len.

You are doing a lot of unrelated cleanups in this change.  This
makes it hard to review.

The important parts of the fix seems to be the added checks to make
sure that we don't access the CMSG_DATA() unless we have more than
CMSG_ALIGN(sizeof(struct cmsghdr)) bytes.

I think you can fix that with a few one-line tests rather than
restructuring all of the CMSG_*() macros.

Also:

> @@ -223,7 +223,7 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
>  	if (MSG_CMSG_COMPAT & msg->msg_flags)
>  		return put_cmsg_compat(msg, level, type, len, data);
>  
> -	if (cm==NULL || msg->msg_controllen < sizeof(*cm)) {
> +	if (cm == NULL || msg->msg_controllen < sizeof(*cm)) {
>  		msg->msg_flags |= MSG_CTRUNC;
>  		return 0; /* XXX: return error? check spec. */
>  	}

This is a coding style fix unrelated to the purpose of this change.

Thanks.