netdev - Re: [PATCH] net:l2tp: Allow MAC to be configured via netlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20180118.154455.870472017446178168.davem@davemloft.net>
Date:   Thu, 18 Jan 2018 15:44:55 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     isaac.lee@...iedtelesis.co.nz
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] net:l2tp: Allow MAC to be configured via netlink

From: Isaac Lee <isaac.lee@...iedtelesis.co.nz>
Date: Wed, 17 Jan 2018 14:35:37 +1300

> The linux kernel by default uses random MAC address
> for l2tp interfaces. However, there are situations
> where it is desirable to have a deterministic MAC address.
> 
> A sample scenario would be where the host IP stack is attached
> directly to a tunnel hence the "random" address is now propagated
> via ARP/ND to the other end of the tunnel. If the device reboots,
> a new MAC is used and the communication over the tunnel will be
> disrupted until the new MAC address is re-learned by the peer.
> Therefore it can be useful to have the mac address the same across
> reboots.
> 
> The patch makes the MAC address to be configurable via
> netlink so that a userspace program can specify what MAC address to
> use at interface creation time in the kernel.
> 
> Signed-off-by: Isaac Lee <isaac.lee@...iedtelesis.co.nz>
 ...
> diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
> index a1f24fb2be98..dc2933c32121 100644
> --- a/net/l2tp/l2tp_netlink.c
> +++ b/net/l2tp/l2tp_netlink.c
> @@ -607,6 +607,9 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
>  	if (info->attrs[L2TP_ATTR_MRU])
>  		cfg.mru = nla_get_u16(info->attrs[L2TP_ATTR_MRU]);
>  
> +	if (info->attrs[L2TP_ATTR_HWADDR])
> +		memcpy(&cfg.hwaddr, nla_data(info->attrs[L2TP_ATTR_HWADDR]), ETH_ALEN);
> +

I think you should validate the MAC address here, rather then in
l2tp_eth_create().  This way if the address is invalid you can fail
the operation and signal an error back to the user.

Also, dev->dev_addr can never be NULL in the context of
l2tp_eth_dev_init().  As you can see, the existing call to
eth_hw_addr_random() writes there unconditionally.

Thank you.