* Re: [PATCH bpf] bpf: Fix integer overflow in queue_stack_map_alloc.
From: Greg KH @ 2018-11-22 16:04 UTC (permalink / raw)
To: Wei Wu; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eric Dumazet
In-Reply-To: <CACmwppz=P2h-uM2OTc2CvnDg_Qgfhtk5-vzk0Jge1SAi2sym+A@mail.gmail.com>
On Thu, Nov 22, 2018 at 11:59:02PM +0800, Wei Wu wrote:
> Integer overflow in queue_stack_map_alloc when calculating size may
> lead to heap overflow of arbitrary length.
> The patch fix it by checking whether attr->max_entries+1 <
> attr->max_entries and bailing out if it is the case.
> The vulnerability is discovered with the assistance of syzkaller.
>
> Reported-by: Wei Wu <ww9210@gmail.com>
> To: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: netdev <netdev@vger.kernel.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Wei Wu <ww9210@gmail.com>
> ---
> kernel/bpf/queue_stack_maps.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> index 8bbd72d3a121..c35a8a4721c8 100644
> --- a/kernel/bpf/queue_stack_maps.c
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -67,6 +67,8 @@ static struct bpf_map *queue_stack_map_alloc(union
> bpf_attr *attr)
> u64 queue_size, cost;
>
> size = attr->max_entries + 1;
> + if (size < attr->max_entries)
> + return ERR_PTR(-EINVAL);
> value_size = attr->value_size;
Your tabs got eaten by your email client and they all disappeared,
making the patch impossible to apply :(
Care to try again?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: bridge: add no_linklocal_learn bool option
From: Nikolay Aleksandrov @ 2018-11-22 16:06 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, roopa, davem, bridge
In-Reply-To: <20181122160435.GI15403@lunn.ch>
On 22/11/2018 18:04, Andrew Lunn wrote:
>> int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt)
>> {
>> - int optval = 0;
>> -
>> switch (opt) {
>> + case BR_BOOLOPT_NO_LL_LEARN:
>> + return br_opt_get(br, BROPT_NO_LL_LEARN);
>> default:
>> break;
>> }
>>
>> - return optval;
>> + return 0;
>> }
>
> It seems like 1/2 of that change belongs in the previous patch.
>
Yes, I could squash this into patch 01.
>> --- a/net/bridge/br_sysfs_br.c
>> +++ b/net/bridge/br_sysfs_br.c
>> @@ -328,6 +328,27 @@ static ssize_t flush_store(struct device *d,
>> }
>> static DEVICE_ATTR_WO(flush);
>>
>> +static ssize_t no_linklocal_learn_show(struct device *d,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct net_bridge *br = to_bridge(d);
>> + return sprintf(buf, "%d\n", br_boolopt_get(br, BR_BOOLOPT_NO_LL_LEARN));
>> +}
>> +
>> +static int set_no_linklocal_learn(struct net_bridge *br, unsigned long val)
>> +{
>> + return br_boolopt_toggle(br, BR_BOOLOPT_NO_LL_LEARN, !!val);
>> +}
>> +
>> +static ssize_t no_linklocal_learn_store(struct device *d,
>> + struct device_attribute *attr,
>> + const char *buf, size_t len)
>> +{
>> + return store_bridge_parm(d, buf, len, set_no_linklocal_learn);
>> +}
>> +static DEVICE_ATTR_RW(no_linklocal_learn);
>
> I thought we where trying to move away from sysfs? Do we need to add
> new options here? It seems like forcing people to use iproute2 for
> newer options is a good way to get people to convert to iproute2.
>
Being consistent, all of the bridge options are exported via sysfs. If we start
ignoring it now it'll be confusing.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: bridge: add support for user-controlled bool options
From: Nikolay Aleksandrov @ 2018-11-22 16:07 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, roopa, davem, bridge
In-Reply-To: <20181122155256.GH15403@lunn.ch>
On 22/11/2018 17:52, Andrew Lunn wrote:
>> +void br_boolopt_multi_get(const struct net_bridge *br,
>> + struct br_boolopt_multi *bm)
>> +{
>> + u32 optval = 0;
>> + int opt_id;
>> +
>> + for (opt_id = 0; opt_id < BR_BOOLOPT_MAX; opt_id++)
>> + optval |= (br_boolopt_get(br, opt_id) << opt_id);
>> +
>> + bm->optval = optval;
>> + bm->optmask = 0;
>
> Maybe set optmask to indicate which bits this kernel supports?
>
I like the idea, will add for v2.
Thanks,
Nik
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: bridge: add support for user-controlled bool options
From: Nikolay Aleksandrov @ 2018-11-22 16:13 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, roopa, davem, bridge
In-Reply-To: <20181122154951.GG15403@lunn.ch>
On 22/11/2018 17:49, Andrew Lunn wrote:
>> +/* br_boolopt_toggle - change user-controlled boolean option
>> + *
>> + * @br: bridge device
>> + * @opt: id of the option to change
>> + * @on: new option value
>> + *
>> + * Changes the value of the respective boolean option to @on taking care of
>> + * any internal option value mapping and configuration.
>> + */
>> +int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on)
>> +{
>> + int err = -ENOENT;
>> +
>> + switch (opt) {
>> + default:
>> + break;
>> + }
>> +
>> + return err;
>> +}
>> +
>
>
>> +int br_boolopt_multi_toggle(struct net_bridge *br,
>> + struct br_boolopt_multi *bm)
>> +{
>> + unsigned long bitmap = bm->optmask;
>> + int err = 0;
>> + int opt_id;
>> +
>> + for_each_set_bit(opt_id, &bitmap, BR_BOOLOPT_MAX) {
>> + bool on = !!(bm->optval & BIT(opt_id));
>> +
>> + err = br_boolopt_toggle(br, opt_id, on);
>> + if (err) {
>> + br_debug(br, "boolopt multi-toggle error: option: %d current: %d new: %d error: %d\n",
>> + opt_id, br_boolopt_get(br, opt_id), on, err);
>> + break;
>> + }
>
> An old kernel with a new iproute2 might partially succeed in toggling
> some low bits, but then silently ignore a high bit that is not
> supported by the kernel. As a user, i want to know the kernel does not
> support an option i'm trying to toggle.
>
That is already how netlink option setting works, if the option isn't supported
it is silently ignored. I tried to stay as close to the current behaviour as possible.
It also applies to partial option changes, some options will be changed until an error
is encountered.
> Can you walk all the bits in the u32 from the MSB to the LSB? That
> should avoid this problem.
>
I did wonder about this and left it as netlink option behaviour but
I can leave the walk as is and just check if the highest order set bit >= BR_BOOLOPT_MAX
and err out with ENOTSUPP for example. Will reconsider for v2.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] netns: remove net arg from rtnl_net_fill()
From: David Ahern @ 2018-11-22 16:18 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem
In-Reply-To: <20181122155031.3495-2-nicolas.dichtel@6wind.com>
On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
> This argument is not used anymore.
>
> Fixes: cab3c8ec8d57 ("netns: always provide the id to rtnl_net_fill()")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/core/net_namespace.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [PATCH net-next 0/3] basic in order support for vhost_net
From: Jason Wang @ 2018-11-23 3:00 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
Hi:
This series implement basic in order feature support for
vhost_net. This feature requires both driver and device to use
descriptors in order which can simplify the implementation and
optimizaton for both side. The series also implement a simple
optimization that avoid read available ring. Test shows 10%
performance improvement.
More optimizations could be done on top.
Jason Wang (3):
virtio: introduce in order feature bit
vhost_net: support in order feature
vhost: don't touch avail ring if in_order is negotiated
drivers/vhost/net.c | 6 ++++--
drivers/vhost/vhost.c | 19 ++++++++++++-------
include/uapi/linux/virtio_config.h | 6 ++++++
3 files changed, 22 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 2/3] vhost_net: support in order feature
From: Jason Wang @ 2018-11-23 3:00 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20181123030016.4924-1-jasowang@redhat.com>
This makes vhost_net to support in order feature. This is as simple as
use datacopy path when it was negotiated. An alternative is not to
advertise in order when zerocopy is enabled which tends to be
suboptimal consider zerocopy may suffer from e.g HOL issues.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d919284f103b..bdf5de5a7eb2 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -74,7 +74,8 @@ enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
(1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
(1ULL << VIRTIO_NET_F_MRG_RXBUF) |
- (1ULL << VIRTIO_F_IOMMU_PLATFORM)
+ (1ULL << VIRTIO_F_IOMMU_PLATFORM) |
+ (1ULL << VIRTIO_F_IN_ORDER)
};
enum {
@@ -971,7 +972,8 @@ static void handle_tx(struct vhost_net *net)
vhost_disable_notify(&net->dev, vq);
vhost_net_disable_vq(net, vq);
- if (vhost_sock_zcopy(sock))
+ if (vhost_sock_zcopy(sock) &&
+ !vhost_has_feature(vq, VIRTIO_F_IN_ORDER))
handle_tx_zerocopy(net, sock);
else
handle_tx_copy(net, sock);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/3] vhost: don't touch avail ring if in_order is negotiated
From: Jason Wang @ 2018-11-23 3:00 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20181123030016.4924-1-jasowang@redhat.com>
Device use descriptors table in order, so there's no need to read
index from available ring. This eliminate the cache contention on
avail ring completely.
Virito-user + vhost_kernel + XDP_DROP gives about ~10% improvement on
TX from 4.8Mpps to 5.3Mpps on Intel(R) Core(TM) i7-5600U CPU @
2.60GHz.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3a5f81a66d34..c8be151bc897 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2002,6 +2002,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
__virtio16 avail_idx;
__virtio16 ring_head;
int ret, access;
+ bool in_order = vhost_has_feature(vq, VIRTIO_F_IN_ORDER);
/* Check it isn't doing very strange things with descriptor numbers. */
last_avail_idx = vq->last_avail_idx;
@@ -2034,15 +2035,19 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Grab the next descriptor number they're advertising, and increment
* the index we've seen. */
- if (unlikely(vhost_get_avail(vq, ring_head,
- &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
- vq_err(vq, "Failed to read head: idx %d address %p\n",
- last_avail_idx,
- &vq->avail->ring[last_avail_idx % vq->num]);
- return -EFAULT;
+ if (!in_order) {
+ if (unlikely(vhost_get_avail(vq, ring_head,
+ &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
+ vq_err(vq, "Failed to read head: idx %d address %p\n",
+ last_avail_idx,
+ &vq->avail->ring[last_avail_idx % vq->num]);
+ return -EFAULT;
+ }
+ head = vhost16_to_cpu(vq, ring_head);
+ } else {
+ head = last_avail_idx & (vq->num - 1);
}
- head = vhost16_to_cpu(vq, ring_head);
/* If their number is silly, that's an error. */
if (unlikely(head >= vq->num)) {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next,v3 00/12] add flow_rule infrastructure
From: Marcelo Ricardo Leitner @ 2018-11-22 16:22 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
On Wed, Nov 21, 2018 at 03:51:20AM +0100, Pablo Neira Ayuso wrote:
> Hi,
>
> This patchset is the third iteration [1] [2] [3] to introduce a kernel
> intermediate (IR) to express ACL hardware offloads.
On v2 cover letter you had:
"""
However, cost of this layer is very small, adding 1 million rules via
tc -batch, perf shows:
0.06% tc [kernel.vmlinux] [k] tc_setup_flow_action
"""
The above doesn't include time spent on children calls and I'm worried
about the new allocation done by flow_rule_alloc(), as it can impact
rule insertion rate. I'll run some tests here and report back.
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] netns: introduce 'struct net_fill_args'
From: David Ahern @ 2018-11-22 16:23 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem
In-Reply-To: <20181122155031.3495-3-nicolas.dichtel@6wind.com>
On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
> This is a preparatory work. To avoid having to much arguments for the
> function rtnl_net_fill(), a new structure is defined.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/core/net_namespace.c | 48 ++++++++++++++++++++++++++++------------
> 1 file changed, 34 insertions(+), 14 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] netns: add support of NETNSA_TARGET_NSID
From: David Ahern @ 2018-11-22 16:32 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem
In-Reply-To: <20181122155031.3495-4-nicolas.dichtel@6wind.com>
On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
> Like it was done for link and address, add the ability to perform get/dump
> in another netns by specifying a target nsid attribute.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> include/uapi/linux/net_namespace.h | 1 +
> net/core/net_namespace.c | 86 ++++++++++++++++++++++++++----
> 2 files changed, 76 insertions(+), 11 deletions(-)
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] netns: enable to specify a nsid for a get request
From: David Ahern @ 2018-11-22 16:32 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem
In-Reply-To: <20181122155031.3495-5-nicolas.dichtel@6wind.com>
On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
> Combined with NETNSA_TARGET_NSID, it enables to "translate" a nsid from one
> netns to a nsid of another netns.
> This is useful when using NETLINK_F_LISTEN_ALL_NSID because it helps the
> user to interpret a nsid received from an other netns.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/core/net_namespace.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next,v3 04/12] cls_api: add translator to flow_action representation
From: Marcelo Ricardo Leitner @ 2018-11-22 16:33 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-5-pablo@netfilter.org>
On Wed, Nov 21, 2018 at 03:51:24AM +0100, Pablo Neira Ayuso wrote:
...
> +int tc_setup_flow_action(struct flow_action *flow_action,
> + const struct tcf_exts *exts)
> +{
> + const struct tc_action *act;
> + int i, j, k;
> +
> + if (!exts)
> + return 0;
> +
> + j = 0;
> + tcf_exts_for_each_action(i, act, exts) {
> + struct flow_action_entry *key;
^^^^^ ^^^
> +
> + key = &flow_action->entries[j];
^^^ ^^^^^^^
Considering previous changes, what about a s/key/entry/ in the
variable name here too?
> + if (is_tcf_gact_ok(act)) {
> + key->id = FLOW_ACTION_ACCEPT;
> + } else if (is_tcf_gact_shot(act)) {
> + key->id = FLOW_ACTION_DROP;
> + } else if (is_tcf_gact_trap(act)) {
> + key->id = FLOW_ACTION_TRAP;
> + } else if (is_tcf_gact_goto_chain(act)) {
> + key->id = FLOW_ACTION_GOTO;
> + key->chain_index = tcf_gact_goto_chain_index(act);
> + } else if (is_tcf_mirred_egress_redirect(act)) {
> + key->id = FLOW_ACTION_REDIRECT;
> + key->dev = tcf_mirred_dev(act);
> + } else if (is_tcf_mirred_egress_mirror(act)) {
> + key->id = FLOW_ACTION_MIRRED;
> + key->dev = tcf_mirred_dev(act);
> + } else if (is_tcf_vlan(act)) {
> + switch (tcf_vlan_action(act)) {
> + case TCA_VLAN_ACT_PUSH:
> + key->id = FLOW_ACTION_VLAN_PUSH;
> + key->vlan.vid = tcf_vlan_push_vid(act);
> + key->vlan.proto = tcf_vlan_push_proto(act);
> + key->vlan.prio = tcf_vlan_push_prio(act);
> + break;
> + case TCA_VLAN_ACT_POP:
> + key->id = FLOW_ACTION_VLAN_POP;
> + break;
> + case TCA_VLAN_ACT_MODIFY:
> + key->id = FLOW_ACTION_VLAN_MANGLE;
> + key->vlan.vid = tcf_vlan_push_vid(act);
> + key->vlan.proto = tcf_vlan_push_proto(act);
> + key->vlan.prio = tcf_vlan_push_prio(act);
> + break;
> + default:
> + goto err_out;
> + }
> + } else if (is_tcf_tunnel_set(act)) {
> + key->id = FLOW_ACTION_TUNNEL_ENCAP;
> + key->tunnel = tcf_tunnel_info(act);
> + } else if (is_tcf_tunnel_release(act)) {
> + key->id = FLOW_ACTION_TUNNEL_DECAP;
> + key->tunnel = tcf_tunnel_info(act);
> + } else if (is_tcf_pedit(act)) {
> + for (k = 0; k < tcf_pedit_nkeys(act); k++) {
> + switch (tcf_pedit_cmd(act, k)) {
> + case TCA_PEDIT_KEY_EX_CMD_SET:
> + key->id = FLOW_ACTION_MANGLE;
> + break;
> + case TCA_PEDIT_KEY_EX_CMD_ADD:
> + key->id = FLOW_ACTION_ADD;
> + break;
> + default:
> + goto err_out;
> + }
> + key->mangle.htype = tcf_pedit_htype(act, k);
> + key->mangle.mask = tcf_pedit_mask(act, k);
> + key->mangle.val = tcf_pedit_val(act, k);
> + key->mangle.offset = tcf_pedit_offset(act, k);
> + key = &flow_action->entries[++j];
> + }
> + } else if (is_tcf_csum(act)) {
> + key->id = FLOW_ACTION_CSUM;
> + key->csum_flags = tcf_csum_update_flags(act);
> + } else if (is_tcf_skbedit_mark(act)) {
> + key->id = FLOW_ACTION_MARK;
> + key->mark = tcf_skbedit_mark(act);
> + } else {
> + goto err_out;
> + }
> +
> + if (!is_tcf_pedit(act))
> + j++;
> + }
> + return 0;
> +err_out:
> + return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(tc_setup_flow_action);
^ permalink raw reply
* Re: [PATCH net-next 2/3] tcp: implement coalescing on backlog queue
From: Yuchung Cheng @ 2018-11-22 16:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David S . Miller, netdev, Jean-Louis Dupond,
Neal Cardwell
In-Reply-To: <ed3b38f5-14f9-2cae-b52d-35fd011ebcf0@gmail.com>
On Wed, Nov 21, 2018 at 2:40 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 11/21/2018 02:31 PM, Yuchung Cheng wrote:
>> On Wed, Nov 21, 2018 at 9:52 AM, Eric Dumazet <edumazet@google.com> wrote:
>
>>> +
>> Really nice! would it make sense to re-use (some of) the similar
>> tcp_try_coalesce()?
>>
>
> Maybe, but it is a bit complex, since skbs in receive queues (regular or out of order)
> are accounted differently (they have skb->destructor set)
>
> Also they had the TCP header pulled already, while the backlog coalescing also has
> to make sure TCP options match.
>
> Not sure if we want to add extra parameters and conditional checks...
Makes sense.
Acked-by: Yuchung Cheng <ycheng@google.com>
>
>
^ permalink raw reply
* [PATCH bpf] bpf: Fix integer overflow in queue_stack_map_alloc.
From: ww9210 @ 2018-11-22 16:35 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Daniel Borkmann, netdev, Eric Dumazet, Greg KH, ww9210
Integer overflow in queue_stack_map_alloc when calculating size may lead to heap overflow of arbitrary length.
The patch fix it by checking whether attr->max_entries+1 < attr->max_entries and bailing out if it is the case.
The vulnerability is discovered with the assistance of syzkaller.
Reported-by: Wei Wu <ww9210@gmail.com>
To: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: netdev <netdev@vger.kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Wei Wu <ww9210@gmail.com>
---
kernel/bpf/queue_stack_maps.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index 8bbd72d3a121..c35a8a4721c8 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -67,6 +67,8 @@ static struct bpf_map *queue_stack_map_alloc(union bpf_attr *attr)
u64 queue_size, cost;
size = attr->max_entries + 1;
+ if (size < attr->max_entries)
+ return ERR_PTR(-EINVAL);
value_size = attr->value_size;
queue_size = sizeof(*qs) + (u64) value_size * size;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v2 5/5] netns: enable to dump full nsid translation table
From: David Ahern @ 2018-11-22 16:40 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem
In-Reply-To: <20181122155031.3495-6-nicolas.dichtel@6wind.com>
On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index dd25fb22ad45..25030e0317a2 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -745,6 +745,8 @@ struct net_fill_args {
> int flags;
> int cmd;
> int nsid;
> + bool add_ref;
> + int ref_nsid;
> };
>
> static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
> @@ -763,6 +765,10 @@ static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
> if (nla_put_s32(skb, NETNSA_NSID, args->nsid))
> goto nla_put_failure;
>
> + if (args->add_ref &&
> + nla_put_s32(skb, NETNSA_CURRENT_NSID, args->ref_nsid))
> + goto nla_put_failure;
> +
you need to add NETNSA_CURRENT_NSID to rtnl_net_get_size.
^ permalink raw reply
* Re: [PATCH net-next v2 5/5] netns: enable to dump full nsid translation table
From: Nicolas Dichtel @ 2018-11-22 16:42 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem
In-Reply-To: <b17f8a76-6df2-24b7-4867-4d4814426e35@gmail.com>
Le 22/11/2018 à 17:40, David Ahern a écrit :
> On 11/22/18 8:50 AM, Nicolas Dichtel wrote:
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index dd25fb22ad45..25030e0317a2 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -745,6 +745,8 @@ struct net_fill_args {
>> int flags;
>> int cmd;
>> int nsid;
>> + bool add_ref;
>> + int ref_nsid;
>> };
>>
>> static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
>> @@ -763,6 +765,10 @@ static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
>> if (nla_put_s32(skb, NETNSA_NSID, args->nsid))
>> goto nla_put_failure;
>>
>> + if (args->add_ref &&
>> + nla_put_s32(skb, NETNSA_CURRENT_NSID, args->ref_nsid))
>> + goto nla_put_failure;
>> +
>
> you need to add NETNSA_CURRENT_NSID to rtnl_net_get_size.
>
Good catch.
I thought to this and I forgot at the end :/
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: David Ahern @ 2018-11-22 16:43 UTC (permalink / raw)
To: Toshiaki Makita, netdev@vger.kernel.org
Cc: Paweł Staszewski, Jesper Dangaard Brouer, Saeed Mahameed,
Jason Wang, Michael S. Tsirkin, David Miller
In-Reply-To: <30c14d9e-5ae7-b02f-9db9-db2590714e7d@lab.ntt.co.jp>
On 11/21/18 5:53 PM, Toshiaki Makita wrote:
>> We really need consistency in the counters and at a minimum, users
>> should be able to track packet and byte counters for both Rx and Tx
>> including XDP.
>>
>> It seems to me the Rx and Tx packet, byte and dropped counters returned
>> for the standard device stats (/proc/net/dev, ip -s li show, ...) should
>> include all packets managed by the driver regardless of whether they are
>> forwarded / dropped in XDP or go up the Linux stack. This also aligns
>
> Agreed. When I introduced virtio_net XDP counters, I just forgot to
> update tx packets/bytes counters on ndo_xdp_xmit. Probably I thought it
> is handled by free_old_xmit_skbs.
Do you have some time to look at adding the Tx counters to virtio_net?
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: David Ahern @ 2018-11-22 16:51 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Saeed Mahameed,
pstaszewski@itcare.pl, netdev@vger.kernel.org
Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com,
mst@redhat.com
In-Reply-To: <87efbd1prm.fsf@toke.dk>
On 11/22/18 1:26 AM, Toke Høiland-Jørgensen wrote:
> Saeed Mahameed <saeedm@mellanox.com> writes:
>
>>>> I'd say it sounds reasonable to include XDP in the normal traffic
>>>> counters, but having the detailed XDP-specific counters is quite
>>>> useful
>>>> as well... So can't we do both (for all drivers)?
>>>>
>>
>> What are you thinking ?
>> reporting XDP_DROP in interface dropped counter ?
>> and XDP_TX/REDIRECT in the TX counter ?
>> XDP_ABORTED in the err/drop counter ?
>>
>> how about having a special XDP command in the .ndo_bpf that would query
>> the standardized XDP stats ?
>
> Don't have any strong opinions on the mechanism; just pointing out that
> the XDP-specific stats are useful to have separately as well :)
>
I would like to see basic packets, bytes, and dropped counters tracked
for Rx and Tx via the standard netdev counters for all devices. This is
for ease in accounting as well as speed and simplicity for bumping
counters for virtual devices from bpf helpers.
>From there, the XDP ones can be in the driver private stats as they are
currently but with some consistency across drivers for redirects, drops,
any thing else.
So not a radical departure from where we are today, just getting the
agreement for consistency and driver owners to make the changes.
^ permalink raw reply
* Re: [PATCH net-next,v3 12/12] qede: use ethtool_rx_flow_rule() to remove duplicated parser code
From: Marcelo Ricardo Leitner @ 2018-11-22 16:59 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-13-pablo@netfilter.org>
On Wed, Nov 21, 2018 at 03:51:32AM +0100, Pablo Neira Ayuso wrote:
...
> static int
> qede_parse_flower_attr(struct qede_dev *edev, __be16 proto,
> - struct tc_cls_flower_offload *f,
> - struct qede_arfs_tuple *tuple)
> + struct flow_rule *rule, struct qede_arfs_tuple *tuple)
What about s/qede_parse_flower_attr/qede_parse_flow_attr/ or so? As it
is not about flower anymore.
It also helps here:
> -static int qede_flow_spec_to_tuple(struct qede_dev *edev,
> - struct qede_arfs_tuple *t,
> - struct ethtool_rx_flow_spec *fs)
> +static int qede_flow_spec_to_rule(struct qede_dev *edev,
> + struct qede_arfs_tuple *t,
> + struct ethtool_rx_flow_spec *fs)
> {
...
> +
> + if (qede_parse_flower_attr(edev, proto, flow->rule, t)) {
> + err = -EINVAL;
> + goto err_out;
> + }
> +
> + /* Make sure location is valid and filter isn't already set */
> + err = qede_flow_spec_validate(edev, &flow->rule->action, t,
> + fs->location);
...
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: Toke Høiland-Jørgensen @ 2018-11-22 17:00 UTC (permalink / raw)
To: David Ahern, Saeed Mahameed, pstaszewski@itcare.pl,
netdev@vger.kernel.org
Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com,
mst@redhat.com
In-Reply-To: <a27c28b1-a8c7-5821-ea15-97de0fcb7907@gmail.com>
David Ahern <dsahern@gmail.com> writes:
> On 11/22/18 1:26 AM, Toke Høiland-Jørgensen wrote:
>> Saeed Mahameed <saeedm@mellanox.com> writes:
>>
>>>>> I'd say it sounds reasonable to include XDP in the normal traffic
>>>>> counters, but having the detailed XDP-specific counters is quite
>>>>> useful
>>>>> as well... So can't we do both (for all drivers)?
>>>>>
>>>
>>> What are you thinking ?
>>> reporting XDP_DROP in interface dropped counter ?
>>> and XDP_TX/REDIRECT in the TX counter ?
>>> XDP_ABORTED in the err/drop counter ?
>>>
>>> how about having a special XDP command in the .ndo_bpf that would query
>>> the standardized XDP stats ?
>>
>> Don't have any strong opinions on the mechanism; just pointing out that
>> the XDP-specific stats are useful to have separately as well :)
>>
>
> I would like to see basic packets, bytes, and dropped counters tracked
> for Rx and Tx via the standard netdev counters for all devices. This is
> for ease in accounting as well as speed and simplicity for bumping
> counters for virtual devices from bpf helpers.
>
> From there, the XDP ones can be in the driver private stats as they are
> currently but with some consistency across drivers for redirects, drops,
> any thing else.
>
> So not a radical departure from where we are today, just getting the
> agreement for consistency and driver owners to make the changes.
Sounds good to me :)
-Toke
^ permalink raw reply
* patchwork bug?
From: Nicolas Dichtel @ 2018-11-22 17:06 UTC (permalink / raw)
To: David Miller, netdev
Not sure if it's the right place to post that.
When I try to list patches with filters, something like this:
http://patchwork.ozlabs.org/project/netdev/list/?series=&submitter=2036&state=*&q=&archive=both&delegate=34
I can see only page 1. When I click on '2', the page 1 is still displayed and
the page numerotation is removed.
Regards,
Nicolas
^ permalink raw reply
* Re: [RFC v4 1/5] udp_tunnel: add config option to bind to a device
From: David Ahern @ 2018-11-22 17:10 UTC (permalink / raw)
To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181122010713.3995-2-abauvin@scaleway.com>
On 11/21/18 6:07 PM, Alexis Bauvin wrote:
> UDP tunnel sockets are always opened unbound to a specific device. This
> patch allow the socket to be bound on a custom device, which
> incidentally makes UDP tunnels VRF-aware if binding to an l3mdev.
>
> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> ---
> include/net/udp_tunnel.h | 1 +
> net/ipv4/udp_tunnel.c | 10 ++++++++++
> net/ipv6/ip6_udp_tunnel.c | 9 +++++++++
> 3 files changed, 20 insertions(+)
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [RFC v4 2/5] l3mdev: add function to retreive upper master
From: David Ahern @ 2018-11-22 17:11 UTC (permalink / raw)
To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181122010713.3995-3-abauvin@scaleway.com>
On 11/21/18 6:07 PM, Alexis Bauvin wrote:
> Existing functions to retreive the l3mdev of a device did not walk the
> master chain to find the upper master. This patch adds a function to
> find the l3mdev, even indirect through e.g. a bridge:
>
>
...
>
> This will properly resolve the l3mdev of eth0 to vrf-blue.
>
> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> ---
> include/net/l3mdev.h | 22 ++++++++++++++++++++++
> net/l3mdev/l3mdev.c | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [RFC v4 4/5] netdev: add netdev_is_upper_master
From: David Ahern @ 2018-11-22 17:14 UTC (permalink / raw)
To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181122010713.3995-5-abauvin@scaleway.com>
On 11/21/18 6:07 PM, Alexis Bauvin wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 93243479085f..12459036d0da 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7225,6 +7225,23 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
> }
> EXPORT_SYMBOL(netdev_lower_state_changed);
>
> +/**
> + * netdev_is_upper_master - Test if a device is a master, direct or indirect,
> + * of another one.
> + * @dev: device to start looking from
> + * @master: device to test if master of dev
> + */
> +bool netdev_is_upper_master(struct net_device *dev, struct net_device *master)
> +{
> + if (!dev)
> + return false;
> +
> + if (dev->ifindex == master->ifindex)
dev == master should work as well without the dereference.
> + return true;
> + return netdev_is_upper_master(netdev_master_upper_dev_get(dev), master);
> +}
> +EXPORT_SYMBOL(netdev_is_upper_master);
> +
> static void dev_change_rx_flags(struct net_device *dev, int flags)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
>
^ 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