Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 11/20] rtnetlink: Update inet6_dump_ifinfo for strict data checking
From: Christian Brauner @ 2018-10-05 17:49 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181005174827.iudgcjd3r2cc7xqu@brauner.io>

On Fri, Oct 05, 2018 at 07:48:27PM +0200, Christian Brauner wrote:
> On Thu, Oct 04, 2018 at 02:33:46PM -0700, David Ahern wrote:
> > From: David Ahern <dsahern@gmail.com>
> > 
> > Update inet6_dump_ifinfo for strict data checking. If the flag is
> > set, the dump request is expected to have an ifinfomsg struct as
> > the header. All elements of the struct are expected to be 0 and no
> > attributes can be appended.
> > 
> > Signed-off-by: David Ahern <dsahern@gmail.com>
> 
> This is on top of current net-next? Are your patches ensuring that
> ipv6 addr requests don't generate log messages anymore when a wrong
> header is passed but the strict socket option is not passed? The context
> here doesn't seem to indicate that. :)

Nm, 9/10 takes care of that. Thanks! :)

> 
> > ---
> >  net/ipv6/addrconf.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index f749a3ad721a..693199a29426 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -5628,6 +5628,31 @@ static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev,
> >  	return -EMSGSIZE;
> >  }
> >  
> > +static int inet6_valid_dump_ifinfo(const struct nlmsghdr *nlh,
> > +				   struct netlink_ext_ack *extack)
> > +{
> > +	struct ifinfomsg *ifm;
> > +
> > +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
> > +		NL_SET_ERR_MSG(extack, "Invalid header");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (nlh->nlmsg_len > nlmsg_msg_size(sizeof(*ifm))) {
> > +		NL_SET_ERR_MSG(extack, "Invalid data after header");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ifm = nlmsg_data(nlh);
> > +	if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
> > +	    ifm->ifi_change || ifm->ifi_index) {
> > +		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
> >  {
> >  	struct net *net = sock_net(skb->sk);
> > @@ -5637,6 +5662,16 @@ static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
> >  	struct inet6_dev *idev;
> >  	struct hlist_head *head;
> >  
> > +	/* only requests using strict checking can pass data to
> > +	 * influence the dump
> > +	 */
> > +	if (cb->strict_check) {
> > +		int err = inet6_valid_dump_ifinfo(cb->nlh, cb->extack);
> > +
> > +		if (err)
> > +			return err;
> > +	}
> > +
> >  	s_h = cb->args[0];
> >  	s_idx = cb->args[1];
> >  
> > -- 
> > 2.11.0
> > 

^ permalink raw reply

* Re: [PATCH net-next 07/20] net/ipv6: Update inet6_dump_addr for strict data checking
From: Christian Brauner @ 2018-10-05 17:53 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-8-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:42PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update inet6_dump_addr for strict data checking. If the flag is set, the
> dump request is expected to have an ifaddrmsg struct as the header
> potentially followed by one or more attributes. Any data passed in the
> header or as an attribute is taken as a request to influence the data
> returned. Only values suppored by the dump handler are allowed to be
> non-0 or set in the request. At the moment only the IFA_TARGET_NETNSID
> attribute is supported. Follow on patches can add support for other fields
> (e.g., honor ifa_index and only return data for the given device index).
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Acked-by: Christian Brauner <christian@brauner.io>

> ---
>  net/ipv6/addrconf.c | 51 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index afa279170ba5..f749a3ad721a 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5001,6 +5001,8 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
>  static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>  			   enum addr_type_t type)
>  {
> +	struct netlink_ext_ack *extack = cb->extack;
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct inet6_fill_args fillargs = {
>  		.portid = NETLINK_CB(cb->skb).portid,
>  		.seq = cb->nlh->nlmsg_seq,
> @@ -5009,7 +5011,6 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>  		.type = type,
>  	};
>  	struct net *net = sock_net(skb->sk);
> -	struct nlattr *tb[IFA_MAX+1];
>  	struct net *tgt_net = net;
>  	int h, s_h;
>  	int idx, ip_idx;
> @@ -5022,15 +5023,47 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>  	s_idx = idx = cb->args[1];
>  	s_ip_idx = ip_idx = cb->args[2];
>  
> -	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> -			ifa_ipv6_policy, cb->extack) >= 0) {
> -		if (tb[IFA_TARGET_NETNSID]) {
> -			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
> +	if (cb->strict_check) {
> +		struct nlattr *tb[IFA_MAX+1];
> +		struct ifaddrmsg *ifm;
> +		int err, i;
> +
> +		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
> +		}
> +
> +		ifm = nlmsg_data(nlh);
> +		if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
> +			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
> +			return -EINVAL;
> +		}
> +		if (ifm->ifa_index) {
> +			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
> +			return -EINVAL;
> +		}
>  
> -			tgt_net = rtnl_get_net_ns_capable(skb->sk,
> -							  fillargs.netnsid);
> -			if (IS_ERR(tgt_net))
> -				return PTR_ERR(tgt_net);
> +		err = nlmsg_parse(nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> +				  ifa_ipv6_policy, extack);
> +		if (err < 0)
> +			return err;
> +
> +		for (i = 0; i <= IFA_MAX; ++i) {
> +			if (!tb[i])
> +				continue;
> +
> +			if (i == IFA_TARGET_NETNSID) {
> +				fillargs.netnsid = nla_get_s32(tb[i]);
> +				tgt_net = rtnl_get_net_ns_capable(skb->sk,
> +								  fillargs.netnsid);
> +				if (IS_ERR(tgt_net)) {
> +					NL_SET_ERR_MSG(extack, "Invalid target namespace id");
> +					return PTR_ERR(tgt_net);
> +				}
> +			} else {
> +				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
> +				return -EINVAL;
> +			}
>  		}
>  	}
>  
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH net-next 11/20] rtnetlink: Update inet6_dump_ifinfo for strict data checking
From: Christian Brauner @ 2018-10-05 17:54 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-12-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:46PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update inet6_dump_ifinfo for strict data checking. If the flag is
> set, the dump request is expected to have an ifinfomsg struct as
> the header. All elements of the struct are expected to be 0 and no
> attributes can be appended.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv6/addrconf.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index f749a3ad721a..693199a29426 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5628,6 +5628,31 @@ static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev,
>  	return -EMSGSIZE;
>  }
>  
> +static int inet6_valid_dump_ifinfo(const struct nlmsghdr *nlh,
> +				   struct netlink_ext_ack *extack)
> +{
> +	struct ifinfomsg *ifm;
> +
> +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
> +		NL_SET_ERR_MSG(extack, "Invalid header");
> +		return -EINVAL;
> +	}
> +
> +	if (nlh->nlmsg_len > nlmsg_msg_size(sizeof(*ifm))) {

Shouldn't ipv6 specific dump requests at least support IFA_TARGET_NETNSID?

> +		NL_SET_ERR_MSG(extack, "Invalid data after header");
> +		return -EINVAL;
> +	}
> +
> +	ifm = nlmsg_data(nlh);
> +	if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
> +	    ifm->ifi_change || ifm->ifi_index) {
> +		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  {
>  	struct net *net = sock_net(skb->sk);
> @@ -5637,6 +5662,16 @@ static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  	struct inet6_dev *idev;
>  	struct hlist_head *head;
>  
> +	/* only requests using strict checking can pass data to
> +	 * influence the dump
> +	 */
> +	if (cb->strict_check) {
> +		int err = inet6_valid_dump_ifinfo(cb->nlh, cb->extack);
> +
> +		if (err)
> +			return err;
> +	}
> +
>  	s_h = cb->args[0];
>  	s_idx = cb->args[1];
>  
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: pull-request: bpf 2018-10-05
From: David Miller @ 2018-10-05 17:56 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev
In-Reply-To: <20181005174713.4737-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri,  5 Oct 2018 19:47:13 +0200

> The following pull-request contains BPF updates for your *net* tree.
> 
> The main changes are:
> 
> 1) Fix to truncate input on ALU operations in 32 bit mode, from Jann.
> 
> 2) Fixes for cgroup local storage to reject reserved flags on element
>    update and rejection of map allocation with zero-sized value, from Roman.
> 
> Please consider pulling these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git

Pulled, thanks Daniel.

^ permalink raw reply

* Re: [PATCH net-next 08/20] rtnetlink: Update rtnl_dump_ifinfo for strict data checking
From: Christian Brauner @ 2018-10-05 17:59 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-9-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:43PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update rtnl_dump_ifinfo for strict data checking. If the flag is set,
> the dump request is expected to have an ifinfomsg struct as the header
> potentially followed by one or more attributes. Any data passed in the
> header or as an attribute is taken as a request to influence the data
> returned. Only values supported by the dump handler are allowed to be
> non-0 or set in the request. At the moment only the IFA_TARGET_NETNSID,
> IFLA_EXT_MASK, IFLA_MASTER, and IFLA_LINKINFO attributes are supported.
> 
> Existing code does not fail the dump if nlmsg_parse fails. That behavior
> is kept for non-strict checking.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/core/rtnetlink.c | 97 +++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 69 insertions(+), 28 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index c3b434d724ea..4fd27b5db787 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1880,6 +1880,8 @@ EXPORT_SYMBOL_GPL(rtnl_get_net_ns_capable);
>  
>  static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> +	struct netlink_ext_ack *extack = cb->extack;
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct net *net = sock_net(skb->sk);
>  	struct net *tgt_net = net;
>  	int h, s_h;
> @@ -1892,44 +1894,84 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  	unsigned int flags = NLM_F_MULTI;
>  	int master_idx = 0;
>  	int netnsid = -1;
> -	int err;
> +	int err, i;
>  	int hdrlen;
>  
>  	s_h = cb->args[0];
>  	s_idx = cb->args[1];
>  
> -	/* A hack to preserve kernel<->userspace interface.
> -	 * The correct header is ifinfomsg. It is consistent with rtnl_getlink.
> -	 * However, before Linux v3.9 the code here assumed rtgenmsg and that's
> -	 * what iproute2 < v3.9.0 used.
> -	 * We can detect the old iproute2. Even including the IFLA_EXT_MASK
> -	 * attribute, its netlink message is shorter than struct ifinfomsg.
> -	 */
> -	hdrlen = nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg) ?
> -		 sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
> +	if (cb->strict_check) {
> +		struct ifinfomsg *ifm;
>  
> -	if (nlmsg_parse(cb->nlh, hdrlen, tb, IFLA_MAX,
> -			ifla_policy, cb->extack) >= 0) {
> -		if (tb[IFLA_TARGET_NETNSID]) {
> -			netnsid = nla_get_s32(tb[IFLA_TARGET_NETNSID]);
> -			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
> -			if (IS_ERR(tgt_net))
> -				return PTR_ERR(tgt_net);
> +		hdrlen = sizeof(*ifm);
> +		if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
>  		}
>  
> -		if (tb[IFLA_EXT_MASK])
> -			ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
> -
> -		if (tb[IFLA_MASTER])
> -			master_idx = nla_get_u32(tb[IFLA_MASTER]);
> +		ifm = nlmsg_data(nlh);
> +		if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
> +		    ifm->ifi_change) {
> +			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
> +			return -EINVAL;
> +		}
> +		if (ifm->ifi_index) {
> +			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
> +			return -EINVAL;
> +		}
> +	} else {
> +		/* A hack to preserve kernel<->userspace interface.
> +		 * The correct header is ifinfomsg. It is consistent with
> +		 * rtnl_getlink. However, before Linux v3.9 the code here
> +		 * assumed rtgenmsg and that's what iproute2 < v3.9.0 used.
> +		 * We can detect the old iproute2. Even including the
> +		 * IFLA_EXT_MASK attribute, its netlink message is shorter
> +		 * than struct ifinfomsg.
> +		 */
> +		hdrlen = nlmsg_len(nlh) < sizeof(struct ifinfomsg) ?
> +			 sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
> +	}
>  
> -		if (tb[IFLA_LINKINFO])
> -			kind_ops = linkinfo_to_kind_ops(tb[IFLA_LINKINFO]);
> +	err = nlmsg_parse(nlh, hdrlen, tb, IFLA_MAX, ifla_policy, extack);
> +	if (err < 0) {
> +		if (cb->strict_check)
> +			return -EINVAL;
> +		goto walk_entries;
> +	}
>  
> -		if (master_idx || kind_ops)
> -			flags |= NLM_F_DUMP_FILTERED;
> +	for (i = 0; i <= IFLA_MAX; ++i) {
> +		if (!tb[i])
> +			continue;
> +		switch (i) {
> +		case IFLA_TARGET_NETNSID:
> +			netnsid = nla_get_s32(tb[i]);
> +			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
> +			if (IS_ERR(tgt_net)) {
> +				NL_SET_ERR_MSG(extack, "Invalid target namespace id");
> +				return PTR_ERR(tgt_net);
> +			}
> +			break;
> +		case IFLA_EXT_MASK:
> +			ext_filter_mask = nla_get_u32(tb[i]);
> +			break;
> +		case IFLA_MASTER:
> +			master_idx = nla_get_u32(tb[i]);
> +			break;
> +		case IFLA_LINKINFO:
> +			kind_ops = linkinfo_to_kind_ops(tb[i]);
> +			break;
> +		default:
> +			if (cb->strict_check) {
> +				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
> +				return -EINVAL;
> +			}
> +		}

This might make sense to be split into two helpers for parsing:
<blablabla>_strict() and <blablabla>_lenient(). :)


>  	}
>  
> +	if (master_idx || kind_ops)
> +		flags |= NLM_F_DUMP_FILTERED;
> +
> +walk_entries:
>  	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
>  		idx = 0;
>  		head = &tgt_net->dev_index_head[h];
> @@ -1941,8 +1983,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  			err = rtnl_fill_ifinfo(skb, dev, net,
>  					       RTM_NEWLINK,
>  					       NETLINK_CB(cb->skb).portid,
> -					       cb->nlh->nlmsg_seq, 0,
> -					       flags,
> +					       nlh->nlmsg_seq, 0, flags,
>  					       ext_filter_mask, 0, NULL, 0,
>  					       netnsid);
>  
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH net-next 06/20] net/ipv4: Update inet_dump_ifaddr for strict data checking
From: Christian Brauner @ 2018-10-05 18:02 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-7-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:41PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update inet_dump_ifaddr for strict data checking. If the flag is set,
> the dump request is expected to have an ifaddrmsg struct as the header
> potentially followed by one or more attributes. Any data passed in the
> header or as an attribute is taken as a request to influence the data
> returned. Only values supported by the dump handler are allowed to be
> non-0 or set in the request. At the moment only the IFA_TARGET_NETNSID
> attribute is supported. Follow on patches can support for other fields
> (e.g., honor ifa_index and only return data for the given device index).
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv4/devinet.c | 53 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index ab2b11df5ea4..af968d4fe4fc 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1662,15 +1662,16 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
>  
>  static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> +	struct netlink_ext_ack *extack = cb->extack;
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct inet_fill_args fillargs = {
>  		.portid = NETLINK_CB(cb->skb).portid,
> -		.seq = cb->nlh->nlmsg_seq,
> +		.seq = nlh->nlmsg_seq,
>  		.event = RTM_NEWADDR,
>  		.flags = NLM_F_MULTI,
>  		.netnsid = -1,
>  	};
>  	struct net *net = sock_net(skb->sk);
> -	struct nlattr *tb[IFA_MAX+1];
>  	struct net *tgt_net = net;
>  	int h, s_h;
>  	int idx, s_idx;
> @@ -1684,15 +1685,47 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
>  	s_idx = idx = cb->args[1];
>  	s_ip_idx = ip_idx = cb->args[2];
>  
> -	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> -			ifa_ipv4_policy, cb->extack) >= 0) {
> -		if (tb[IFA_TARGET_NETNSID]) {
> -			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
> +	if (cb->strict_check) {
> +		struct nlattr *tb[IFA_MAX+1];
> +		struct ifaddrmsg *ifm;
> +		int err, i;
> +
> +		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
> +		}
> +
> +		ifm = nlmsg_data(nlh);
> +		if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
> +			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
> +			return -EINVAL;
> +		}
> +		if (ifm->ifa_index) {
> +			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
> +			return -EINVAL;
> +		}
> +
> +		err = nlmsg_parse(nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> +				  ifa_ipv4_policy, extack);
> +		if (err < 0)
> +			return err;
>  
> -			tgt_net = rtnl_get_net_ns_capable(skb->sk,
> -							  fillargs.netnsid);
> -			if (IS_ERR(tgt_net))
> -				return PTR_ERR(tgt_net);
> +		for (i = 0; i <= IFA_MAX; ++i) {
> +			if (!tb[i])
> +				continue;
> +			if (i == IFA_TARGET_NETNSID) {

Nit: For the sake of readability there could be an additional newline between the 
"continue" and the next if () condition.

> +				fillargs.netnsid = nla_get_s32(tb[i]);
> +
> +				tgt_net = rtnl_get_net_ns_capable(skb->sk,
> +								  fillargs.netnsid);
> +				if (IS_ERR(tgt_net)) {
> +					NL_SET_ERR_MSG(extack, "Invalid target namespace id");

Nit: Hm, maybe "Invalid target network namespace id" would be better.
You never know what namespace comes along some time later. :)

> +					return PTR_ERR(tgt_net);
> +				}
> +			} else {
> +				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
> +				return -EINVAL;
> +			}
>  		}
>  	}
>  
> -- 
> 2.11.0
> 

^ permalink raw reply

* [PATCH ipsec-next] Clear secpath on loopback_xmit
From: Benedict Wong @ 2018-10-05 18:23 UTC (permalink / raw)
  To: netdev; +Cc: nharold, benedictwong, lorenzo

This patch clears the skb->sp when transmitted over loopback. This
ensures that the loopback-ed packet does not have any secpath
information from the outbound transforms.

At present, this causes XFRM tunnel mode packets to be dropped with
XFRMINNOPOLS, due to the outbound state being in the secpath, without
a matching inbound policy. Clearing the secpath ensures that all states
added to the secpath are exclusively from the inbound processing.

Tests: xfrm tunnel mode tests added for loopback:
    https://android-review.googlesource.com/c/kernel/tests/+/777328
Signed-off-by: Benedict Wong <benedictwong@google.com>
---
 drivers/net/loopback.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 30612497643c..a6bf54df94bd 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -50,6 +50,7 @@
 #include <linux/ethtool.h>
 #include <net/sock.h>
 #include <net/checksum.h>
+#include <net/xfrm.h>
 #include <linux/if_ether.h>	/* For the statistics structure. */
 #include <linux/if_arp.h>	/* For ARPHRD_ETHER */
 #include <linux/ip.h>
@@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 	 */
 	skb_dst_force(skb);
 
+	// Clear secpath to ensure xfrm policy check not tainted by outbound SAs.
+	secpath_reset(skb);
+
 	skb->protocol = eth_type_trans(skb, dev);
 
 	/* it's OK to use per_cpu_ptr() because BHs are off */
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* Re: [PATCH bpf-next 0/6] Error handling when map lookup isn't supported
From: Alexei Starovoitov @ 2018-10-05 18:35 UTC (permalink / raw)
  To: Prashant Bhole
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
	David S . Miller, Quentin Monnet, netdev
In-Reply-To: <20181005033601.6776-1-bhole_prashant_q7@lab.ntt.co.jp>

On Fri, Oct 05, 2018 at 12:35:55PM +0900, Prashant Bhole wrote:
> Currently when map a lookup fails, user space API can not make any
> distinction whether given key was not found or lookup is not supported
> by particular map.
> 
> In this series we modify return value of maps which do not support
> lookup. Lookup on such map implementation will return -EOPNOTSUPP.
> bpf() syscall with BPF_MAP_LOOKUP_ELEM command will set EOPNOTSUPP
> errno. We also handle this error in bpftool to print appropriate
> message.
> 
> Patch 1: adds handling of BPF_MAP_LOOKUP ELEM command of bpf syscall
> such that errno will set to EOPNOTSUPP when map doesn't support lookup
> 
> Patch 2: Modifies the return value of map_lookup_elem() to EOPNOTSUPP
> for maps which do not support lookup
> 
> Patch 3: Splits do_dump() in bpftool/map.c. Element printing code is
> moved out into new function dump_map_elem(). This was done in order to
> reduce deep indentation and accomodate further changes.
> 
> Patch 4: Changes in bpftool to print strerror() message when lookup
> error is occured. This will result in appropriate message like
> "Operation not supported" when map doesn't support lookup.
> 
> Patch 5: test_verifier: change fixup map naming convention as
> suggested by Alexei
> 
> Patch 6: Added verifier tests to check whether verifier rejects call 
> to bpf_map_lookup_elem from bpf program. For all map types those
> do not support map lookup.

for the set:
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [RFC PATCH bpf-next v4 4/7] bpf: add bpf queue and stack maps
From: Alexei Starovoitov @ 2018-10-05 18:41 UTC (permalink / raw)
  To: Mauricio Vasquez; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <86a2498d-4e2e-7fb9-5139-9738e3796b01@polito.it>

On Thu, Oct 04, 2018 at 10:40:55PM -0500, Mauricio Vasquez wrote:
> 
> > > +	/* Round up queue size to nearest power of 2 */
> > > +	max_entries = index_mask + 1;
> > what's the point of roundup ?
> 
> If the size of the buffer is power of two we can wrap the indexes with an
> AND operation instead of MOD.
> 
> > The memory waste becomes quite large when max_entries are high.
> Yes, you are right, we have the different choices described below.
> 
> > 
> > If queue/stack is sized to exact max_entries,
> > then 'count' can be removed too, right?
> 
> If we don't use 'count' and we want to use the AND operation for wrapping
> indexes, the max entries should be 2^ - 1  because a slot is lost to
> distinguish between full/empty queue/stack.
> 
> Just to summarize, we have these options:
> 1. Allow any size, round up, use the AND operation and 'count' (current).
> 2. Allow only power of 2 sizes, use the AND operation and 'count'.
> 3. Allow any size, no roundup, use the MOD operation and leaving an empty
> slot.
> 
> I prefer 1 or 2, but I don't have a strong opinion, maybe allowing only
> power of two max entries could be too limiting.
> Another consideration: is this really too bad to waste memory when user
> requires a size far away of the next power of 2?

I think there is 4th option. Neither AND nor MOD is necessary.
Pls take a look at ptr_ring implementation.

^ permalink raw reply

* Re: [PATCH net-next 03/20] net: Add extack to nlmsg_parse
From: David Ahern @ 2018-10-05 18:42 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005173936.miim5us6wdcomqxl@brauner.io>

On 10/5/18 11:39 AM, Christian Brauner wrote:
> On Thu, Oct 04, 2018 at 02:33:38PM -0700, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> Make sure extack is passed to nlmsg_parse where easy to do so.
>> Most of these are dump handlers and leveraging the extack in
>> the netlink_callback.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
> 
> Yeah, having extack in dump requests sounds really useful to me!
> 

it's been invaluable updating iproute2.

^ permalink raw reply

* Re: [PATCH net-next 05/20] netlink: Add new socket option to enable strict checking on dumps
From: David Ahern @ 2018-10-05 18:43 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005173628.ewdch6aoaz6vd4qj@brauner.io>

On 10/5/18 11:36 AM, Christian Brauner wrote:
>> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
>> index 88c8a2d83eb3..36bdca2aa42d 100644
>> --- a/include/linux/netlink.h
>> +++ b/include/linux/netlink.h
>> @@ -179,6 +179,8 @@ struct netlink_callback {
>>  	struct netlink_ext_ack	*extack;
>>  	u16			family;
>>  	u16			min_dump_alloc;
>> +	unsigned int		strict_check:1,
>> +				unused:31;
> 
> I like this idea a lot. :) but I'm not a fan of bitfields if not
> necessary. Is that really necessary here?
> 

no strong opinions on a bitfield vs a bool.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
From: Alexei Starovoitov @ 2018-10-05 18:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa,
	acme
In-Reply-To: <20181005061409.GA15872@krava>

On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> > > 
> > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:  
> > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > > [...]
> > > > >>
> > > > >> If the purpose of the patch is to give user space visibility into
> > > > >> bpf prog load/unload as a notification, then I completely agree that
> > > > >> some notification mechanism is necessary.  
> > > > 
> > > > Yeah, I did only regard it as only that, nothing more. Some means
> > > > of timeline and notification that can be kept in a record in user
> > > > space and later retrieved e.g. for introspection on what has been
> > > > loaded.
> > > > 
> > > > >> I've started working on such mechanism via perf ring buffer which is
> > > > >> the fastest mechanism we have in the kernel so far.
> > > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  
> 
> cool, could you please CC me if there's another version
> of that patchset?

will do.

> > > > 
> > > > That one is definitely needed in any case to resolve the kallsyms
> > > > limitations, and it does have overlap in that in either case we
> > > > want to look at past BPF programs that have been unloaded in the
> > > > meantime, so I don't have a strong preference either way, and the
> > > > former is needed in any case. Though thought was that audit might
> > > > be an option for those not running profiling daemons 24/7, but
> > > > presumably bpftool could be extended to record these events as
> > > > well if we don't want to reuse audit infra.
> > > 
> > > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > > these events.  I do acknowledge that this perf event is relevant,
> > > especially for catching the kernel symbols (I need that myself), but it
> > > does not cover my use-case.
> > > 
> > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > timeline of these notifications, for later retrieval.  The idea is that
> > > our support engineers can look at these records when troubleshooting
> > > the system.  And the plan is also to collect these records as part of
> > > our sosreport tool, which is part of the support case.
> > 
> > I don't think you're implying that prog load/unload should be spamming dmesg
> > and auditd not even running...
> 
> I think the problem Jesper implied is that in order to collect
> those logs you'll need perf tool running all the time.. which
> it's not equipped for yet

I'm not proposing to run 'perf' binary all the time.
Setting up perf ring buffer just for these new bpf prog load/unload events
and epolling it is simple enough to do from any application including auditd.
selftests/bpf/ do it for bpf output events.

^ permalink raw reply

* Re: [PATCH net-next 05/20] netlink: Add new socket option to enable strict checking on dumps
From: Christian Brauner @ 2018-10-05 18:45 UTC (permalink / raw)
  To: David Ahern, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <336199f4-1afc-f23e-0ccb-183dc0572290@gmail.com>

On October 5, 2018 8:43:55 PM GMT+02:00, David Ahern <dsahern@gmail.com> wrote:
>On 10/5/18 11:36 AM, Christian Brauner wrote:
>>> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
>>> index 88c8a2d83eb3..36bdca2aa42d 100644
>>> --- a/include/linux/netlink.h
>>> +++ b/include/linux/netlink.h
>>> @@ -179,6 +179,8 @@ struct netlink_callback {
>>>  	struct netlink_ext_ack	*extack;
>>>  	u16			family;
>>>  	u16			min_dump_alloc;
>>> +	unsigned int		strict_check:1,
>>> +				unused:31;
>> 
>> I like this idea a lot. :) but I'm not a fan of bitfields if not
>> necessary. Is that really necessary here?
>> 
>
>no strong opinions on a bitfield vs a bool.

Just feels like this is something that is
rarely used. Having a bool or traditional 
flag might be more readable and easier to 
maintain. :)

^ permalink raw reply

* Re: [PATCH net-next 06/20] net/ipv4: Update inet_dump_ifaddr for strict data checking
From: David Ahern @ 2018-10-05 18:48 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005180232.5v5as3yhx4gio6oy@brauner.io>

On 10/5/18 12:02 PM, Christian Brauner wrote:
>> +
>> +		err = nlmsg_parse(nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
>> +				  ifa_ipv4_policy, extack);
>> +		if (err < 0)
>> +			return err;
>>  
>> -			tgt_net = rtnl_get_net_ns_capable(skb->sk,
>> -							  fillargs.netnsid);
>> -			if (IS_ERR(tgt_net))
>> -				return PTR_ERR(tgt_net);
>> +		for (i = 0; i <= IFA_MAX; ++i) {
>> +			if (!tb[i])
>> +				continue;
>> +			if (i == IFA_TARGET_NETNSID) {
> 
> Nit: For the sake of readability there could be an additional newline between the 
> "continue" and the next if () condition.
> 
>> +				fillargs.netnsid = nla_get_s32(tb[i]);
>> +
>> +				tgt_net = rtnl_get_net_ns_capable(skb->sk,
>> +								  fillargs.netnsid);
>> +				if (IS_ERR(tgt_net)) {
>> +					NL_SET_ERR_MSG(extack, "Invalid target namespace id");
> 
> Nit: Hm, maybe "Invalid target network namespace id" would be better.
> You never know what namespace comes along some time later. :)
> 

done.

^ permalink raw reply

* Re: [PATCH net-next] udp: gro behind static key
From: David Miller @ 2018-10-05 18:52 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, pabeni, tom, steffen.klassert, willemb
In-Reply-To: <20181005153140.205743-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Fri,  5 Oct 2018 11:31:40 -0400

> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> udp tunnel callback configured.
> 
> udp_sk(sk)->gro_receive requires a registration with
> setup_udp_tunnel_sock, which enables the static key.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] ipv6: do not leave garbage in rt->fib6_metrics
From: David Miller @ 2018-10-05 18:57 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet, dsahern
In-Reply-To: <20181005161750.179275-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Fri,  5 Oct 2018 09:17:50 -0700

> In case ip_fib_metrics_init() returns an error, we better
> rewrite rt->fib6_metrics with &dst_default_metrics so that
> we do not crash later in ip_fib_metrics_put()
> 
> Fixes: 767a2217533f ("net: common metrics init helper for FIB entries")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>

Applied to net-next, thanks Eric.

Subject should have said net-next instead of net btw.

^ permalink raw reply

* Re: [PATCH net-next] fib_tests: Add tests for invalid metric on route
From: David Miller @ 2018-10-05 18:59 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, eric.dumazet, dsahern
In-Reply-To: <20181005170119.10441-1-dsahern@kernel.org>

From: David Ahern <dsahern@kernel.org>
Date: Fri,  5 Oct 2018 10:01:19 -0700

> From: David Ahern <dsahern@gmail.com>
> 
> Add ipv4 and ipv6 test cases with an invalid metrics option causing
> ip_metrics_convert to fail. Tests clean up path during route add.
> 
> Also, add nodad to to ipv6 address add. When running ipv6_route_metrics
> directly seeing an occasional failure on the "Using route with mtu metric"
> test case.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> This test catches the error Eric reported and verifies the fix.

Applied, thanks David.

^ permalink raw reply

* Re: [PATCH net] ipv6: do not leave garbage in rt->fib6_metrics
From: Eric Dumazet @ 2018-10-05 19:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, David Ahern
In-Reply-To: <20181005.115732.771251816122036025.davem@davemloft.net>

On Fri, Oct 5, 2018 at 11:57 AM David Miller <davem@davemloft.net> wrote:

> Applied to net-next, thanks Eric.
>
> Subject should have said net-next instead of net btw.

Oops, sorry for that, I used the wrong script.

Thanks.

^ permalink raw reply

* [PATCH net-next 2/2] net/ncsi: Add NCSI Mellanox OEM command
From: Vijay Khemka @ 2018-10-05 19:02 UTC (permalink / raw)
  To: Samuel Mendoza-Jonas, David S. Miller, netdev, linux-kernel
  Cc: vijaykhemka, openbmc @ lists . ozlabs . org,
	Justin . Lee1 @ Dell . com, joel @ jms . id . au,
	linux-aspeed @ lists . ozlabs . org, Sai Dasari,
	christian @ cmd . nu

This patch adds OEM Mellanox commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.

ncsi_oem_gma_handler_mlx: This handler send NCSI mellanox command for
getting mac address.
ncsi_rsp_handler_oem_mlx: This handles response received for all
mellanox OEM commands.
ncsi_rsp_handler_oem_mlx_gma: This handles get mac address response and
set it to device.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 net/ncsi/internal.h    |  5 +++++
 net/ncsi/ncsi-manage.c | 24 +++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  9 +++++++++
 net/ncsi/ncsi-rsp.c    | 41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 45883b32790e..d4558628a991 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -73,10 +73,15 @@ enum {
 #define NCSI_OEM_MFR_BCM_ID             0x113d
 /* Broadcom specific OEM Command */
 #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
+/* Mellanox specific OEM Command */
+#define NCSI_OEM_MLX_CMD_GMA            0x00   /* CMD ID for Get MAC */
+#define NCSI_OEM_MLX_CMD_GMA_PARAM      0x1b   /* Parameter for GMA  */
 /* OEM Command payload lengths*/
 #define NCSI_OEM_BCM_CMD_GMA_LEN        12
+#define NCSI_OEM_MLX_CMD_GMA_LEN        8
 /* Mac address offset in OEM response */
 #define BCM_MAC_ADDR_OFFSET             28
+#define MLX_MAC_ADDR_OFFSET             8
 
 
 struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index e5bfd9245b5d..38aef27d2e67 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -658,12 +658,34 @@ static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
 			   nca->type);
 }
 
+static void ncsi_oem_gma_handler_mlx(struct ncsi_cmd_arg *nca)
+{
+	int ret = 0;
+	unsigned char data[NCSI_OEM_MLX_CMD_GMA_LEN];
+
+	nca->payload = NCSI_OEM_MLX_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_MLX_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl(NCSI_OEM_MFR_MLX_ID);
+	data[5] = NCSI_OEM_MLX_CMD_GMA;
+	data[6] = NCSI_OEM_MLX_CMD_GMA_PARAM;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+}
+
 /* OEM Command handlers initialization */
 static struct ncsi_oem_gma_handler {
 	unsigned int	mfr_id;
 	void		(*handler)(struct ncsi_cmd_arg *nca);
 } ncsi_oem_gma_handlers[] = {
-	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
 };
 
 #endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 4d3f06be38bd..2a6d83a596c9 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,15 @@ struct ncsi_rsp_oem_pkt {
 	unsigned char           data[];      /* Payload data      */
 };
 
+/* Mellanox Response Data */
+struct ncsi_rsp_oem_mlx_pkt {
+	unsigned char           cmd_rev;     /* Command Revision  */
+	unsigned char           cmd;         /* Command ID        */
+	unsigned char           param;       /* Parameter         */
+	unsigned char           optional;    /* Optional data     */
+	unsigned char           data[];      /* Data              */
+};
+
 /* Broadcom Response Data */
 struct ncsi_rsp_oem_bcm_pkt {
 	unsigned char           ver;         /* Payload Version   */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index bc20f7036579..a6f5a59eb6ef 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,45 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Mellanox command Get Mac Address */
+static int ncsi_rsp_handler_oem_mlx_gma(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	int ret = 0;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct sockaddr saddr;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[MLX_MAC_ADDR_OFFSET], ETH_ALEN);
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+
+	return ret;
+}
+
+/* Response handler for Mellanox card */
+static int ncsi_rsp_handler_oem_mlx(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_rsp_oem_mlx_pkt *mlx;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	mlx = (struct ncsi_rsp_oem_mlx_pkt *)(rsp->data);
+
+	if (mlx->cmd == NCSI_OEM_MLX_CMD_GMA &&
+	    mlx->param == NCSI_OEM_MLX_CMD_GMA_PARAM)
+		return ncsi_rsp_handler_oem_mlx_gma(nr);
+	return 0;
+}
+
 /* Response handler for Broadcom command Get Mac Address */
 static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
 {
@@ -640,7 +679,7 @@ static struct ncsi_rsp_oem_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_request *nr);
 } ncsi_rsp_oem_handlers[] = {
-	{ NCSI_OEM_MFR_MLX_ID, NULL },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_rsp_handler_oem_mlx },
 	{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
 };
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net-next 08/20] rtnetlink: Update rtnl_dump_ifinfo for strict data checking
From: David Ahern @ 2018-10-05 19:22 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005175922.4oiwihofhsvcu4ok@brauner.io>

On 10/5/18 11:59 AM, Christian Brauner wrote:
>> +	err = nlmsg_parse(nlh, hdrlen, tb, IFLA_MAX, ifla_policy, extack);
>> +	if (err < 0) {
>> +		if (cb->strict_check)
>> +			return -EINVAL;
>> +		goto walk_entries;
>> +	}
>>  
>> -		if (master_idx || kind_ops)
>> -			flags |= NLM_F_DUMP_FILTERED;
>> +	for (i = 0; i <= IFLA_MAX; ++i) {
>> +		if (!tb[i])
>> +			continue;
>> +		switch (i) {
>> +		case IFLA_TARGET_NETNSID:
>> +			netnsid = nla_get_s32(tb[i]);
>> +			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
>> +			if (IS_ERR(tgt_net)) {
>> +				NL_SET_ERR_MSG(extack, "Invalid target namespace id");
>> +				return PTR_ERR(tgt_net);
>> +			}
>> +			break;
>> +		case IFLA_EXT_MASK:
>> +			ext_filter_mask = nla_get_u32(tb[i]);
>> +			break;
>> +		case IFLA_MASTER:
>> +			master_idx = nla_get_u32(tb[i]);
>> +			break;
>> +		case IFLA_LINKINFO:
>> +			kind_ops = linkinfo_to_kind_ops(tb[i]);
>> +			break;
>> +		default:
>> +			if (cb->strict_check) {
>> +				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
>> +				return -EINVAL;
>> +			}
>> +		}
> 
> This might make sense to be split into two helpers for parsing:
> <blablabla>_strict() and <blablabla>_lenient(). :)

I thought about that, but there is so much overlap - they are mostly
common. Besides, ifinfomsg is the header for link dumps, and ifinfomsg
is the one that has been (ab)used for other message types, so strict
versus lenient does not really have a differentiator for this message
type - other than checking the elements of the struct.

^ permalink raw reply

* [Report] net: wireless: iwlegacy: Several possible data races
From: Jia-Ju Bai @ 2018-10-06  2:25 UTC (permalink / raw)
  To: sgruszka, kvalo, davem; +Cc: linux-wireless, netdev, Linux Kernel Mailing List

****** Possible race 0 ******
CPU0:
il4965_mac_tx
     il4965_tx_skb
         line 1687: spin_lock_irqsave(&il->lock, flags)
         line 1894: q->write_ptr = il_queue_inc_wrap(...) [WRITE]

CPU1:
il4965_irq_tasklet
     il_txq_update_write_ptr
         line 2749: il_wr(..., txq->q.write_ptr); [READ]
         line 2757: _il_wr(..., txq->q.write_ptr); [READ]

CPU2:
il_bg_watchdog
     il_check_stuck_queue
         line 4817: if (q->read_ptr == q->write_ptr) [READ]

CPU3:
il4965_irq_tasklet
     il4965_rx_handle
         line 4827, function pointer: il->handlers[pkt->hdr.cmd] (il, rxb);
         il4965_hdl_tx
             il_queue_used
                 line 868: return q->write_ptr >= ... q->write_ptr ... 
[READ]

As for q->write_ptr, the WRITE operation in CPU0 is performed with holding
a spinlock il->lock, but the READ operations in CPU1, CPU2 and CPU3
are performed without holding this spinlock, so there may exist data races.

****** Possible race 1 ******
CPU0:
il4965_irq_tasklet
     il4965_rx_handle
         line 4827, function pointer: il->handlers[pkt->hdr.cmd] (il, rxb);
         il4965_hdl_tx
             line 2834: spin_lock_irqsave(&il->sta_lock, flags)
             il4965_tx_queue_reclaim
                 line 2490: q->read_ptr = ... [WRITE]

CPU1:
il_bg_watchdog
     il_check_stuck_queue
         line 4817: q->read_ptr == q->write_ptr [READ]

As for q->read_ptr, the WRITE operation in CPU0 is performed with
holding a spinlock il->sta_lock, but the READ operation in CPU1 is
performed without holding this spinlock, so there may exist a data race.

****** Possible race 2 ******
CPU0:
il4965_mac_tx
     il4965_tx_skb
         line 1687: spin_lock_irqsave(&il->lock, flags)
         line 1863: txq->need_update = 1 [WRITE]

CPU1:
il4965_irq_tasklet
     il_txq_update_write_ptr
         line 2731: if (txq->need_update == 0) [READ]

As for txq->need_update, the WRITE operation in CPU0 is performed with
holding a spinlock il->lock, but the READ operation in CPU1 is performed
without holding this spinlock, so there may exist a data race.

I do not find good ways to fix these possible data races, so I only 
report them.


Best wishes,
Jia-Ju Bai

^ permalink raw reply

* Re: [PATCH net-next 11/20] rtnetlink: Update inet6_dump_ifinfo for strict data checking
From: David Ahern @ 2018-10-05 19:25 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005174827.iudgcjd3r2cc7xqu@brauner.io>

On 10/5/18 11:48 AM, Christian Brauner wrote:
> On Thu, Oct 04, 2018 at 02:33:46PM -0700, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> Update inet6_dump_ifinfo for strict data checking. If the flag is
>> set, the dump request is expected to have an ifinfomsg struct as
>> the header. All elements of the struct are expected to be 0 and no
>> attributes can be appended.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
> This is on top of current net-next? Are your patches ensuring that
> ipv6 addr requests don't generate log messages anymore when a wrong
> header is passed but the strict socket option is not passed? The context
> here doesn't seem to indicate that. :)
> 

this is an AF_INET6 GETLINK handler. Why? no idea, but I think you are
confusing this patch with the GETADDR patch which generated the
"netlink: 16 bytes leftover after parsing attributes in process `ip'."
message before this set.

^ permalink raw reply

* Re: [PATCH net-next 11/20] rtnetlink: Update inet6_dump_ifinfo for strict data checking
From: David Ahern @ 2018-10-05 19:26 UTC (permalink / raw)
  To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181005175448.jqjefd3fpve3y5ty@brauner.io>

On 10/5/18 11:54 AM, Christian Brauner wrote:
>> +static int inet6_valid_dump_ifinfo(const struct nlmsghdr *nlh,
>> +				   struct netlink_ext_ack *extack)
>> +{
>> +	struct ifinfomsg *ifm;
>> +
>> +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
>> +		NL_SET_ERR_MSG(extack, "Invalid header");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (nlh->nlmsg_len > nlmsg_msg_size(sizeof(*ifm))) {
> 
> Shouldn't ipv6 specific dump requests at least support IFA_TARGET_NETNSID?

It does not today. The AF_UNSPEC GETLINK dumps it but the AF_INET6 does
not.

Some one can add it later if desired.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
From: Arnaldo Carvalho de Melo @ 2018-10-05 19:42 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev,
	Jiri Olsa
In-Reply-To: <20181005184434.uphwrbqkfx2isbx4@ast-mbp.dhcp.thefacebook.com>

Em Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov escreveu:
> On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > > timeline of these notifications, for later retrieval.  The idea is that
> > > > our support engineers can look at these records when troubleshooting
> > > > the system.  And the plan is also to collect these records as part of
> > > > our sosreport tool, which is part of the support case.

> > > I don't think you're implying that prog load/unload should be spamming dmesg
> > > and auditd not even running...

> > I think the problem Jesper implied is that in order to collect
> > those logs you'll need perf tool running all the time.. which
> > it's not equipped for yet

> I'm not proposing to run 'perf' binary all the time.

I think Jiri just said that one would have to run something all the time
to get all the records, see below

> Setting up perf ring buffer just for these new bpf prog load/unload events
> and epolling it is simple enough to do from any application including auditd.
> selftests/bpf/ do it for bpf output events.

I think he is talking about the preexisting loaded BPF programs. We have
the same problem with mmaps, where the perf tool will, with races,
enumerate the existing mmaps as PERF_RECORD_MMAP synthesized from
/proc/PIDS/smaps.

There was talk in the past to ask the kernel to emit PERF_RECORD_MMAP
into the ring buffer for those pre-existing entries, reducing a bit the
races, but as there doesn't seem to have a good way of doing it, we
continued with the synthesizing from procfs.

Is there a way for us to synthesize those prog load/unload for
preexisting loaded bpf objects?

- Arnaldo

^ permalink raw reply

* [PATCH net-next v7 00/28] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-10-06  2:56 UTC (permalink / raw)
  To: linux-kernel, netdev, davem, gregkh; +Cc: Jason A. Donenfeld

Changes v6->v7, along with who suggested it.
--------------------------------------------
  - Account for big-endian 2^26 conversion in Poly1305.
  - Account for big-endian NEON in Curve25519.
  - Fix macros in big-endian AArch64 code so that this will actually run
    there at all.
  - [Ard Biesheuvel] Prefer if (IS_ENABLED(...)) over ifdef mazes when
    possible.
  - [Ard Biesheuvel] Call simd_relax() within any preempt-disabling glue
    code every once in a while so as not to increase latency if folks
    pass in super long buffers.
  - [Andy Polyakov] Prefer compiler-defined architecture macros in
    assembly code, which puts us in closer alignment with upstream
    CRYPTOGAMS code, and is cleaner.
  - [Andrew Lunn] Non-static symbols are prefixed with wg_ to avoid
    polluting the global namespace.
  - [Ard Biesheuvel] Return a bool from simd_relax() indicating whether
    or not we were rescheduled.
  - [Ard Biesheuvel] Reflect the proper simd conditions on arm.
  - [Ard Biesheuvel] Do not reorder lines in Kbuild files for the simd
    asm-generic addition, since we don't want to cause merge conflicts.
  - [Ard Biesheuvel] WARN() if the selftests fail in Zinc, since if this
    is an initcall, it won't block module loading, so we want to be loud.
  - [Ard Biesheuvel] Document some interdependencies beside include
    statements.
  - [Ard Biesheuvel] Remove HAVE_*_ARCH_IMPLEMENTATION in intermediate
    commits.
  - Add missing static statement to fpu init functions.
  - Use union in chacha to access state words as a flat matrix, instead
    of casting a struct to a u8 and hoping all goes well. Then, by passing
    around that array as a struct for as long as possible, we can update
    counter[0] instead of state[12] in the generic blocks, which makes it
    clearer what's happening.
  - [Ard Biesheuvel] Remove __aligned(32) for chacha20_ctx since we no
    longer use vmovdqa on x86, and the other implementations do not require
    that kind of alignment either.
  - Submit patch to ARM tree for adjusting RiscPC's cflags to be -march=armv3
    so that we can build code that uses umull.
  - [Ard Biesheuvel] Allow CONFIG_ARM[64] to imply [!]CONFIG_64BIT, and use
    zinc arch config variables consistently throughout.
  - [Eric Biggers] Document rationale for the 2^26->2^64/32 conversion in
    code comments.
  - [Andrew Lunn] Convert all of remaining BUG_ON to WARN_ON.
  - [Eric Biggers] Include the Z3 proof of 2^26 conversion correctness in
    commit message for curosity of others (https://xn--4db.cc/ltPtHCKN/py).
  - [Ard Biesheuvel] Show importing of Andy Polyakov's code always in a
    separate commit for all architectures.
  - [Herbert Xu] Make more clear what "old" and "new" columns refer to in
    benchmarks.
  - [Ard Biesheuvel] Replace `bxeq lr` with `reteq lr` in ARM assembler to be
    compatible with old ISAs via the macro in <asm/assembler.h>.
  - [Eric Biggers] Account for multiple threads accessing the same tfm in the
    port of the current crypto API to Zinc.
  - [Ard Biesheuvel] Do not allow WireGuard to be a built-in if IPv6 is a
    module.
  - [Ard Biesheuvel] As neat as it is to have `default m`, nothing else in
    drivers/net has that, so we remove it, alas.
  - [Ard Biesheuvel] Writeback the base register and reorder multiplications
    in the NEON x25519 implementation.
  - Try all combinations of different implementations in selftests, so that
    potential bugs are more immediately unearthed.
  - The selftest infrastructure now generally prefers IS_ENABLED(..) over
    ifdefs, since the compiler is more than capable of trimming unused
    functions and static variables.
  - [Sultan Alsawaf] Self tests and SIMD glue code work with #include,
    which lets the compiler optimize these. Previously these files were .h,
    because they were included, but a simple grep of the kernel tree shows
    259 other files that carry out this same pattern. Only they prefer to
    instead name the files with a .c instead of a .h, so we now follow the
    convention.

-----------------------------------------------------------

This patchset is available on git.kernel.org in this branch, where it may be
pulled directly for inclusion into net-next:

  * https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/log/?h=jd/wireguard

-----------------------------------------------------------

WireGuard is a secure network tunnel written especially for Linux, which
has faced around three years of serious development, deployment, and
scrutiny. It delivers excellent performance and is extremely easy to
use and configure. It has been designed with the primary goal of being
both easy to audit by virtue of being small and highly secure from a
cryptography and systems security perspective. WireGuard is used by some
massive companies pushing enormous amounts of traffic, and likely
already today you've consumed bytes that at some point transited through
a WireGuard tunnel. Even as an out-of-tree module, WireGuard has been
integrated into various userspace tools, Linux distributions, mobile
phones, and data centers. There are ports in several languages to
several operating systems, and even commercial hardware and services
sold integrating WireGuard. It is time, therefore, for WireGuard to be
properly integrated into Linux.

Ample information, including documentation, installation instructions,
and project details, is available at:

  * https://www.wireguard.com/
  * https://www.wireguard.com/papers/wireguard.pdf

As it is currently an out-of-tree module, it lives in its own git repo
and has its own mailing list, and every commit for the module is tested
against every stable kernel since 3.10 on a variety of architectures
using an extensive test suite:

  * https://git.zx2c4.com/WireGuard
    https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/WireGuard.git/
  * https://lists.zx2c4.com/mailman/listinfo/wireguard
  * https://www.wireguard.com/build-status/

The project has been broadly discussed at conferences, and was presented
to the Netdev developers in Seoul last November, where a paper was
released detailing some interesting aspects of the project. Dave asked
me after the talk if I would consider sending in a v1 "sooner rather
than later", hence this patchset. Zinc was presented at Kernel Recipes
in September, and a video is available online. Both Zinc and WireGuard
will be presented at the conference in Vancouver in November.

  * https://www.wireguard.com/presentations/
  * https://www.wireguard.com/papers/wireguard-netdev22.pdf
  * Zinc talk: https://www.youtube.com/watch?v=bFhdln8aJ_U
  * Netdev talk: https://www.youtube.com/watch?v=54orFwtQ1XY

The cryptography in the protocol itself has been formally verified by
several independent academic teams with positive results, and I know of
two additional efforts on their way to further corroborate those
findings. The version 1 protocol is "complete", and so the purpose of
this review is to assess the implementation of the protocol. However, it
still may be of interest to know that the thing you're reviewing uses a
protocol with various nice security properties:

  * https://www.wireguard.com/formal-verification/

This patchset is divided into four segments. The first introduces a very
simple helper for working with the FPU state for the purposes of amortizing
SIMD operations. The second segment is a small collection of cryptographic
primitives, split up into several commits by primitive and by hardware. The
third shows usage of Zinc within the existing crypto API and as a replacement
to the existing crypto API. The last is WireGuard itself, presented as an
unintrusive and self-contained virtual network driver.

It is intended that this entire patch series enter the kernel through
DaveM's net-next tree. Subsequently, WireGuard patches will go through
DaveM's net-next tree, while Zinc patches will go through Greg KH's tree in
cases when an entire development cycle has no relationships with existing code
in crypto/; however, if there are any relationships with code in crypto/, then
pull requests will be sent to Herbert instead in case there are merge
conflicts.

Enjoy,
Jason

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox