* [Patch net] rtnetlink: allocate more memory for dev_set_mac_address()
@ 2017-07-20 18:27 Cong Wang
2017-07-20 18:27 ` [Patch net] net: check mac address length " Cong Wang
2017-07-20 22:23 ` [Patch net] rtnetlink: allocate more memory " David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-20 18:27 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Cong Wang
virtnet_set_mac_address() interprets mac address as struct
sockaddr, but upper layer only allocates dev->addr_len
which is ETH_ALEN + sizeof(sa_family_t) in this case.
We lack a unified definition for mac address, so just fix
the upper layer, this also allows drivers to interpret it
to struct sockaddr freely.
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/core/rtnetlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d1ba90980be1..ebaa26cf777f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2031,7 +2031,8 @@ static int do_setlink(const struct sk_buff *skb,
struct sockaddr *sa;
int len;
- len = sizeof(sa_family_t) + dev->addr_len;
+ len = sizeof(sa_family_t) + max_t(size_t, dev->addr_len,
+ sizeof(*sa));
sa = kmalloc(len, GFP_KERNEL);
if (!sa) {
err = -ENOMEM;
--
2.13.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net] net: check mac address length for dev_set_mac_address()
2017-07-20 18:27 [Patch net] rtnetlink: allocate more memory for dev_set_mac_address() Cong Wang
@ 2017-07-20 18:27 ` Cong Wang
2017-07-20 23:39 ` Cong Wang
2017-07-21 18:50 ` Cong Wang
2017-07-20 22:23 ` [Patch net] rtnetlink: allocate more memory " David Miller
1 sibling, 2 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-20 18:27 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Cong Wang, Jiri Pirko
dev_set_mac_address() accepts a struct sockaddr pointer as
input but we have various types of mac addresse whose lengths
are up to MAX_ADDR_LEN, this is confusing.
Make it void like ->ndo_set_mac_address() and let callers check
its length before calling it. It is too late to fix dev_ifsioc()
due to API compatibility, so just reject those larger than
sizeof(struct sockaddr).
Fortunately, only a few IPv6 tunnel devices have addr_len
larger than sizeof(struct sockaddr) and they don't support
ndo_set_mac_addr(). But team driver seems still buggy without
this patch.
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/netdevice.h | 2 +-
net/core/dev.c | 10 +++++++---
net/core/dev_ioctl.c | 2 ++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 779b23595596..d7e872fa4656 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3287,7 +3287,7 @@ int dev_change_net_namespace(struct net_device *, struct net *, const char *);
int __dev_set_mtu(struct net_device *, int);
int dev_set_mtu(struct net_device *, int);
void dev_set_group(struct net_device *, int);
-int dev_set_mac_address(struct net_device *, struct sockaddr *);
+int dev_set_mac_address(struct net_device *, void *);
int dev_change_carrier(struct net_device *, bool new_carrier);
int dev_get_phys_port_id(struct net_device *dev,
struct netdev_phys_item_id *ppid);
diff --git a/net/core/dev.c b/net/core/dev.c
index 02440518dd69..1802303bd0a7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6844,13 +6844,17 @@ EXPORT_SYMBOL(dev_set_group);
/**
* dev_set_mac_address - Change Media Access Control Address
* @dev: device
- * @sa: new address
+ * @addr: new address, whose type could be either struct sockaddr or
+ * any other compatible type whose length is up to MAX_ADDR_LEN depending
+ * on the dev->addr_len. Callers should check if its length is smaller than
+ * dev->addr_len!!
*
* Change the hardware (MAC) address of the device
*/
-int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
+int dev_set_mac_address(struct net_device *dev, void *addr)
{
const struct net_device_ops *ops = dev->netdev_ops;
+ struct sockaddr *sa = addr;
int err;
if (!ops->ndo_set_mac_address)
@@ -6859,7 +6863,7 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
return -EINVAL;
if (!netif_device_present(dev))
return -ENODEV;
- err = ops->ndo_set_mac_address(dev, sa);
+ err = ops->ndo_set_mac_address(dev, addr);
if (err)
return err;
dev->addr_assign_type = NET_ADDR_SET;
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 82fd4c9c4a1b..3f41601d7b7c 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -262,6 +262,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
return dev_set_mtu(dev, ifr->ifr_mtu);
case SIOCSIFHWADDR:
+ if (dev->addr_len > sizeof(struct sockaddr))
+ return -EINVAL;
return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
case SIOCSIFHWBROADCAST:
--
2.13.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch net] rtnetlink: allocate more memory for dev_set_mac_address()
2017-07-20 18:27 [Patch net] rtnetlink: allocate more memory for dev_set_mac_address() Cong Wang
2017-07-20 18:27 ` [Patch net] net: check mac address length " Cong Wang
@ 2017-07-20 22:23 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2017-07-20 22:23 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, dsahern
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 20 Jul 2017 11:27:57 -0700
> virtnet_set_mac_address() interprets mac address as struct
> sockaddr, but upper layer only allocates dev->addr_len
> which is ETH_ALEN + sizeof(sa_family_t) in this case.
>
> We lack a unified definition for mac address, so just fix
> the upper layer, this also allows drivers to interpret it
> to struct sockaddr freely.
>
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable, thanks Cong.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch net] net: check mac address length for dev_set_mac_address()
2017-07-20 18:27 ` [Patch net] net: check mac address length " Cong Wang
@ 2017-07-20 23:39 ` Cong Wang
2017-07-21 18:50 ` Cong Wang
1 sibling, 0 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-20 23:39 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: David Ahern, Cong Wang, Jiri Pirko
On Thu, Jul 20, 2017 at 11:27 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> dev_set_mac_address() accepts a struct sockaddr pointer as
> input but we have various types of mac addresse whose lengths
> are up to MAX_ADDR_LEN, this is confusing.
>
> Make it void like ->ndo_set_mac_address() and let callers check
> its length before calling it. It is too late to fix dev_ifsioc()
> due to API compatibility, so just reject those larger than
> sizeof(struct sockaddr).
>
> Fortunately, only a few IPv6 tunnel devices have addr_len
> larger than sizeof(struct sockaddr) and they don't support
> ndo_set_mac_addr(). But team driver seems still buggy without
> this patch.
Note, in team lb mode, I can successfully enslave ip6gre device
to a team device and make its mac addr look like:
# ip li show dev team0
17: team0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1448 qdisc noqueue
state UP mode DEFAULT group default qlen 1000
link/gre6 00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00 brd
00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
But ifconfig seems not recognize gre6 family, so it is just a matter of
a few lines of C code to trigger the bug.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch net] net: check mac address length for dev_set_mac_address()
2017-07-20 18:27 ` [Patch net] net: check mac address length " Cong Wang
2017-07-20 23:39 ` Cong Wang
@ 2017-07-21 18:50 ` Cong Wang
1 sibling, 0 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-21 18:50 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: David Ahern, Jiri Pirko
On Thu, Jul 20, 2017 at 11:27 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index 82fd4c9c4a1b..3f41601d7b7c 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -262,6 +262,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
> return dev_set_mtu(dev, ifr->ifr_mtu);
>
> case SIOCSIFHWADDR:
> + if (dev->addr_len > sizeof(struct sockaddr))
> + return -EINVAL;
> return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
>
Thinking a bit more, I should keep this patch simpler for -net and -stable,
so only the above piece is necessary, the rest pieces can be put to
-net-next.
I will resend this, and the "team: use a larger struct for mac address".
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-21 18:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-20 18:27 [Patch net] rtnetlink: allocate more memory for dev_set_mac_address() Cong Wang
2017-07-20 18:27 ` [Patch net] net: check mac address length " Cong Wang
2017-07-20 23:39 ` Cong Wang
2017-07-21 18:50 ` Cong Wang
2017-07-20 22:23 ` [Patch net] rtnetlink: allocate more memory " David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.