* 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 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] 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 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
* 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 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
* 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 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 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 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 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 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/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 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] 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 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 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 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 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 iproute2 net-next v2 2/6] include: Add helper to retrieve a __s64 from a netlink msg
From: Vinicius Costa Gomes @ 2018-10-05 17:08 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: netdev, jhs, xiyou.wangcong, jiri, jesus.sanchez-palencia,
simon.fok
In-Reply-To: <20181005084107.GA16936@apalos>
Hi Ilias,
Ilias Apalodimas <ilias.apalodimas@linaro.org> writes:
> On Thu, Oct 04, 2018 at 04:17:07PM -0700, Vinicius Costa Gomes wrote:
>> This allows signed 64-bit integers to be retrieved from a netlink
>> message.
>> ---
>> include/libnetlink.h | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/include/libnetlink.h b/include/libnetlink.h
>> index 9d9249e6..88164975 100644
>> --- a/include/libnetlink.h
>> +++ b/include/libnetlink.h
>> @@ -185,6 +185,13 @@ static inline __u64 rta_getattr_u64(const struct rtattr *rta)
>> memcpy(&tmp, RTA_DATA(rta), sizeof(__u64));
>> return tmp;
>> }
>> +static inline __s64 rta_getattr_s64(const struct rtattr *rta)
>> +{
>> + __s64 tmp;
>> +
>> + memcpy(&tmp, RTA_DATA(rta), sizeof(__s64));
> Maybe change it to sizeof(tmp)?
Will fix. Thanks.
>> + return tmp;
>> +}
>> static inline const char *rta_getattr_str(const struct rtattr *rta)
>> {
>> return (const char *)RTA_DATA(rta);
>> --
>> 2.19.0
>>
Cheers,
^ permalink raw reply
* [PATCH net-next 6/6] net: hns3: Fix for rx vlan id handle to support Rev 0x21 hardware
From: Salil Mehta @ 2018-10-05 17:03 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, Jian Shen
In-Reply-To: <20181005170329.6512-1-salil.mehta@huawei.com>
From: Jian Shen <shenjian15@huawei.com>
In revision 0x20, we use vlan id != 0 to check whether a vlan tag
has been offloaded, so vlan id 0 is not supported.
In revision 0x21, rx buffer descriptor adds two bits to indicate
whether one or more vlan tags have been offloaded, so vlan id 0
is valid now.
This patch seperates the handle for vlan id 0, add vlan id 0 support
for revision 0x21.
Fixes: 5b5455a9ed5a ("net: hns3: Add STRP_TAGP field support for hardware revision 0x21")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 30 +++++++++++--------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 3f36c38..e9d4564 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2200,18 +2200,18 @@ static void hns3_rx_skb(struct hns3_enet_ring *ring, struct sk_buff *skb)
napi_gro_receive(&ring->tqp_vector->napi, skb);
}
-static u16 hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
- struct hns3_desc *desc, u32 l234info)
+static bool hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
+ struct hns3_desc *desc, u32 l234info,
+ u16 *vlan_tag)
{
struct pci_dev *pdev = ring->tqp->handle->pdev;
- u16 vlan_tag;
if (pdev->revision == 0x20) {
- vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
- if (!(vlan_tag & VLAN_VID_MASK))
- vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
+ *vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
+ if (!(*vlan_tag & VLAN_VID_MASK))
+ *vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
- return vlan_tag;
+ return (*vlan_tag != 0);
}
#define HNS3_STRP_OUTER_VLAN 0x1
@@ -2220,17 +2220,14 @@ static u16 hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
switch (hnae3_get_field(l234info, HNS3_RXD_STRP_TAGP_M,
HNS3_RXD_STRP_TAGP_S)) {
case HNS3_STRP_OUTER_VLAN:
- vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
- break;
+ *vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
+ return true;
case HNS3_STRP_INNER_VLAN:
- vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
- break;
+ *vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
+ return true;
default:
- vlan_tag = 0;
- break;
+ return false;
}
-
- return vlan_tag;
}
static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
@@ -2332,8 +2329,7 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX) {
u16 vlan_tag;
- vlan_tag = hns3_parse_vlan_tag(ring, desc, l234info);
- if (vlan_tag & VLAN_VID_MASK)
+ if (hns3_parse_vlan_tag(ring, desc, l234info, &vlan_tag))
__vlan_hwaccel_put_tag(skb,
htons(ETH_P_8021Q),
vlan_tag);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 5/6] net: hns3: Add egress/ingress vlan filter for revision 0x21
From: Salil Mehta @ 2018-10-05 17:03 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, Zhongzhu Liu, Jian Shen
In-Reply-To: <20181005170329.6512-1-salil.mehta@huawei.com>
From: Zhongzhu Liu <liuzhongzhu@huawei.com>
In revision 0x21, hw supports both ingress and egress vlan filter.
This patch adds support for it.
Signed-off-by: Zhongzhu Liu <liuzhongzhu@huawei.com>
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 45 +++++++++++++++++-----
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index d95ea9b..ca1a936 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5523,7 +5523,7 @@ static int hclge_do_ioctl(struct hnae3_handle *handle, struct ifreq *ifr,
}
static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
- bool filter_en)
+ u8 fe_type, bool filter_en)
{
struct hclge_vlan_filter_ctrl_cmd *req;
struct hclge_desc desc;
@@ -5533,7 +5533,7 @@ static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
req = (struct hclge_vlan_filter_ctrl_cmd *)desc.data;
req->vlan_type = vlan_type;
- req->vlan_fe = filter_en;
+ req->vlan_fe = filter_en ? fe_type : 0;
ret = hclge_cmd_send(&hdev->hw, &desc, 1);
if (ret)
@@ -5545,13 +5545,30 @@ static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
#define HCLGE_FILTER_TYPE_VF 0
#define HCLGE_FILTER_TYPE_PORT 1
+#define HCLGE_FILTER_FE_EGRESS_V1_B BIT(0)
+#define HCLGE_FILTER_FE_NIC_INGRESS_B BIT(0)
+#define HCLGE_FILTER_FE_NIC_EGRESS_B BIT(1)
+#define HCLGE_FILTER_FE_ROCE_INGRESS_B BIT(2)
+#define HCLGE_FILTER_FE_ROCE_EGRESS_B BIT(3)
+#define HCLGE_FILTER_FE_EGRESS (HCLGE_FILTER_FE_NIC_EGRESS_B \
+ | HCLGE_FILTER_FE_ROCE_EGRESS_B)
+#define HCLGE_FILTER_FE_INGRESS (HCLGE_FILTER_FE_NIC_INGRESS_B \
+ | HCLGE_FILTER_FE_ROCE_INGRESS_B)
static void hclge_enable_vlan_filter(struct hnae3_handle *handle, bool enable)
{
struct hclge_vport *vport = hclge_get_vport(handle);
struct hclge_dev *hdev = vport->back;
- hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF, enable);
+ if (hdev->pdev->revision >= 0x21) {
+ hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+ HCLGE_FILTER_FE_EGRESS, enable);
+ hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
+ HCLGE_FILTER_FE_INGRESS, enable);
+ } else {
+ hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+ HCLGE_FILTER_FE_EGRESS_V1_B, enable);
+ }
}
static int hclge_set_vf_vlan_common(struct hclge_dev *hdev, int vfid,
@@ -5853,13 +5870,23 @@ static int hclge_init_vlan_config(struct hclge_dev *hdev)
int ret;
int i;
- ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF, true);
- if (ret)
- return ret;
+ if (hdev->pdev->revision >= 0x21) {
+ ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+ HCLGE_FILTER_FE_EGRESS, true);
+ if (ret)
+ return ret;
- ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT, true);
- if (ret)
- return ret;
+ ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
+ HCLGE_FILTER_FE_INGRESS, true);
+ if (ret)
+ return ret;
+ } else {
+ ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+ HCLGE_FILTER_FE_EGRESS_V1_B,
+ true);
+ if (ret)
+ return ret;
+ }
hdev->vlan_type_cfg.rx_in_fst_vlan_type = HCLGE_DEF_VLAN_TYPE;
hdev->vlan_type_cfg.rx_in_sec_vlan_type = HCLGE_DEF_VLAN_TYPE;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next] fib_tests: Add tests for invalid metric on route
From: David Ahern @ 2018-10-05 17:01 UTC (permalink / raw)
To: netdev, davem; +Cc: eric.dumazet, David Ahern
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.
tools/testing/selftests/net/fib_tests.sh | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 491332713dd9..802b4af18729 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -711,14 +711,14 @@ route_setup()
ip -netns ns2 li add dummy1 type dummy
ip -netns ns2 li set dummy1 up
- $IP -6 addr add 2001:db8:101::1/64 dev veth1
- $IP -6 addr add 2001:db8:103::1/64 dev veth3
+ $IP -6 addr add 2001:db8:101::1/64 dev veth1 nodad
+ $IP -6 addr add 2001:db8:103::1/64 dev veth3 nodad
$IP addr add 172.16.101.1/24 dev veth1
$IP addr add 172.16.103.1/24 dev veth3
- ip -netns ns2 -6 addr add 2001:db8:101::2/64 dev veth2
- ip -netns ns2 -6 addr add 2001:db8:103::2/64 dev veth4
- ip -netns ns2 -6 addr add 2001:db8:104::1/64 dev dummy1
+ ip -netns ns2 -6 addr add 2001:db8:101::2/64 dev veth2 nodad
+ ip -netns ns2 -6 addr add 2001:db8:103::2/64 dev veth4 nodad
+ ip -netns ns2 -6 addr add 2001:db8:104::1/64 dev dummy1 nodad
ip -netns ns2 addr add 172.16.101.2/24 dev veth2
ip -netns ns2 addr add 172.16.103.2/24 dev veth4
@@ -1043,6 +1043,9 @@ ipv6_route_metrics_test()
run_cmd "ip netns exec ns1 ping6 -w1 -c1 -s 1500 2001:db8:104::1"
log_test $? 0 "Using route with mtu metric"
+ run_cmd "$IP -6 ro add 2001:db8:114::/64 via 2001:db8:101::2 congctl lock foo"
+ log_test $? 2 "Invalid metric (fails metric_convert)"
+
route_cleanup
}
@@ -1432,6 +1435,9 @@ ipv4_route_metrics_test()
run_cmd "ip netns exec ns1 ping -w1 -c1 -s 1500 172.16.104.1"
log_test $? 0 "Using route with mtu metric"
+ run_cmd "$IP ro add 172.16.111.0/24 via 172.16.101.2 congctl lock foo"
+ log_test $? 2 "Invalid metric (fails metric_convert)"
+
route_cleanup
}
--
2.11.0
^ permalink raw reply related
* RE: [RFC PATCH] skb: Define NET_IP_ALIGN based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
From: David Laight @ 2018-10-05 16:59 UTC (permalink / raw)
To: 'Ben Hutchings', netdev@vger.kernel.org
Cc: Ben Dooks, linux-kernel@lists.codethink.co.uk,
linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org
In-Reply-To: <20181004173631.3nchegr6rm3jgz24@xylophone.i.decadent.org.uk>
From: Ben Hutchings
> Sent: 04 October 2018 18:37
>
> NET_IP_ALIGN is supposed to be defined as 0 if DMA writes to an
> unaligned buffer would be more expensive than CPU access to unaligned
> header fields, and otherwise defined as 2.
>
> Currently only ppc64 and x86 configurations define it to be 0.
> However several other architectures (conditionally) define
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, which seems to imply that
> NET_IP_ALIGN should be 0.
>
> Remove the overriding definitions for ppc64 and x86 and define
> NET_IP_ALIGN solely based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
Even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set unaligned
accesses are likely to be slightly slower than aligned ones.
So having NET_IP_ALIGN set to 2 might make sense even on x86.
(ISTR DM saying why this isn't done.)
I've also met systems when misaligned DMA transfers (for ethernet receive)
were so bad that is was necessary to DMA to a 4n aligned buffer and
then do a misaligned copy to the real rx buffer (skb equiv) for the
network stack - which required the buffer be 4n+2 aligned.
(sparc sbus with the original DMA part.)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
From: Al Viro @ 2018-10-05 23:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Network Development, LKML, kernel-team
In-Reply-To: <20181005222752.l5da54rpww6tlyfy@ast-mbp.dhcp.thefacebook.com>
On Fri, Oct 05, 2018 at 03:27:54PM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 05, 2018 at 03:09:20PM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 5, 2018 at 3:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Oct 05, 2018 at 05:46:59AM +0100, Al Viro wrote:
> > >
> > > > Another problem is your direct poking in ->i_ino. It's not
> > > > something directly exposed to userland at the moment and it should
> > > > not become such.
> > >
> > > The patch is not making i_ino directly exposed.
> > > Only 'struct bpf_file_info' is exposed to user space / bpf programs.
> >
> > I think Al is saying that the valie of i_ino is not something that
> > user code is permitted to know regardless of how you format it because
> > it may or may not actually match the value returned by stat().
> > Another way of saying that is that your patch is digging into an
> > internal data structure and is doing it wrong.
>
> several fs implementation I've looked at just do generic_fillattr()
> for these fields. Are you saying some FS don't use inode->i_ino at all?
> And it's bogus, hence shouldn't be read?
Bloody wonderful... "Several instances use a library function and do not
override that part of its results; ergo, let's assume that all of them
do the same".
generic_fillattr() is a library function. In a lot of cases this is all
->getattr() instance needs to do. And yes, use of ->i_dev and ->i_ino
to intialize ->st_dev and ->st_ino happens to be the default. However,
this is entirely up to the filesystem in question. These fields are
fs-private; whether to use them for stat(2) (or anything userland-visible,
really) or to calculate some other value is up to individual filesystem.
FWIW, finding which instances do that is as simple as
grep -n '[-]>[[:space:]]*ino[[:space:]]*=' `git grep -l -w generic_fillattr`
on the plausible theory that ->getattr() instances will be using that helper
at least for some of the fields. Discarding fs/stat.c, where generic_fillattr()
itself lives, we are left with
fs/ceph/inode.c:558: if (realm->ino == ci->i_vino.ino)
fs/ceph/inode.c:2268: stat->ino = ceph_translate_ino(inode->i_sb, inode->i_ino);
fs/cifs/inode.c:2067: stat->ino = CIFS_I(inode)->uniqueid;
fs/fat/file.c:410: stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode);
fs/fuse/dir.c:866: stat->ino = attr->ino;
fs/fuse/dir.c:954: stat->ino = fi->orig_ino;
fs/nfs/inode.c:841: stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
Trivial examination shows that all matches except the first one *are* in ->getattr()
instances of the filesystems in question or are called from such.
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.
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.
The same goes for tracepoints, etc. - turning random details of implementation into
a carved in stone ABI is actively harmful.
NAK.
PS: If anything, visibility to hooks should be opt-in. 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. Нахуй нам это надо?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox