netdev - Re: [PATCH] net: rework recvmsg handler msg_name and 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] [day] [month] [year] [list]

Message-ID: <20131122064549.GA15822@order.stressinduktion.org>
Date:	Fri, 22 Nov 2013 07:45:49 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	David Miller <davem@...emloft.net>
Cc:	eric.dumazet@...il.com, vyasevich@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH] net: rework recvmsg handler msg_name and msg_namelen logic

On Thu, Nov 21, 2013 at 12:57:07PM -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@...il.com>
> Date: Thu, 21 Nov 2013 07:00:14 -0800
> 
> > So a protocol should use BUILD_BUG_ON() to make sure sockaddr_storage is
> > big enough.
> 
> Hannes mentioned trying to do this, somehow, automatically.  Best I could
> come up with is something like:
> 
> #define DECLARE_RECVMSG_HANDLER(name, ..., sockaddr_type) \
> static ... __name(...); \
> static ... name(...)	\
> {	\
> 	BUILD_BUG_ON(sizeof(sockaddr_type) > sizeof(sockaddr_storage); \
> 	__name(...); \
> } \
> static ... __name(...)
> 
> And then the protocols go:
> 
> DECLARE_RECVMSG_HANDLER(udp_recvmsg, ..., struct sockaddr_in)
> {
> 	struct inet_sock *inet = inet_sk(sk);
> 	struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;
>  ...
> 
> You get the idea.
> 
> Slightly convoluted, and we can do something special with types or the
> prot ops member name to enforce usage of the macro.

Actually we already have a simpler solution in the tree:

#define __sockaddr_check_size(size)     \
        BUILD_BUG_ON(((size) > sizeof(struct __kernel_sockaddr_storage)))

and

#define DECLARE_SOCKADDR(type, dst, src)        \
        type dst = ({ __sockaddr_check_size(sizeof(*dst)); (type) src; })

Unfortunately I missed it before (it only gets used in the getname functions).
So, some checks are already in place (af_packet, af_inet, llcp_sock, af_unix
and af_netlink).

I still want to explore my idea regarding coccinelle or sparse. Checking
these with sparse should be relatively straightforward. We would just
have to lift the msg_name pointer into its own address_space universe
and add a __force to the cast in DECLARE_SOCKADDR. But because sparse
errors are already high in numbers I fear it will be simply overlooked.

I'll want to test if a coccicheck is reasonable, too.

Will cook something up soon.

Greetings,

  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