netdev - Re: [PATCH] net: socket: do not validate msg_namelen unless msg

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]

Date:	Fri, 05 Sep 2014 23:14:20 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Ani Sinha <ani@...sta.com>, David Miller <davem@...emloft.net>,
	matthew.leach@....com
Cc:	netdev@...r.kernel.org, fenner <fenner@...sta.com>,
	fruggeri <fruggeri@...sta.com>, travisb <travisb@...sta.com>
Subject: Re: [PATCH] net: socket: do not validate msg_namelen unless msg_name is
 non-NULL

Hi,

On Fri, Sep 5, 2014, at 23:00, Ani Sinha wrote:
> Hi guys :
> 
> I am looking at the thread :
> 
>  [PATCH] net: socket: error on a negative msg_namelen
> 
> and the patch that was submitted in that thread :
> 
> commit dbb490b96584d4e958533fb637f08b557f505657
> Author: Matthew Leach <matthew.leach@....com>
> Date:   Tue Mar 11 11:58:27 2014 +0000
> 
>     net: socket: error on a negative msg_namelen
> 
> 
> According to the linux recvmsg manpage, the caller of recvmsg() may
> set msg_name to NULL if he does not care about source address but the
> manpage does not say that one has to set msg_namelen to 0 in this
> case. Essentially msg_namelen is a don't care if msg_name is NULL. I
> think in the kernel, we should validate msg_namelen only if the caller
> has also set msg_name and return EINVAL only when msg_name is non-null
> and msg_namelen is negative.
> 
> The following patch will do the intended :
> 
> 
> From ef8e8bd78635ac677f2d4b76fec9990ed1db763c Mon Sep 17 00:00:00 2001
> From: Ani Sinha <ani@...stanetworks.com>
> Date: Fri, 5 Sep 2014 13:25:22 -0700
> Subject:[PATCH] net: socket: do not validate msg_namelen unless
> msg_name is non-NULL
> 
> The value of msg_namelen in msghdr structure is irrelevant
>  when msg_name is NULL. We should not validate the value
>  passed in msg_namelen unless msg_name is non-NULL.
> 
> Signed-off-by: Ani Sinha <ani@...stanetworks.com>
> ---
>  net/socket.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/socket.c b/net/socket.c
> index 95ee7d8..a5dfe01 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1997,7 +1997,7 @@ static int copy_msghdr_from_user(struct msghdr
> *kmsg,
>   if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
>   return -EFAULT;
> 
> - if (kmsg->msg_namelen < 0)
> + if (kmsg->msg_name && kmsg->msg_namelen < 0)
>   return -EINVAL;
> 
>   if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))

The reason for the above mentioned commit was the signed/unsigned
conversion by this check. To not trigger any static checker tools, I
would suggest to just set kmsg->msg_namelen to zero in case msg_name is
NULL.

Thanks,
Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html