Message ID | 20190429154655.9141-1-willemdebruijn.kernel@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net,v2] packet: in recvmsg msg_name return at least sizeof sockaddr_ll | expand |
From: Willem de Bruijn > Sent: 29 April 2019 16:47 > Packet send checks that msg_name is at least sizeof sockaddr_ll. > Packet recv must return at least this length, so that its output > can be passed unmodified to packet send. > > This ceased to be true since adding support for lladdr longer than > sll_addr. Since, the return value uses true address length. > > Always return at least sizeof sockaddr_ll, even if address length > is shorter. Zero the padding bytes. > > Change v1->v2: do not overwrite zeroed padding again. use copy_len. > > Fixes: 0fb375fb9b93 ("[AF_PACKET]: Allow for > 8 byte hardware addresses.") > Suggested-by: David Laight <David.Laight@aculab.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> > --- > net/packet/af_packet.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index e726aaba73b9f..5fe3d75b6212d 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -3348,20 +3348,29 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, > sock_recv_ts_and_drops(msg, sk, skb); > > if (msg->msg_name) { > + int copy_len; > + > /* If the address length field is there to be filled > * in, we fill it in now. > */ > if (sock->type == SOCK_PACKET) { > __sockaddr_check_size(sizeof(struct sockaddr_pkt)); > msg->msg_namelen = sizeof(struct sockaddr_pkt); > + copy_len = msg->msg_namelen; > } else { > struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll; > > msg->msg_namelen = sll->sll_halen + > offsetof(struct sockaddr_ll, sll_addr); > + copy_len = msg->msg_namelen; > + if (msg->msg_namelen < sizeof(struct sockaddr_ll)) { > + memset(msg->msg_name + > + offsetof(struct sockaddr_ll, sll_addr), > + 0, sizeof(sll->sll_addr)); > + msg->msg_namelen = sizeof(struct sockaddr_ll); > + } > } > - memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa, > - msg->msg_namelen); > + memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa, copy_len); > } > > if (pkt_sk(sk)->auxdata) { > -- > 2.21.0.593.g511ec345e18-goog Looks ok to me, not tried to compile it though. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Apr 29, 2019 at 11:49 AM David Laight <David.Laight@aculab.com> wrote: > > From: Willem de Bruijn > > Sent: 29 April 2019 16:47 > > Packet send checks that msg_name is at least sizeof sockaddr_ll. > > Packet recv must return at least this length, so that its output > > can be passed unmodified to packet send. > > > > This ceased to be true since adding support for lladdr longer than > > sll_addr. Since, the return value uses true address length. > > > > Always return at least sizeof sockaddr_ll, even if address length > > is shorter. Zero the padding bytes. > > > > Change v1->v2: do not overwrite zeroed padding again. use copy_len. > > > > Fixes: 0fb375fb9b93 ("[AF_PACKET]: Allow for > 8 byte hardware addresses.") > > Suggested-by: David Laight <David.Laight@aculab.com> > > Signed-off-by: Willem de Bruijn <willemb@google.com> > > --- .. > > Looks ok to me, not tried to compile it though. Thanks again. I did that and also ran a small recv test that verifies namelen (but clearly did not help me see the stupid bug I made in v1..).
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: Mon, 29 Apr 2019 11:46:55 -0400 > From: Willem de Bruijn <willemb@google.com> > > Packet send checks that msg_name is at least sizeof sockaddr_ll. > Packet recv must return at least this length, so that its output > can be passed unmodified to packet send. > > This ceased to be true since adding support for lladdr longer than > sll_addr. Since, the return value uses true address length. > > Always return at least sizeof sockaddr_ll, even if address length > is shorter. Zero the padding bytes. > > Change v1->v2: do not overwrite zeroed padding again. use copy_len. > > Fixes: 0fb375fb9b93 ("[AF_PACKET]: Allow for > 8 byte hardware addresses.") > Suggested-by: David Laight <David.Laight@aculab.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Applied and queued up for -stable.
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index e726aaba73b9f..5fe3d75b6212d 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3348,20 +3348,29 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, sock_recv_ts_and_drops(msg, sk, skb); if (msg->msg_name) { + int copy_len; + /* If the address length field is there to be filled * in, we fill it in now. */ if (sock->type == SOCK_PACKET) { __sockaddr_check_size(sizeof(struct sockaddr_pkt)); msg->msg_namelen = sizeof(struct sockaddr_pkt); + copy_len = msg->msg_namelen; } else { struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll; msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr); + copy_len = msg->msg_namelen; + if (msg->msg_namelen < sizeof(struct sockaddr_ll)) { + memset(msg->msg_name + + offsetof(struct sockaddr_ll, sll_addr), + 0, sizeof(sll->sll_addr)); + msg->msg_namelen = sizeof(struct sockaddr_ll); + } } - memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa, - msg->msg_namelen); + memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa, copy_len); } if (pkt_sk(sk)->auxdata) {