| Message ID | 1439845607-2824-1-git-send-email-phil@nwl.cc |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Phil Sutter <phil@nwl.cc> Date: Mon, 17 Aug 2015 23:06:47 +0200 > Upon evaluation of IFLA_ADDRESS and IFLA_BROADCAST messages, make sure > the passed argument length matches dev->addr_len exactly. > > This fixes dubious behaviour of 'ip link set eth0 addr <MAC>' where > '<MAC>' is too long, e.g. '00:11:22:33:44:55:66:77'. Called like this, > 'ip' would return successfully and the kernel sets eth0's MAC address to > the leading six octets of the passed argument. > > Signed-off-by: Phil Sutter <phil@nwl.cc> I don't think this behavior is very "dubious" and making the check more strict risks breaking things that have worked for a very long time. I'm not applying this, sorry. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 17, 2015 at 02:09:33PM -0700, David Miller wrote: > From: Phil Sutter <phil@nwl.cc> > Date: Mon, 17 Aug 2015 23:06:47 +0200 > > > Upon evaluation of IFLA_ADDRESS and IFLA_BROADCAST messages, make sure > > the passed argument length matches dev->addr_len exactly. > > > > This fixes dubious behaviour of 'ip link set eth0 addr <MAC>' where > > '<MAC>' is too long, e.g. '00:11:22:33:44:55:66:77'. Called like this, > > 'ip' would return successfully and the kernel sets eth0's MAC address to > > the leading six octets of the passed argument. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > I don't think this behavior is very "dubious" and making the check more > strict risks breaking things that have worked for a very long time. > > I'm not applying this, sorry. No problem. I was already afraid of this breaking something as commit 1840bb1, which introduces validate_linkmsg(), already mentions something in that direction. Therefore I appreciate your feedback clarifying this for me, although it is not quite the outcome I had wished for. Seeing that the kernel checks the minimum length already, shifting the maximum length check to kernel space felt like a natural choice. Nevertheless, I think iproute2 should not behave this way. Stephen, do you think it is reasonable to add a similar logic to iplink_parse() as done in do_set() (i.e., finding out which is the correct length of LL address for a given interface and validating input based on this)? If so, is it appropriate to recycle get_address() function or should the information come from netlink? Thanks, Phil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 788ceed..1d61cd1 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1456,11 +1456,11 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[]) { if (dev) { if (tb[IFLA_ADDRESS] && - nla_len(tb[IFLA_ADDRESS]) < dev->addr_len) + nla_len(tb[IFLA_ADDRESS]) != dev->addr_len) return -EINVAL; if (tb[IFLA_BROADCAST] && - nla_len(tb[IFLA_BROADCAST]) < dev->addr_len) + nla_len(tb[IFLA_BROADCAST]) != dev->addr_len) return -EINVAL; }
Upon evaluation of IFLA_ADDRESS and IFLA_BROADCAST messages, make sure the passed argument length matches dev->addr_len exactly. This fixes dubious behaviour of 'ip link set eth0 addr <MAC>' where '<MAC>' is too long, e.g. '00:11:22:33:44:55:66:77'. Called like this, 'ip' would return successfully and the kernel sets eth0's MAC address to the leading six octets of the passed argument. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/core/rtnetlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)