On 2017-04-26 1:40 AM, Jiri Pirko wrote:
Wed, Apr 26, 2017 at 07:03:23AM CEST, xiyou.wangc...@gmail.com wrote:
IPv6 tunnels use sizeof(struct in6_addr) as dev->addr_len,
but in many places especially bonding, we use struct sockaddr
to copy and set mac addr, this could lead to stack out-of-bounds
access.

Fix it by using a larger address storage.

Reported-by: Andrey Konovalov <andreyk...@google.com>
Cc: Jiri Pirko <j...@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
---
drivers/net/team/team.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 85c0124..88878f1 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -60,10 +60,13 @@ static struct team_port *team_port_get_rtnl(const struct 
net_device *dev)
static int __set_port_dev_addr(struct net_device *port_dev,
                               const unsigned char *dev_addr)
{
-       struct sockaddr addr;
+       struct {
+               unsigned short type;
+               unsigned char addr[MAX_ADDR_LEN];
+       } addr;

Wouldn't it make sense to define this struct somewhere in the core
headers?

We already have struct sockaddr_storage that could be used throughout this set as well. We just converted a few pieces of the bonding driver over to using it for better support of ipoib bonds, via commit faeeb317a5615076dff1ff44b51e862e6064dbd0. Might be better to just use that in both bonding and team, rather than having different per-driver structs, or Yet Another Address Storage implementation.

--
Jarod Wilson
ja...@redhat.com

Reply via email to