[PATCH] sockaddr_ll change for IPoIB interface

[PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

Hi,

This is a repost of a patch which was posted last week which appears to
have been lost in the shuffle.

The patch below is to accomodate IPoIB link layer address in the
sockaddr_ll struct so that user space can send and receive IPoIB link
later packets. Unfortunately, IPoIB has 20 bytes LL addresses rather
than the 8 byte MAC addresses (or under) used by other LLs.

There is a similar change to both:
/usr/include/linux/if_packet.h
/usr/include/netpacket/packet.h
as in:
include/linux/if_packet.h below
to increase sll_addr from 8 to 20 bytes.

Thanks.

-- Hal

sockaddr_ll changes to accomodate IPoIB interfaces.
This is due to the fact that the IPoIB link layer
address is 20 bytes rather than 8 bytes. With the current 8 byte
address, it is not possible to send ARPs and RARPs from userspace as the
broadcast and unicast IPoIB addresses cannot be supplied properly.
There is backward compatibility support for those applications built
with the existing structure (prior to this patch).

Signed-off-by: Hal Rosenstock <halr@voltaire.com>

--- include/linux/if_packet.h.orig	2005-06-29 19:00:53.000000000 -0400
+++ include/linux/if_packet.h	2005-08-05 10:04:06.000000000 -0400
@@ -8,6 +8,7 @@ struct sockaddr_pkt
 	unsigned short spkt_protocol;
 };
 
+#define SOCKADDR_LL_COMPAT	12
 struct sockaddr_ll
 {
 	unsigned short	sll_family;
@@ -16,7 +17,7 @@ struct sockaddr_ll
 	unsigned short	sll_hatype;
 	unsigned char	sll_pkttype;
 	unsigned char	sll_halen;
-	unsigned char	sll_addr[8];
+	unsigned char	sll_addr[20];
 };
 
 /* Packet types */

--- af_packet.c.orig	2005-06-29 19:00:53.000000000 -0400
+++ af_packet.c	2005-08-05 13:28:49.000000000 -0400
@@ -708,8 +708,12 @@ static int packet_sendmsg(struct kiocb *
 		addr	= NULL;
 	} else {
 		err = -EINVAL;
-		if (msg->msg_namelen < sizeof(struct sockaddr_ll))
-			goto out;
+		if (msg->msg_namelen < sizeof(struct sockaddr_ll)) {
+			/* Support for older sockaddr_ll structs */
+			if ((msg->msg_namelen != sizeof(struct sockaddr_ll) - SOCKADDR_LL_COMPAT) ||
+			    (saddr->sll_hatype == ARPHRD_INFINIBAND))
+				goto out;
+		}
 		ifindex	= saddr->sll_ifindex;
 		proto	= saddr->sll_protocol;
 		addr	= saddr->sll_addr;
@@ -937,7 +941,11 @@ static int packet_bind(struct socket *so
 	 */
 	 
 	if (addr_len < sizeof(struct sockaddr_ll))
-		return -EINVAL;
+		/* Support for older sockaddr_ll structs */
+		if ((addr_len != sizeof(struct sockaddr_ll) -
+				 SOCKADDR_LL_COMPAT) || 
+		    (sll->sll_hatype == ARPHRD_INFINIBAND))
+			return -EINVAL;
 	if (sll->sll_family != AF_PACKET)
 		return -EINVAL;
 

Re: [PATCH] sockaddr_ll change for IPoIB interface

[David S. Miller]

From: Hal Rosenstock <halr@voltaire.com>
Date: 11 Aug 2005 14:48:37 -0400

> The patch below is to accomodate IPoIB link layer address in the
> sockaddr_ll struct so that user space can send and receive IPoIB link
> later packets. Unfortunately, IPoIB has 20 bytes LL addresses rather
> than the 8 byte MAC addresses (or under) used by other LLs.

Two problems.  1) it's a really ugly IPoIB specific hack to extend the
sockaddr_ll structure, it won't work for anything else without adding
more special tests to that af_packet.c code and 2) you inproperly
rooted your patch, so we get stuff like this:

> --- af_packet.c.orig	2005-06-29 19:00:53.000000000 -0400
> +++ af_packet.c	2005-08-05 13:28:49.000000000 -0400

Please find another way to extend the structure.

That's why I didn't respond, this patch was too ugly for words
so it went to the bottom of my prioritized list of things to
do.

[PATCH] af_packet: Allow for > 8 byte hardware addresses.

[Eric W. Biederman]

Does this approach look sound?

The convention it adopts is that longer addresses will simply extend
the hardeware address byte arrays at the end of sockaddr_ll and
packet_mreq.

In making this change a small information leak was also closed.
The code only initializes the hardware address bytes that are
used, but all of struct sockaddr_ll was copied to userspace.
Now we just copy sockaddr_ll to the last byte of the hardware
address used.  

Hopefully for the common case of ethernet returning a value that is 
2 bytes smaller than sockaddr_ll won't break anything.  Given that it
simplifies the code and removes an information leak I thought
the small chance of breakage was worth it.  If not this can
be easily fixed.

For error checking larger structures than our internal
maximums continue to be allowed but an error is signaled if we can
not fit the hardware address into our internal structure.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>


---

 net/packet/af_packet.c |   64 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 47 insertions(+), 17 deletions(-)

39a41e0363bab6661f40f24b404416b99de0c15a
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -36,6 +36,11 @@
  *	Michal Ostrowski        :       Module initialization cleanup.
  *         Ulises Alonso        :       Frame number limit removal and 
  *                                      packet_set_ring memory leak.
+ *		Eric Biederman	:	Allow for > 8 byte hardware addresses.
+ *					The convention is that longer addresses
+ *					will simply extend the hardware address
+ *					byte arrays at the end of sockaddr_ll 
+ *					and packet_mreq.
  *
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License
@@ -161,7 +166,17 @@ struct packet_mclist
 	int			count;
 	unsigned short		type;
 	unsigned short		alen;
-	unsigned char		addr[8];
+	unsigned char		addr[MAX_ADDR_LEN];
+};
+/* identical to struct packet_mreq except it has
+ * a longer address field.
+ */
+struct packet_mreq_max
+{
+	int		mr_ifindex;
+	unsigned short	mr_type;
+	unsigned short	mr_alen;
+	unsigned char	mr_address[MAX_ADDR_LEN];
 };
 #endif
 #ifdef CONFIG_PACKET_MMAP
@@ -716,6 +731,8 @@ static int packet_sendmsg(struct kiocb *
 		err = -EINVAL;
 		if (msg->msg_namelen < sizeof(struct sockaddr_ll))
 			goto out;
+		if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr)))
+			goto out;
 		ifindex	= saddr->sll_ifindex;
 		proto	= saddr->sll_protocol;
 		addr	= saddr->sll_addr;
@@ -744,6 +761,12 @@ static int packet_sendmsg(struct kiocb *
 	if (dev->hard_header) {
 		int res;
 		err = -EINVAL;
+		if (saddr) {
+			if (saddr->sll_halen != dev->addr_len)
+				goto out_free;
+			if (saddr->sll_hatype != dev->type)
+				goto out_free;
+		}
 		res = dev->hard_header(skb, dev, ntohs(proto), addr, NULL, len);
 		if (sock->type != SOCK_DGRAM) {
 			skb->tail = skb->data;
@@ -1045,6 +1068,7 @@ static int packet_recvmsg(struct kiocb *
 	struct sock *sk = sock->sk;
 	struct sk_buff *skb;
 	int copied, err;
+	struct sockaddr_ll *sll;
 
 	err = -EINVAL;
 	if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
@@ -1057,16 +1081,6 @@ static int packet_recvmsg(struct kiocb *
 #endif
 
 	/*
-	 *	If the address length field is there to be filled in, we fill
-	 *	it in now.
-	 */
-
-	if (sock->type == SOCK_PACKET)
-		msg->msg_namelen = sizeof(struct sockaddr_pkt);
-	else
-		msg->msg_namelen = sizeof(struct sockaddr_ll);
-
-	/*
 	 *	Call the generic datagram receiver. This handles all sorts
 	 *	of horrible races and re-entrancy so we can forget about it
 	 *	in the protocol layers.
@@ -1087,6 +1101,17 @@ static int packet_recvmsg(struct kiocb *
 		goto out;
 
 	/*
+	 *	If the address length field is there to be filled in, we fill
+	 *	it in now.
+	 */
+
+	sll = (struct sockaddr_ll*)skb->cb;
+	if (sock->type == SOCK_PACKET)
+		msg->msg_namelen = sizeof(struct sockaddr_pkt);
+	else
+		msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr);
+
+	/*
 	 *	You lose any data beyond the buffer you gave. If it worries a
 	 *	user program they can ask the device for its MTU anyway.
 	 */
@@ -1166,7 +1191,7 @@ static int packet_getname(struct socket 
 		sll->sll_hatype = 0;	/* Bad: we have no ARPHRD_UNSPEC */
 		sll->sll_halen = 0;
 	}
-	*uaddr_len = sizeof(*sll);
+	*uaddr_len = offsetof(struct sockaddr_ll, sll_addr) + sll->sll_halen;
 
 	return 0;
 }
@@ -1199,7 +1224,7 @@ static void packet_dev_mclist(struct net
 	}
 }
 
-static int packet_mc_add(struct sock *sk, struct packet_mreq *mreq)
+static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
 {
 	struct packet_sock *po = pkt_sk(sk);
 	struct packet_mclist *ml, *i;
@@ -1249,7 +1274,7 @@ done:
 	return err;
 }
 
-static int packet_mc_drop(struct sock *sk, struct packet_mreq *mreq)
+static int packet_mc_drop(struct sock *sk, struct packet_mreq_max *mreq)
 {
 	struct packet_mclist *ml, **mlp;
 
@@ -1315,11 +1340,16 @@ packet_setsockopt(struct socket *sock, i
 	case PACKET_ADD_MEMBERSHIP:	
 	case PACKET_DROP_MEMBERSHIP:
 	{
-		struct packet_mreq mreq;
-		if (optlen<sizeof(mreq))
+		struct packet_mreq_max mreq;
+		int len = optlen;
+		if (len < sizeof(struct packet_mreq))
 			return -EINVAL;
-		if (copy_from_user(&mreq,optval,sizeof(mreq)))
+		if (len > sizeof(mreq))
+			len = sizeof(mreq);
+		if (copy_from_user(&mreq,optval,len))
 			return -EFAULT;
+		if (len < (mreq.mr_alen + offsetof(struct packet_mreq, mr_address)))
+			return -EINVAL;
 		if (optname == PACKET_ADD_MEMBERSHIP)
 			ret = packet_mc_add(sk, &mreq);
 		else

Re: [PATCH] af_packet: Allow for > 8 byte hardware addresses.

[David S. Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sat, 10 Sep 2005 11:25:27 -0600

> @@ -1315,11 +1340,16 @@ packet_setsockopt(struct socket *sock, i
>  	case PACKET_ADD_MEMBERSHIP:	
>  	case PACKET_DROP_MEMBERSHIP:
>  	{
> -		struct packet_mreq mreq;
> -		if (optlen<sizeof(mreq))
> +		struct packet_mreq_max mreq;
> +		int len = optlen;
> +		if (len < sizeof(struct packet_mreq))
>  			return -EINVAL;
> -		if (copy_from_user(&mreq,optval,sizeof(mreq)))
> +		if (len > sizeof(mreq))
> +			len = sizeof(mreq);
> +		if (copy_from_user(&mreq,optval,len))
>  			return -EFAULT;

I would suggest memset()'ing out any packet_mreq_max structure,
before copying a smaller amount of data into it, just to be
safe.  Please check this out in all such possible uses in
the patch.

Thanks.

Re: [PATCH] af_packet: Allow for > 8 byte hardware addresses.

[Eric W. Biederman]

"David S. Miller" <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Sat, 10 Sep 2005 11:25:27 -0600
>
>> @@ -1315,11 +1340,16 @@ packet_setsockopt(struct socket *sock, i
>>  	case PACKET_ADD_MEMBERSHIP:	
>>  	case PACKET_DROP_MEMBERSHIP:
>>  	{
>> -		struct packet_mreq mreq;
>> -		if (optlen<sizeof(mreq))
>> +		struct packet_mreq_max mreq;
>> +		int len = optlen;
>> +		if (len < sizeof(struct packet_mreq))
>>  			return -EINVAL;
>> -		if (copy_from_user(&mreq,optval,sizeof(mreq)))
>> +		if (len > sizeof(mreq))
>> +			len = sizeof(mreq);
>> +		if (copy_from_user(&mreq,optval,len))
>>  			return -EFAULT;
>
> I would suggest memset()'ing out any packet_mreq_max structure,
> before copying a smaller amount of data into it, just to be
> safe.  Please check this out in all such possible uses in
> the patch.
>
> Thanks.

Ok.  For that specific case you have quoted the only instance.

In a practical sense it doesn't matter because halen determines
how many of the bytes we actually look at.  But if something
is buggy I can see the memset causing the bug to act in a
more deterministic fashion.

Updated patch will follow in a bit.

Eric

Re: [PATCH] af_packet: Allow for > 8 byte hardware addresses.

[David S. Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 12 Sep 2005 16:13:23 -0600

> Updated patch will follow in a bit.

Thanks for following up on this Eric.

[PATCH] af_packet: Allow for > 8 byte hardware addresses.

[Eric W. Biederman]

Dave sorry for the delay getting back to this...
This version of the patch adds the one memset you were clearly 
asking for.

The convention is that longer addresses will simply extend
the hardeware address byte arrays at the end of sockaddr_ll and
packet_mreq.

In making this change a small information leak was also closed.
The code only initializes the hardware address bytes that are
used, but all of struct sockaddr_ll was copied to userspace.
Now we just copy sockaddr_ll to the last byte of the hardware
address used.

For error checking larger structures than our internal
maximums continue to be allowed but an error is signaled if we can
not fit the hardware address into our internal structure.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>


---

 net/packet/af_packet.c |   65 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 48 insertions(+), 17 deletions(-)

0117e4931d1884ae71c74378590bde4cc76f403a
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -36,6 +36,11 @@
  *	Michal Ostrowski        :       Module initialization cleanup.
  *         Ulises Alonso        :       Frame number limit removal and 
  *                                      packet_set_ring memory leak.
+ *		Eric Biederman	:	Allow for > 8 byte hardware addresses.
+ *					The convention is that longer addresses
+ *					will simply extend the hardware address
+ *					byte arrays at the end of sockaddr_ll 
+ *					and packet_mreq.
  *
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License
@@ -161,7 +166,17 @@ struct packet_mclist
 	int			count;
 	unsigned short		type;
 	unsigned short		alen;
-	unsigned char		addr[8];
+	unsigned char		addr[MAX_ADDR_LEN];
+};
+/* identical to struct packet_mreq except it has
+ * a longer address field.
+ */
+struct packet_mreq_max
+{
+	int		mr_ifindex;
+	unsigned short	mr_type;
+	unsigned short	mr_alen;
+	unsigned char	mr_address[MAX_ADDR_LEN];
 };
 #endif
 #ifdef CONFIG_PACKET_MMAP
@@ -716,6 +731,8 @@ static int packet_sendmsg(struct kiocb *
 		err = -EINVAL;
 		if (msg->msg_namelen < sizeof(struct sockaddr_ll))
 			goto out;
+		if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr)))
+			goto out;
 		ifindex	= saddr->sll_ifindex;
 		proto	= saddr->sll_protocol;
 		addr	= saddr->sll_addr;
@@ -744,6 +761,12 @@ static int packet_sendmsg(struct kiocb *
 	if (dev->hard_header) {
 		int res;
 		err = -EINVAL;
+		if (saddr) {
+			if (saddr->sll_halen != dev->addr_len)
+				goto out_free;
+			if (saddr->sll_hatype != dev->type)
+				goto out_free;
+		}
 		res = dev->hard_header(skb, dev, ntohs(proto), addr, NULL, len);
 		if (sock->type != SOCK_DGRAM) {
 			skb->tail = skb->data;
@@ -1045,6 +1068,7 @@ static int packet_recvmsg(struct kiocb *
 	struct sock *sk = sock->sk;
 	struct sk_buff *skb;
 	int copied, err;
+	struct sockaddr_ll *sll;
 
 	err = -EINVAL;
 	if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
@@ -1057,16 +1081,6 @@ static int packet_recvmsg(struct kiocb *
 #endif
 
 	/*
-	 *	If the address length field is there to be filled in, we fill
-	 *	it in now.
-	 */
-
-	if (sock->type == SOCK_PACKET)
-		msg->msg_namelen = sizeof(struct sockaddr_pkt);
-	else
-		msg->msg_namelen = sizeof(struct sockaddr_ll);
-
-	/*
 	 *	Call the generic datagram receiver. This handles all sorts
 	 *	of horrible races and re-entrancy so we can forget about it
 	 *	in the protocol layers.
@@ -1087,6 +1101,17 @@ static int packet_recvmsg(struct kiocb *
 		goto out;
 
 	/*
+	 *	If the address length field is there to be filled in, we fill
+	 *	it in now.
+	 */
+
+	sll = (struct sockaddr_ll*)skb->cb;
+	if (sock->type == SOCK_PACKET)
+		msg->msg_namelen = sizeof(struct sockaddr_pkt);
+	else
+		msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr);
+
+	/*
 	 *	You lose any data beyond the buffer you gave. If it worries a
 	 *	user program they can ask the device for its MTU anyway.
 	 */
@@ -1166,7 +1191,7 @@ static int packet_getname(struct socket 
 		sll->sll_hatype = 0;	/* Bad: we have no ARPHRD_UNSPEC */
 		sll->sll_halen = 0;
 	}
-	*uaddr_len = sizeof(*sll);
+	*uaddr_len = offsetof(struct sockaddr_ll, sll_addr) + sll->sll_halen;
 
 	return 0;
 }
@@ -1199,7 +1224,7 @@ static void packet_dev_mclist(struct net
 	}
 }
 
-static int packet_mc_add(struct sock *sk, struct packet_mreq *mreq)
+static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
 {
 	struct packet_sock *po = pkt_sk(sk);
 	struct packet_mclist *ml, *i;
@@ -1249,7 +1274,7 @@ done:
 	return err;
 }
 
-static int packet_mc_drop(struct sock *sk, struct packet_mreq *mreq)
+static int packet_mc_drop(struct sock *sk, struct packet_mreq_max *mreq)
 {
 	struct packet_mclist *ml, **mlp;
 
@@ -1315,11 +1340,17 @@ packet_setsockopt(struct socket *sock, i
 	case PACKET_ADD_MEMBERSHIP:	
 	case PACKET_DROP_MEMBERSHIP:
 	{
-		struct packet_mreq mreq;
-		if (optlen<sizeof(mreq))
+		struct packet_mreq_max mreq;
+		int len = optlen;
+		memset(&mreq, 0, sizeof(mreq));
+		if (len < sizeof(struct packet_mreq))
 			return -EINVAL;
-		if (copy_from_user(&mreq,optval,sizeof(mreq)))
+		if (len > sizeof(mreq))
+			len = sizeof(mreq);
+		if (copy_from_user(&mreq,optval,len))
 			return -EFAULT;
+		if (len < (mreq.mr_alen + offsetof(struct packet_mreq, mr_address)))
+			return -EINVAL;
 		if (optname == PACKET_ADD_MEMBERSHIP)
 			ret = packet_mc_add(sk, &mreq);
 		else

[PATCH] [NET] socket.c: zero socket addresses before use.

[Eric W. Biederman]

Dave I don't know if this is part of what you want but
zeroing the socket address buffer before use seem to be implied
by what you were asking for.   So here is an additional patch
to implement that.

This is a paranoid precaution to guard against accidental
information leaks to user space or other consumers/producers
may fail to properly fail to set or read the hardware
address length. af_packet over ethernet has had at least
has one small but in this respect.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>


---

 net/socket.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

957ae0f034aa1482e42da948b2d87ae6fc13366e
diff --git a/net/socket.c b/net/socket.c
--- a/net/socket.c
+++ b/net/socket.c
@@ -1285,6 +1285,7 @@ asmlinkage long sys_bind(int fd, struct 
 	char address[MAX_SOCK_ADDR];
 	int err;
 
+	memset(address, 0, sizeof(address));
 	if((sock = sockfd_lookup(fd,&err))!=NULL)
 	{
 		if((err=move_addr_to_kernel(umyaddr,addrlen,address))>=0) {
@@ -1349,6 +1350,7 @@ asmlinkage long sys_accept(int fd, struc
 	int err, len;
 	char address[MAX_SOCK_ADDR];
 
+	memset(address, 0, sizeof(address));
 	sock = sockfd_lookup(fd, &err);
 	if (!sock)
 		goto out;
@@ -1419,6 +1421,7 @@ asmlinkage long sys_connect(int fd, stru
 	char address[MAX_SOCK_ADDR];
 	int err;
 
+	memset(address, 0, sizeof(address));
 	sock = sockfd_lookup(fd, &err);
 	if (!sock)
 		goto out;
@@ -1449,6 +1452,7 @@ asmlinkage long sys_getsockname(int fd, 
 	char address[MAX_SOCK_ADDR];
 	int len, err;
 	
+	memset(address, 0, sizeof(address));
 	sock = sockfd_lookup(fd, &err);
 	if (!sock)
 		goto out;
@@ -1479,6 +1483,7 @@ asmlinkage long sys_getpeername(int fd, 
 	char address[MAX_SOCK_ADDR];
 	int len, err;
 
+	memset(address, 0, sizeof(address));
 	if ((sock = sockfd_lookup(fd, &err))!=NULL)
 	{
 		err = security_socket_getpeername(sock);
@@ -1510,6 +1515,7 @@ asmlinkage long sys_sendto(int fd, void 
 	struct msghdr msg;
 	struct iovec iov;
 	
+	memset(address, 0, sizeof(address));
 	sock = sockfd_lookup(fd, &err);
 	if (!sock)
 		goto out;
@@ -1564,6 +1570,7 @@ asmlinkage long sys_recvfrom(int fd, voi
 	char address[MAX_SOCK_ADDR];
 	int err,err2;
 
+	memset(address, 0, sizeof(address));
 	sock = sockfd_lookup(fd, &err);
 	if (!sock)
 		goto out;
@@ -1705,6 +1712,7 @@ asmlinkage long sys_sendmsg(int fd, stru
 	struct msghdr msg_sys;
 	int err, ctl_len, iov_size, total_len;
 	
+	memset(address, 0, sizeof(address));
 	err = -EFAULT;
 	if (MSG_CMSG_COMPAT & flags) {
 		if (get_compat_msghdr(&msg_sys, msg_compat))
@@ -1806,6 +1814,7 @@ asmlinkage long sys_recvmsg(int fd, stru
 	struct sockaddr __user *uaddr;
 	int __user *uaddr_len;
 	
+	memset(addr, 0, sizeof(addr));
 	if (MSG_CMSG_COMPAT & flags) {
 		if (get_compat_msghdr(&msg_sys, msg_compat))
 			return -EFAULT;

Re: [PATCH] af_packet: Allow for > 8 byte hardware addresses.

[David S. Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 20 Sep 2005 11:17:14 -0600

> Dave sorry for the delay getting back to this...
> This version of the patch adds the one memset you were clearly 
> asking for.

Applied, thanks a lot Eric.

Re: [PATCH] [NET] socket.c: zero socket addresses before use.

[David S. Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 20 Sep 2005 11:18:23 -0600

> Dave I don't know if this is part of what you want but
> zeroing the socket address buffer before use seem to be implied
> by what you were asking for.   So here is an additional patch
> to implement that.
> 
> This is a paranoid precaution to guard against accidental
> information leaks to user space or other consumers/producers
> may fail to properly fail to set or read the hardware
> address length. af_packet over ethernet has had at least
> has one small but in this respect.

I think this patch might be a bit overkill, but thanks for cooking it
up.  I'm willing to be convinced otherwise though :-)

Re: [PATCH] [NET] socket.c: zero socket addresses before use.

[Eric W. Biederman]

"David S. Miller" <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Tue, 20 Sep 2005 11:18:23 -0600
>
>> Dave I don't know if this is part of what you want but
>> zeroing the socket address buffer before use seem to be implied
>> by what you were asking for.   So here is an additional patch
>> to implement that.
>> 
>> This is a paranoid precaution to guard against accidental
>> information leaks to user space or other consumers/producers
>> may fail to properly fail to set or read the hardware
>> address length. af_packet over ethernet has had at least
>> has one small but in this respect.
>
> I think this patch might be a bit overkill, but thanks for cooking it
> up.  I'm willing to be convinced otherwise though :-)

Personally I agree. But if we don't we probably need to audit
all of the other protocols besides af_packet and see if
they could possible have the problem that af_packet did
with struct sockaddr_ll.

What happened is struct sockaddr_ll had an array of 8 bytes
where it placed a hardware address.  It reported to socket.c
the length of struct sockaddr_ll for returning to user space.
The code then only filled in enough bytes for the actual hardware
address.  6 bytes in the common case of ethernet.  So only 6 bytes
were ever written and we returned 8 bytes to user space.

The transmission is similar except the kernel code was responsible
for simply not caring about the additional bytes.  But confused
kernel code could have the problem.

This is the same problem you were concerned about with 
my new struct packet_mreq_max.  The only way to take the same
precautions on the other code paths is to modify socket.c

I am concerned enough to point out the possibility and send
a patch to add some memset() as a cheap insurance plan.  Auditing
the address handling for all of the rest of the network protcols is
more than I am ready to volunteer for.

Eric