Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 06/19] net: usb: aqc111: Introduce link management
From: Andrew Lunn @ 2018-10-05 17:11 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: David S . Miller, linux-usb@vger.kernel.org,
	netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <cec5cd88988e0985e3fdd1343906ef649b01f646.1538734658.git.igor.russkikh@aquantia.com>

On Fri, Oct 05, 2018 at 10:24:55AM +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> 
> Add full hardware initialization sequence and link configuration logic

Hi Igor, Dmitry

Please could you explain why you decided to not use drivers/net/phy?
The previous patch introduced basically what you need to export a
standard Linux MDIO bus. From that you can use a standard Linux PHY
driver.

	Andrew

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
From: Alexei Starovoitov @ 2018-10-06  0:22 UTC (permalink / raw)
  To: Al Viro
  Cc: Andy Lutomirski, Alexei Starovoitov, David S. Miller,
	Daniel Borkmann, Network Development, LKML, kernel-team
In-Reply-To: <20181005234706.GX32577@ZenIV.linux.org.uk>

On Sat, Oct 06, 2018 at 12:47:06AM +0100, Al Viro wrote:
> 
> And no, it's not that each of those filesystems does not use inode->i_ino at all.
> There's a bunch of library helpers in fs/*.c that happen to use the value filesystem
> has seen fit to store there.  Whether to use those helpers or not, what to store in
> that field, etc. is, again, entirely up to the filesystem in question.
> generic_fillattr() is one of those, that's all there is to it.

makes sense.

> That's precisely why I really do not like the idea of hooks poking in the internals
> of kernel data structures.  Especially since not even "it's not visible outside of
> a subsystem-internal header" appears to slow you down.

that's why there is a code review to get the patches in shape that
don't expose kernel internals by _accident_.

> The same goes for tracepoints, etc. - turning random details of implementation into
> a carved in stone ABI is actively harmful.

we're not talking about tracepoints here.

> PS: If anything, visibility to hooks should be opt-in.

not sure how do you expect 'opt-in' to be imlemented.
As far as exposing inode and dev I agree that was a wrong approach.
Going to switch to struct file_handle instead.
Doing statx for every file_open is probably too slow for practical use.

> Sure, we can start actively hiding
> the things, but that's a winless arms race  - even if we bloody went and encrypted the
> private stuff, you'd still be able to pull decryption key from where it would be accessed
> by legitimate users, etc.  Нахуй нам это надо?

why do this ?
the use cases are explained in commit log.

Back to internals exposure.
Do you see an issue in the following:
struct bpf_file_info {
        __u32 fs_magic; // file->f_inode->i_sb->s_magic
        __u32 mnt_id;   // real_mount(file->f_path.mnt)->mnt_id
        __u32 nlink;    // file->f_inode->i_nlink
        __u32 mode;     // file->f_inode->i_mode
        __u32 flags;    // file->f_flags
};
and separate helper that returns struct file_handle ?
and ctx rewriting moved to fs/bpf_file_filter.c ?

^ permalink raw reply

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

On Thu, Oct 04, 2018 at 02:33:55PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update br_mdb_dump for strict data checking. If the flag is set,
> the dump request is expected to have a br_port_msg 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/bridge/br_mdb.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index a4a848bf827b..7beeee658d6c 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -162,6 +162,28 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
>  	return err;
>  }
>  
> +static int br_mdb_valid_dump_req(const struct nlmsghdr *nlh,
> +				 struct netlink_ext_ack *extack)
> +{
> +	struct br_port_msg *bpm;
> +
> +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*bpm))) {
> +		NL_SET_ERR_MSG(extack, "Invalid header");
> +		return -EINVAL;
> +	}
> +	if (bpm->ifindex) {
> +		NL_SET_ERR_MSG(extack,
> +			       "Filtering by device index is not supported");
> +		return -EINVAL;
> +	}
> +	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*bpm))) {
> +		NL_SET_ERR_MSG(extack, "Invalid data after header");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  {
>  	struct net_device *dev;
> @@ -169,6 +191,13 @@ static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  	struct nlmsghdr *nlh = NULL;
>  	int idx = 0, s_idx;
>  
> +	if (cb->strict_check) {
> +		int err = br_mdb_valid_dump_req(cb->nlh, cb->extack);
> +
> +		if (err)
> +			return err;

Nit: unnecessarry newline :)

> +	}
> +
>  	s_idx = cb->args[0];
>  
>  	rcu_read_lock();
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH v3] net/ncsi: Add NCSI OEM command support
From: Vijay Khemka @ 2018-10-05 17:32 UTC (permalink / raw)
  To: Samuel Mendoza-Jonas, Justin . Lee1 @ Dell . com,
	joel @ jms . id . au, linux-aspeed @ lists . ozlabs . org,
	openbmc @ lists . ozlabs . org, Sai Dasari,
	netdev @ vger . kernel . org, christian @ cmd . nu
In-Reply-To: <88dc95b120e89acbfaa0e8442822c1b3f5696a57.camel@mendozajonas.com>



On 10/4/18, 9:48 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:

    On Wed, 2018-10-03 at 16:32 -0700, Vijay Khemka wrote:
    > This patch adds OEM commands and response handling. It also defines OEM
    > command and response structure as per NCSI specification along with its
    > handlers.
    > 
    > ncsi_cmd_handler_oem: This is a generic command request handler for OEM
    > commands
    > ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
    > 
    > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    
    Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
    
    Technically this patch should also be marked [PATCH net-next] to target
    David's next tree.
  
  Thanks Sam, I will mark it and send it again to larger audience.
    
  


^ permalink raw reply

* Re: [PATCH net-next] cxgb4: use FW_PORT_ACTION_L1_CFG32 for 32 bit capability
From: David Miller @ 2018-10-05 17:35 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, indranil, dt, leedom
In-Reply-To: <1538732085-13643-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri,  5 Oct 2018 15:04:45 +0530

> when 32 bit port capability is in use, use FW_PORT_ACTION_L1_CFG32
> rather than FW_PORT_ACTION_L1_CFG.
> 
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>

Applied.

^ 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 17:36 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-6-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:40PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Add a new socket option, NETLINK_DUMP_STRICT_CHK, that userspace
> can use via setsockopt to request strict checking of headers and
> attributes on dump requests.
> 
> To get dump features such as kernel side filtering based on data in
> the header or attributes appended to the dump request, userspace
> must call setsockopt() for NETLINK_DUMP_STRICT_CHK and a non-zero
> value. Since the netlink sock and its flags are private to the
> af_netlink code, the strict checking flag is passed to dump handlers
> via a flag in the netlink_callback struct.
> 
> For old userspace on new kernel there is no impact as all of the data
> checks in later patches are wrapped in a check on the new strict flag.
> 
> For new userspace on old kernel, the setsockopt will fail and even if
> new userspace sets data in the headers and appended attributes the
> kernel will silently ignore it.
> 
> New userspace on new kernel setting the socket option gets the benefit
> of the improved data dump.
> 
> Kernel side the NETLINK_DUMP_STRICT_CHK uapi is converted to a generic
> NETLINK_F_STRICT_CHK flag which can potentially be leveraged for tighter
> checking on the NEW, DEL, and SET commands.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  include/linux/netlink.h      |  2 ++
>  include/uapi/linux/netlink.h |  1 +
>  net/netlink/af_netlink.c     | 21 ++++++++++++++++++++-
>  net/netlink/af_netlink.h     |  1 +
>  4 files changed, 24 insertions(+), 1 deletion(-)
> 
> 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?

>  	unsigned int		prev_seq, seq;
>  	long			args[6];
>  };
> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> index 776bc92e9118..486ed1f0c0bc 100644
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -155,6 +155,7 @@ enum nlmsgerr_attrs {
>  #define NETLINK_LIST_MEMBERSHIPS	9
>  #define NETLINK_CAP_ACK			10
>  #define NETLINK_EXT_ACK			11
> +#define NETLINK_DUMP_STRICT_CHK		12
>  
>  struct nl_pktinfo {
>  	__u32	group;
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 7ac585f33a9e..e613a9f89600 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1706,6 +1706,13 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
>  			nlk->flags &= ~NETLINK_F_EXT_ACK;
>  		err = 0;
>  		break;
> +	case NETLINK_DUMP_STRICT_CHK:
> +		if (val)
> +			nlk->flags |= NETLINK_F_STRICT_CHK;
> +		else
> +			nlk->flags &= ~NETLINK_F_STRICT_CHK;
> +		err = 0;
> +		break;
>  	default:
>  		err = -ENOPROTOOPT;
>  	}
> @@ -1799,6 +1806,15 @@ static int netlink_getsockopt(struct socket *sock, int level, int optname,
>  			return -EFAULT;
>  		err = 0;
>  		break;
> +	case NETLINK_DUMP_STRICT_CHK:
> +		if (len < sizeof(int))
> +			return -EINVAL;
> +		len = sizeof(int);
> +		val = nlk->flags & NETLINK_F_STRICT_CHK ? 1 : 0;
> +		if (put_user(len, optlen) || put_user(val, optval))
> +			return -EFAULT;
> +		err = 0;
> +		break;
>  	default:
>  		err = -ENOPROTOOPT;
>  	}
> @@ -2282,9 +2298,9 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
>  			 const struct nlmsghdr *nlh,
>  			 struct netlink_dump_control *control)
>  {
> +	struct netlink_sock *nlk, *nlk2;
>  	struct netlink_callback *cb;
>  	struct sock *sk;
> -	struct netlink_sock *nlk;
>  	int ret;
>  
>  	refcount_inc(&skb->users);
> @@ -2318,6 +2334,9 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
>  	cb->min_dump_alloc = control->min_dump_alloc;
>  	cb->skb = skb;
>  
> +	nlk2 = nlk_sk(NETLINK_CB(skb).sk);
> +	cb->strict_check = !!(nlk2->flags & NETLINK_F_STRICT_CHK);
> +
>  	if (control->start) {
>  		ret = control->start(cb);
>  		if (ret)
> diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
> index 962de7b3c023..5f454c8de6a4 100644
> --- a/net/netlink/af_netlink.h
> +++ b/net/netlink/af_netlink.h
> @@ -15,6 +15,7 @@
>  #define NETLINK_F_LISTEN_ALL_NSID	0x10
>  #define NETLINK_F_CAP_ACK		0x20
>  #define NETLINK_F_EXT_ACK		0x40
> +#define NETLINK_F_STRICT_CHK		0x80
>  
>  #define NLGRPSZ(x)	(ALIGN(x, sizeof(unsigned long) * 8) / 8)
>  #define NLGRPLONGS(x)	(NLGRPSZ(x)/sizeof(unsigned long))
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH net-next 03/20] net: Add extack to nlmsg_parse
From: Christian Brauner @ 2018-10-05 17:39 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-4-dsahern@kernel.org>

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!

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

> ---
>  net/core/devlink.c             | 2 +-
>  net/core/neighbour.c           | 3 ++-
>  net/core/rtnetlink.c           | 4 ++--
>  net/ipv4/devinet.c             | 9 +++++----
>  net/ipv6/addrconf.c            | 2 +-
>  net/ipv6/route.c               | 2 +-
>  net/mpls/af_mpls.c             | 2 +-
>  net/netfilter/ipvs/ip_vs_ctl.c | 2 +-
>  net/sched/act_api.c            | 2 +-
>  net/sched/cls_api.c            | 6 ++++--
>  net/sched/sch_api.c            | 3 ++-
>  net/xfrm/xfrm_user.c           | 2 +-
>  12 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index de6adad7ccbe..b207ba1188e2 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -3489,7 +3489,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
>  	start_offset = *((u64 *)&cb->args[0]);
>  
>  	err = nlmsg_parse(cb->nlh, GENL_HDRLEN + devlink_nl_family.hdrsize,
> -			  attrs, DEVLINK_ATTR_MAX, ops->policy, NULL);
> +			  attrs, DEVLINK_ATTR_MAX, ops->policy, cb->extack);
>  	if (err)
>  		goto out;
>  
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index fb023df48b83..b06f794bf91e 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2445,7 +2445,8 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
>  	    ((struct ndmsg *)nlmsg_data(nlh))->ndm_flags == NTF_PROXY)
>  		proxy = 1;
>  
> -	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
> +	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL,
> +			  cb->extack);
>  	if (!err) {
>  		if (tb[NDA_IFINDEX]) {
>  			if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 57bf96d73e3b..c3b434d724ea 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1909,7 +1909,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  		 sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
>  
>  	if (nlmsg_parse(cb->nlh, hdrlen, tb, IFLA_MAX,
> -			ifla_policy, NULL) >= 0) {
> +			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);
> @@ -3764,7 +3764,7 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  	int fidx = 0;
>  
>  	err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb,
> -			  IFLA_MAX, ifla_policy, NULL);
> +			  IFLA_MAX, ifla_policy, cb->extack);
>  	if (err < 0) {
>  		return -EINVAL;
>  	} else if (err == 0) {
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 44d931a3cd50..ab2b11df5ea4 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -782,7 +782,8 @@ static void set_ifa_lifetime(struct in_ifaddr *ifa, __u32 valid_lft,
>  }
>  
>  static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
> -				       __u32 *pvalid_lft, __u32 *pprefered_lft)
> +				       __u32 *pvalid_lft, __u32 *pprefered_lft,
> +				       struct netlink_ext_ack *extack)
>  {
>  	struct nlattr *tb[IFA_MAX+1];
>  	struct in_ifaddr *ifa;
> @@ -792,7 +793,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
>  	int err;
>  
>  	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv4_policy,
> -			  NULL);
> +			  extack);
>  	if (err < 0)
>  		goto errout;
>  
> @@ -897,7 +898,7 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
>  
>  	ASSERT_RTNL();
>  
> -	ifa = rtm_to_ifaddr(net, nlh, &valid_lft, &prefered_lft);
> +	ifa = rtm_to_ifaddr(net, nlh, &valid_lft, &prefered_lft, extack);
>  	if (IS_ERR(ifa))
>  		return PTR_ERR(ifa);
>  
> @@ -1684,7 +1685,7 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
>  	s_ip_idx = ip_idx = cb->args[2];
>  
>  	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> -			ifa_ipv4_policy, NULL) >= 0) {
> +			ifa_ipv4_policy, cb->extack) >= 0) {
>  		if (tb[IFA_TARGET_NETNSID]) {
>  			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
>  
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index a9a317322388..2f8aa4fd5e55 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5021,7 +5021,7 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>  	s_ip_idx = ip_idx = cb->args[2];
>  
>  	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> -			ifa_ipv6_policy, NULL) >= 0) {
> +			ifa_ipv6_policy, cb->extack) >= 0) {
>  		if (tb[IFA_TARGET_NETNSID]) {
>  			netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
>  
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 3adf107b42d2..64ae1e383030 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4137,7 +4137,7 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	int err;
>  
>  	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv6_policy,
> -			  NULL);
> +			  extack);
>  	if (err < 0)
>  		goto errout;
>  
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index 8fbe6cdbe255..55a30ee3d820 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -1223,7 +1223,7 @@ static int mpls_netconf_get_devconf(struct sk_buff *in_skb,
>  	int err;
>  
>  	err = nlmsg_parse(nlh, sizeof(*ncm), tb, NETCONFA_MAX,
> -			  devconf_mpls_policy, NULL);
> +			  devconf_mpls_policy, extack);
>  	if (err < 0)
>  		goto errout;
>  
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 62eefea48973..83395bf6dc35 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3234,7 +3234,7 @@ static int ip_vs_genl_dump_dests(struct sk_buff *skb,
>  
>  	/* Try to find the service for which to dump destinations */
>  	if (nlmsg_parse(cb->nlh, GENL_HDRLEN, attrs, IPVS_CMD_ATTR_MAX,
> -			ip_vs_cmd_policy, NULL))
> +			ip_vs_cmd_policy, cb->extack))
>  		goto out_err;
>  
>  
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 3c7c23421885..5764e1af2ef9 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1452,7 +1452,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>  	u32 act_count = 0;
>  
>  	ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tb, TCA_ROOT_MAX,
> -			  tcaa_policy, NULL);
> +			  tcaa_policy, cb->extack);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index d670d3066ebd..43c8559aca56 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1727,7 +1727,8 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
>  	if (nlmsg_len(cb->nlh) < sizeof(*tcm))
>  		return skb->len;
>  
> -	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
> +	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL,
> +			  cb->extack);
>  	if (err)
>  		return err;
>  
> @@ -2054,7 +2055,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
>  	if (nlmsg_len(cb->nlh) < sizeof(*tcm))
>  		return skb->len;
>  
> -	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
> +	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL,
> +			  cb->extack);
>  	if (err)
>  		return err;
>  
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 22e9799e5b69..121454f15f0f 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1656,7 +1656,8 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  	idx = 0;
>  	ASSERT_RTNL();
>  
> -	err = nlmsg_parse(nlh, sizeof(struct tcmsg), tca, TCA_MAX, NULL, NULL);
> +	err = nlmsg_parse(nlh, sizeof(struct tcmsg), tca, TCA_MAX, NULL,
> +			  cb->extack);
>  	if (err < 0)
>  		return err;
>  
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index df7ca2dabc48..ca7a207b81a9 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1007,7 +1007,7 @@ static int xfrm_dump_sa(struct sk_buff *skb, struct netlink_callback *cb)
>  		int err;
>  
>  		err = nlmsg_parse(cb->nlh, 0, attrs, XFRMA_MAX, xfrma_policy,
> -				  NULL);
> +				  cb->extack);
>  		if (err < 0)
>  			return err;
>  
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH net-next 02/19] net: usb: aqc111: Add bind and empty unbind callbacks
From: David Miller @ 2018-10-05 17:39 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, Dmitry.Bezrukov
In-Reply-To: <3f15ced11f712a8e99ac5461dcaec22265503241.1538734658.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Fri, 5 Oct 2018 10:24:42 +0000

> +static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> +	int ret;
> +	struct usb_device *udev = interface_to_usbdev(intf);

Please always order local variable declarations from longest to shortest
line (reverse christmas tree).

^ permalink raw reply

* Re: [PATCH net-next 03/19] net: usb: aqc111: Add implementation of read and write commands
From: David Miller @ 2018-10-05 17:40 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, Dmitry.Bezrukov
In-Reply-To: <ebf0867900ae849581fbd20b52ee9e855e6345c8.1538734658.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Fri, 5 Oct 2018 10:24:44 +0000

> +static int __aqc111_read_cmd(struct usbnet *dev, u8 cmd, u16 value,
> +			     u16 index, u16 size, void *data, int nopm)
> +{
> +	int ret;
> +	int (*fn)(struct usbnet *dev, u8 cmd, u8 reqtype, u16 value,
> +		  u16 index, void *data, u16 size);

Again, please order local variables from longest to shortest line.

I won't explicitly point out the others, you need to audit your entire
submission for this problem and fix it up.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next 01/20] netlink: Pass extack to dump handlers
From: Christian Brauner @ 2018-10-05 17:41 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-2-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:36PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Declare extack in netlink_dump and pass to dump handlers via
> netlink_callback. Add any extack message after the dump_done_errno
> allowing error messages to be returned. This will be useful when
> strict checking is done on dump requests, returning why the dump
> fails EINVAL.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

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

> ---
>  include/linux/netlink.h  |  1 +
>  net/netlink/af_netlink.c | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index 71f121b66ca8..88c8a2d83eb3 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -176,6 +176,7 @@ struct netlink_callback {
>  	void			*data;
>  	/* the module that dump function belong to */
>  	struct module		*module;
> +	struct netlink_ext_ack	*extack;
>  	u16			family;
>  	u16			min_dump_alloc;
>  	unsigned int		prev_seq, seq;
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index e3a0538ec0be..7ac585f33a9e 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2171,6 +2171,7 @@ EXPORT_SYMBOL(__nlmsg_put);
>  static int netlink_dump(struct sock *sk)
>  {
>  	struct netlink_sock *nlk = nlk_sk(sk);
> +	struct netlink_ext_ack extack = {};
>  	struct netlink_callback *cb;
>  	struct sk_buff *skb = NULL;
>  	struct nlmsghdr *nlh;
> @@ -2222,8 +2223,11 @@ static int netlink_dump(struct sock *sk)
>  	skb_reserve(skb, skb_tailroom(skb) - alloc_size);
>  	netlink_skb_set_owner_r(skb, sk);
>  
> -	if (nlk->dump_done_errno > 0)
> +	if (nlk->dump_done_errno > 0) {
> +		cb->extack = &extack;
>  		nlk->dump_done_errno = cb->dump(skb, cb);
> +		cb->extack = NULL;
> +	}
>  
>  	if (nlk->dump_done_errno > 0 ||
>  	    skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {
> @@ -2246,6 +2250,12 @@ static int netlink_dump(struct sock *sk)
>  	memcpy(nlmsg_data(nlh), &nlk->dump_done_errno,
>  	       sizeof(nlk->dump_done_errno));
>  
> +	if (extack._msg && nlk->flags & NETLINK_F_EXT_ACK) {
> +		nlh->nlmsg_flags |= NLM_F_ACK_TLVS;
> +		if (!nla_put_string(skb, NLMSGERR_ATTR_MSG, extack._msg))
> +			nlmsg_end(skb, nlh);
> +	}
> +
>  	if (sk_filter(sk, skb))
>  		kfree_skb(skb);
>  	else
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH net-next 02/20] netlink: Add extack message to nlmsg_parse for invalid header length
From: Christian Brauner @ 2018-10-05 17:41 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-3-dsahern@kernel.org>

On Thu, Oct 04, 2018 at 02:33:37PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Give a user a reason why EINVAL is returned in nlmsg_parse.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

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

> ---
>  include/net/netlink.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 589683091f16..9522a0bf1f3a 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -516,8 +516,10 @@ static inline int nlmsg_parse(const struct nlmsghdr *nlh, int hdrlen,
>  			      const struct nla_policy *policy,
>  			      struct netlink_ext_ack *extack)
>  {
> -	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen))
> +	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) {
> +		NL_SET_ERR_MSG(extack, "Invalid header length");
>  		return -EINVAL;
> +	}
>  
>  	return nla_parse(tb, maxtype, nlmsg_attrdata(nlh, hdrlen),
>  			 nlmsg_attrlen(nlh, hdrlen), policy, extack);
> -- 
> 2.11.0
> 

^ permalink raw reply

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

On Thu, Oct 04, 2018 at 02:33:51PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update rtnl_net_dumpid for strict data checking. If the flag is set,
> the dump request is expected to have an rtgenmsg struct as the header
> which has the family as the only element. No data may be appended.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

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

> ---
>  net/core/net_namespace.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 670c84b1bfc2..63659c512ba8 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -844,6 +844,7 @@ static int rtnl_net_dumpid_one(int id, void *peer, void *data)
>  
>  static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct net *net = sock_net(skb->sk);
>  	struct rtnl_net_dump_cb net_cb = {
>  		.net = net,
> @@ -853,6 +854,13 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
>  		.s_idx = cb->args[0],
>  	};
>  
> +	if (cb->strict_check) {
> +		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(struct rtgenmsg))) {
> +			NL_SET_ERR_MSG(cb->extack, "Unknown data in dump request");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	spin_lock_bh(&net->nsid_lock);
>  	idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
>  	spin_unlock_bh(&net->nsid_lock);
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH] bpf: 32-bit RSH verification must truncate input before the ALU op
From: Edward Cree @ 2018-10-05 17:45 UTC (permalink / raw)
  To: Jann Horn, Daniel Borkmann, Alexei Starovoitov, netdev
  Cc: David S. Miller, linux-kernel
In-Reply-To: <20181005161759.177992-1-jannh@google.com>

On 05/10/18 17:17, Jann Horn wrote:
> When I wrote commit 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification"), I
> assumed that, in order to emulate 64-bit arithmetic with 32-bit logic, it
> is sufficient to just truncate the output to 32 bits; and so I just moved
> the register size coercion that used to be at the start of the function to
> the end of the function.
>
> That assumption is true for almost every op, but not for 32-bit right
> shifts, because those can propagate information towards the least
> significant bit. Fix it by always truncating inputs for 32-bit ops to 32
> bits.
>
> Also get rid of the coerce_reg_to_size() after the ALU op, since that has
> no effect.
Might be worth saying something like "because src_reg is passed by value".
> Fixes: 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification")
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
Acked-by: Edward Cree <ecree@solarflare.com>
>  kernel/bpf/verifier.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index bb07e74b34a2..465952a8e465 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2896,6 +2896,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
>  	u64 umin_val, umax_val;
>  	u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
Incidentally, I don't see why this needs to be a u64 (rather than say a u8).

-Ed
>  
> +	if (insn_bitness == 32) {
> +		/* Relevant for 32-bit RSH: Information can propagate towards
> +		 * LSB, so it isn't sufficient to only truncate the output to
> +		 * 32 bits.
> +		 */
> +		coerce_reg_to_size(dst_reg, 4);
> +		coerce_reg_to_size(&src_reg, 4);
> +	}
> +
>  	smin_val = src_reg.smin_value;
>  	smax_val = src_reg.smax_value;
>  	umin_val = src_reg.umin_value;
> @@ -3131,7 +3140,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
>  	if (BPF_CLASS(insn->code) != BPF_ALU64) {
>  		/* 32-bit ALU ops are (32,32)->32 */
>  		coerce_reg_to_size(dst_reg, 4);
> -		coerce_reg_to_size(&src_reg, 4);
>  	}
>  
>  	__reg_deduce_bounds(dst_reg);

^ permalink raw reply

* Re: [PATCH net-next 06/19] net: usb: aqc111: Introduce link management
From: David Miller @ 2018-10-05 17:46 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, Dmitry.Bezrukov
In-Reply-To: <cec5cd88988e0985e3fdd1343906ef649b01f646.1538734658.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Fri, 5 Oct 2018 10:24:55 +0000

> +	switch (aqc111_data->link_speed) {
> +	case AQ_INT_SPEED_5G:
> +	{
> +		link_speed = 5000;
> +		reg8 = 0x05;
> +		reg16 = 0x001F;
> +		break;
> +	}

Please do not put curly braces around switch cases.

You aren't declaring local scope variables in these case statements so
the curly braces are completely unnecessary and look ugly.

^ permalink raw reply

* pull-request: bpf 2018-10-05
From: Daniel Borkmann @ 2018-10-05 17:47 UTC (permalink / raw)
  To: davem; +Cc: daniel, ast, netdev

Hi David,

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

Thanks a lot!

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

The following changes since commit d4ce58082f206bf6e7d697380c7bc5480a8b0264:

  net-tcp: /proc/sys/net/ipv4/tcp_probe_interval is a u32 not int (2018-09-26 20:33:21 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git 

for you to fetch changes up to b799207e1e1816b09e7a5920fbb2d5fcf6edd681:

  bpf: 32-bit RSH verification must truncate input before the ALU op (2018-10-05 18:41:45 +0200)

----------------------------------------------------------------
Jann Horn (1):
      bpf: 32-bit RSH verification must truncate input before the ALU op

Roman Gushchin (2):
      bpf: harden flags check in cgroup_storage_update_elem()
      bpf: don't accept cgroup local storage with zero value size

 kernel/bpf/local_storage.c |  5 ++++-
 kernel/bpf/verifier.c      | 10 +++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

^ 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:48 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>

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. :)

> ---
>  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] bpf: 32-bit RSH verification must truncate input before the ALU op
From: Jann Horn @ 2018-10-05 17:48 UTC (permalink / raw)
  To: Edward Cree
  Cc: Daniel Borkmann, Alexei Starovoitov, Network Development,
	David S. Miller, kernel list
In-Reply-To: <c34442b0-be90-450e-8c20-6e490cc45be9@solarflare.com>

On Fri, Oct 5, 2018 at 7:45 PM Edward Cree <ecree@solarflare.com> wrote:
> On 05/10/18 17:17, Jann Horn wrote:
> > When I wrote commit 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification"), I
> > assumed that, in order to emulate 64-bit arithmetic with 32-bit logic, it
> > is sufficient to just truncate the output to 32 bits; and so I just moved
> > the register size coercion that used to be at the start of the function to
> > the end of the function.
> >
> > That assumption is true for almost every op, but not for 32-bit right
> > shifts, because those can propagate information towards the least
> > significant bit. Fix it by always truncating inputs for 32-bit ops to 32
> > bits.
> >
> > Also get rid of the coerce_reg_to_size() after the ALU op, since that has
> > no effect.
> Might be worth saying something like "because src_reg is passed by value".
> > Fixes: 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification")
> > Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> Acked-by: Edward Cree <ecree@solarflare.com>
> >  kernel/bpf/verifier.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index bb07e74b34a2..465952a8e465 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -2896,6 +2896,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
> >       u64 umin_val, umax_val;
> >       u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
> Incidentally, I don't see why this needs to be a u64 (rather than say a u8).

Yeah, the size of the integer doesn't really matter there... but it's
being compared against other u64 values further down, so I also don't
see a particular need to change it.

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: David Miller @ 2018-10-05 17:50 UTC (permalink / raw)
  To: eric.dumazet; +Cc: dsahern, netdev, weiwan, sd, xiyou.wangcong, dsahern
In-Reply-To: <d6cd6b12-9e7c-b84b-7dfb-20e47b77b8f6@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 5 Oct 2018 06:08:57 -0700

> For some reason I have not received the patch series in my inbox, I
> only got your "series applied" message.

I see what happened.

I did something stupid on vger.kernel.org yesterday which ran a
partition out of disk space, and some postings got lost as a result.

Sorry about that.

> Commit 767a2217533fed6 ("net: common metrics init helper for FIB entries")
> is not correct because we need to better deal with error paths.
> 
> I will submit this more formally when I can reach my workstation in a few minutes :

Thanks Eric.

^ 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: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


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