* Re: Request for suggestion on net device re-naming/re-ordering based on DT alias
From: Florian Fainelli @ 2019-02-27 18:45 UTC (permalink / raw)
To: Stephen Hemminger, Harini Katakam
Cc: netdev, linux-kernel, Nicolas Ferre, David Miller, Michal Simek,
Harini Katakam, Harini Katakam
In-Reply-To: <20190227104028.707a75e2@shemminger-XPS-13-9360>
On 2/27/19 10:40 AM, Stephen Hemminger wrote:
> On Wed, 27 Feb 2019 17:24:03 +0530
> Harini Katakam <harinik@xilinx.com> wrote:
>
>> Hi,
>>
>> We've had some users requesting control over net device name order
>> when multiple ethernet devices are present on a system. I've tried a
>> few solutions to this and looked it up on forums. But I apologize if
>> I have missed something.
>>
>> I know that the current system allocates eth<n> as per probe order
>> but that is obviously not stably controlled by user (tried DT
>> re-ordering and defer probe). One solution is to use DT alias names
>> to write to (net_device)->name as follows:
>> Devicetree:
>> aliases {
>> ethernet0 = &mac1;
>> ethernet1 = &mac0;
>> };
>> Driver probe:
>> + /* Read ethernet DT alias id and assign to right device name*/
>> + id = of_alias_get_id(np, "ethernet");
>> + if (id < 0) {
>> + dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
>> + return id;
>> + }
>> + snprintf(dev->name, sizeof(dev->name), "eth%d", id);
>> +
>>
>> These three drivers seem to have something similar for mdio/phy bus IDs:
>> drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
>> of_alias_get_id(dn, "eth");
>> drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
>> of_alias_get_id(np, "ethernet");
>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
>> plat->bus_id = of_alias_get_id(np, "ethernet");
>>
>> Drawback: This approach will break if alias is not provided for one
>> of the interfaces on board. Not to mention, there could be systems
>> with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
>> If one of the drivers does not have an alias read mechanism, it is
>> possible to have clashing ID assignments. Is there any way this
>> solution can be changed to be stable/acceptable?
>>
>> One other alternative I've tried is netdev kernel bootargs but this
>> device name was not being picked by the kernel:
>> https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
>> netdev= <mac1’s interrupt id>, <mac1’s base address>, eth0
>> netdev=<mac0’s interrupt id>, <mac0’s base address>, eth1
>>
>> Could you please suggest any alternatives?
>> Thanks!
>>
>> Regards,
>> Harini
>
> Device naming is a hard problem, and there is no perfect solution.
>
> Device tree should be providing hints to userspace policy for naming, not
> trying to do it in the kernel.
And Device Tree does already, if you look at the uevent attributes that
the kernel sends, there should be ample information to uniquely
determine which physical network device the network device maps to, e
for instance:
cat /sys/class/net/eth*/device/uevent
DRIVER=brcm-systemport
OF_NAME=ethernet
OF_FULLNAME=/rdb/ethernet@9300000
OF_TYPE=network
OF_COMPATIBLE_0=brcm,systemportlite-v1.00
OF_COMPATIBLE_1=brcm,systemport
OF_COMPATIBLE_N=2
OF_ALIAS_0=eth0
MODALIAS=of:NethernetTnetworkCbrcm,systemportlite-v1.00Cbrcm,systemport
--
Florian
^ permalink raw reply
* Re: Request for suggestion on net device re-naming/re-ordering based on DT alias
From: Stephen Hemminger @ 2019-02-27 18:40 UTC (permalink / raw)
To: Harini Katakam
Cc: netdev, linux-kernel, Nicolas Ferre, David Miller, Michal Simek,
Harini Katakam, Harini Katakam
In-Reply-To: <CAFcVEC+8P25MKWhizUUEuamikaZYUOjpppV=59=W3POTEfQqXw@mail.gmail.com>
On Wed, 27 Feb 2019 17:24:03 +0530
Harini Katakam <harinik@xilinx.com> wrote:
> Hi,
>
> We've had some users requesting control over net device name order
> when multiple ethernet devices are present on a system. I've tried a
> few solutions to this and looked it up on forums. But I apologize if
> I have missed something.
>
> I know that the current system allocates eth<n> as per probe order
> but that is obviously not stably controlled by user (tried DT
> re-ordering and defer probe). One solution is to use DT alias names
> to write to (net_device)->name as follows:
> Devicetree:
> aliases {
> ethernet0 = &mac1;
> ethernet1 = &mac0;
> };
> Driver probe:
> + /* Read ethernet DT alias id and assign to right device name*/
> + id = of_alias_get_id(np, "ethernet");
> + if (id < 0) {
> + dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
> + return id;
> + }
> + snprintf(dev->name, sizeof(dev->name), "eth%d", id);
> +
>
> These three drivers seem to have something similar for mdio/phy bus IDs:
> drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
> of_alias_get_id(dn, "eth");
> drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
> of_alias_get_id(np, "ethernet");
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
> plat->bus_id = of_alias_get_id(np, "ethernet");
>
> Drawback: This approach will break if alias is not provided for one
> of the interfaces on board. Not to mention, there could be systems
> with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
> If one of the drivers does not have an alias read mechanism, it is
> possible to have clashing ID assignments. Is there any way this
> solution can be changed to be stable/acceptable?
>
> One other alternative I've tried is netdev kernel bootargs but this
> device name was not being picked by the kernel:
> https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
> netdev= <mac1’s interrupt id>, <mac1’s base address>, eth0
> netdev=<mac0’s interrupt id>, <mac0’s base address>, eth1
>
> Could you please suggest any alternatives?
> Thanks!
>
> Regards,
> Harini
Device naming is a hard problem, and there is no perfect solution.
Device tree should be providing hints to userspace policy for naming, not
trying to do it in the kernel.
See: https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
^ permalink raw reply
* Re: [PATCH -next] appletalk: use remove_proc_subtree to simplify procfs code
From: kbuild test robot @ 2019-02-27 18:32 UTC (permalink / raw)
To: Yue Haibing
Cc: kbuild-all, davem, joe, gregkh, linux-kernel, netdev, YueHaibing
In-Reply-To: <20190227150023.28420-1-yuehaibing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]
Hi YueHaibing,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20190227]
url: https://github.com/0day-ci/linux/commits/Yue-Haibing/appletalk-use-remove_proc_subtree-to-simplify-procfs-code/20190228-014856
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
net//appletalk/atalk_proc.c: In function 'atalk_proc_init':
>> net//appletalk/atalk_proc.c:218:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!proc_create_seq("atalk/interface", 0444, init_net.proc_net,
^~
net//appletalk/atalk_proc.c:220:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
goto out;
^~~~
net//appletalk/atalk_proc.c:222:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!proc_create_seq("atalk/route", 0444, init_net.proc_net,
^~
net//appletalk/atalk_proc.c:224:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
goto out;
^~~~
net//appletalk/atalk_proc.c:226:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!proc_create_seq("atalk/socket", 0444, init_net.proc_net,
^~
net//appletalk/atalk_proc.c:228:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
goto out;
^~~~
net//appletalk/atalk_proc.c:230:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!proc_create_seq_private("atalk/arp", 0444, init_net.proc_net,
^~
net//appletalk/atalk_proc.c:233:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
goto out;
^~~~
vim +/if +218 net//appletalk/atalk_proc.c
212
213 int __init atalk_proc_init(void)
214 {
215 if (!proc_mkdir("atalk", init_net.proc_net))
216 return -ENOMEM;
217
> 218 if (!proc_create_seq("atalk/interface", 0444, init_net.proc_net,
219 &atalk_seq_interface_ops));
220 goto out;
221
222 if (!proc_create_seq("atalk/route", 0444, init_net.proc_net,
223 &atalk_seq_route_ops));
224 goto out;
225
226 if (!proc_create_seq("atalk/socket", 0444, init_net.proc_net,
227 &atalk_seq_socket_ops));
228 goto out;
229
230 if (!proc_create_seq_private("atalk/arp", 0444, init_net.proc_net,
231 &aarp_seq_ops,
232 sizeof(struct aarp_iter_state), NULL));
233 goto out;
234
235 out:
236 remove_proc_subtree("atalk", init_net.proc_net);
237 return -ENOMEM;
238 }
239
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56809 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports
From: Jakub Kicinski @ 2019-02-27 18:30 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, oss-drivers, netdev, parav, jgg
In-Reply-To: <20190227123753.GB2240@nanopsycho>
On Wed, 27 Feb 2019 13:37:53 +0100, Jiri Pirko wrote:
> Tue, Feb 26, 2019 at 07:24:32PM CET, jakub.kicinski@netronome.com wrote:
> >PCI endpoint corresponds to a PCI device, but such device
> >can have one more more logical device ports associated with it.
> >We need a way to distinguish those. Add a PCI subport in the
> >dumps and print the info in phys_port_name appropriately.
> >
> >This is not equivalent to port splitting, there is no split
> >group. It's just a way of representing multiple netdevs on
> >a single PCI function.
> >
> >Note that the quality of being multiport pertains only to
> >the PCI function itself. A PF having multiple netdevs does
> >not mean that its VFs will also have multiple, or that VFs
> >are associated with any particular port of a multiport VF.
>
> We've been discussing the problem of subport (we call it "subfunction"
> or "SF") for some time internally. Turned out, this is probably harder
> task to model. Please prove me wrong.
>
> The nature of VF makes it a logically separate entity. It has a separate
> PCI address, it should therefore have a separate devlink instance.
> You can pass it through to VM, then the same devlink instance should be
> created inside the VM and disappear from the host.
Depends what a devlink instance represents :/ On one hand you may want
to create an instance for a VF to allow it to spawn soft ports, on the
other you may want to group multiple functions together.
IOW if devlink instance is for an ASIC, there should be one per device
per host. So if we start connecting multiple functions (PFs and/or VFs)
to one host we should probably introduce the notion of devlink aliases
or some such (so that multiple bus addresses can target the same
devlink instance). Those less pipelined NICs can forward between
ports, but still want a function per port (otherwise user space
sometimes gets confused). If we have multiple functions which are on
the same "switchid" they should have a single devlink instance if you
ask me. That instance will have all the ports of the device.
You say disappear from the host - what do you mean. Are you referring
to the VF port disappearing? But on the switch the port is still
there, and you should show the subports on the PF side IMHO. Devlink
ports should allow users to understand the topology of the switch.
Is spawning VMDq sub-instances the only thing we can think of that VMs
may want to do? Are there any other uses?
> SF (or subport) feels similar to that. Basically it is exactly the same
> thing as VF, only does reside under PF PCI function.
>
> That is why I think, for sake of consistency, it should have a separate
> devlink entity as well. The problem is correct sysfs modelling and
> devlink handle derived from that. Parav is working on a simple soft
> bus for this purpose called "subbus". There is a RFC floating around on
> Mellanox internal mailing list, looks like it is time to send it
> upstream.
>
> Then each PF driver which have SFs would register subbus devices
> according to SFs/subports and they would be properly handled by bus
> probe, devlink and devlink port and netdev instances created.
>
> Ccing Parav and Jason.
You guys come from the RDMA side of the world, with which I'm less
familiar, and the soft bus + spawning devices seems to be a popular
design there. Could you describe the advantages of that model for
the sake of the netdev-only folks? :)
Another term that gets thrown into the mix here is mediated devices,
right? If you wanna pass the sub-spawn-soft-port to a VM. Or run
DPDK on some queues.
To state the obvious AF_XDP and macvlan offload were are previous
answers to some of those use cases. What is the forwarding model
for those subports? Are we going to allow flower rules from VMs?
Is it going to be dst MAC only? Or is the hypervisor going to forward
as it sees appropriate (OvS + "repr"/port netdev)?
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: add missing entries to bpf_helpers.h
From: Song Liu @ 2019-02-27 18:28 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Networking, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Stanislav Fomichev, Willem de Bruijn
In-Reply-To: <20190227160806.195218-1-willemdebruijn.kernel@gmail.com>
On Wed, Feb 27, 2019 at 8:09 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> This header defines the BPF functions enumerated in uapi/linux.bpf.h
> in a callable format. Expand to include all registered functions.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
>
> ---
>
> changes pre->v1:
> - add brief commit message
> (not copying Acks as they do not necessarily agree with that)
>
> - change return type from ull to uint for
> bpf_get_cgroup_classid, bpf_get_route_realm, bpf_get_hash_recalc
>
> - change size arg from int to __u32 for bpf_probe_read_str
>
> ---
> tools/testing/selftests/bpf/bpf_helpers.h | 30 +++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index d9999f1ed1d2a..026bea831e039 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -232,6 +232,36 @@ static int (*bpf_skb_change_head)(void *, int len, int flags) =
> (void *) BPF_FUNC_skb_change_head;
> static int (*bpf_skb_pull_data)(void *, int len) =
> (void *) BPF_FUNC_skb_pull_data;
> +static unsigned int (*bpf_get_cgroup_classid)(void *ctx) =
> + (void *) BPF_FUNC_get_cgroup_classid;
> +static unsigned int (*bpf_get_route_realm)(void *ctx) =
> + (void *) BPF_FUNC_get_route_realm;
> +static int (*bpf_skb_change_proto)(void *ctx, __be16 proto, __u64 flags) =
> + (void *) BPF_FUNC_skb_change_proto;
> +static int (*bpf_skb_change_type)(void *ctx, __u32 type) =
> + (void *) BPF_FUNC_skb_change_type;
> +static unsigned int (*bpf_get_hash_recalc)(void *ctx) =
> + (void *) BPF_FUNC_get_hash_recalc;
> +static unsigned long long (*bpf_get_current_task)(void *ctx) =
> + (void *) BPF_FUNC_get_current_task;
> +static int (*bpf_skb_change_tail)(void *ctx, __u32 len, __u64 flags) =
> + (void *) BPF_FUNC_skb_change_tail;
> +static long long (*bpf_csum_update)(void *ctx, __u32 csum) =
> + (void *) BPF_FUNC_csum_update;
> +static void (*bpf_set_hash_invalid)(void *ctx) =
> + (void *) BPF_FUNC_set_hash_invalid;
> +static int (*bpf_get_numa_node_id)(void) =
> + (void *) BPF_FUNC_get_numa_node_id;
> +static int (*bpf_probe_read_str)(void *ctx, __u32 size,
> + const void *unsafe_ptr) =
> + (void *) BPF_FUNC_probe_read_str;
> +static unsigned int (*bpf_get_socket_uid)(void *ctx) =
> + (void *) BPF_FUNC_get_socket_uid;
> +static unsigned int (*bpf_set_hash)(void *ctx, __u32 hash) =
> + (void *) BPF_FUNC_set_hash;
> +static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> + unsigned long long flags) =
> + (void *) BPF_FUNC_skb_adjust_room;
>
> /* Scan the ARCH passed in from ARCH env variable (see Makefile) */
> #if defined(__TARGET_ARCH_x86)
> --
> 2.21.0.rc2.261.ga7da99ff1b-goog
>
^ permalink raw reply
* [PATCH] netfilter: xt_IDLETIMER: fix sysfs callback function type
From: Sami Tolvanen @ 2019-02-27 18:19 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: David S . Miller, netfilter-devel, coreteam, netdev, linux-kernel,
Sami Tolvanen
Use struct device_attribute instead of struct idletimer_tg_attr, and
the correct callback function type to avoid indirect call mismatches
with Control Flow Integrity checking.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
net/netfilter/xt_IDLETIMER.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
index eb4cbd244c3d..5f9b37e12801 100644
--- a/net/netfilter/xt_IDLETIMER.c
+++ b/net/netfilter/xt_IDLETIMER.c
@@ -41,19 +41,13 @@
#include <linux/workqueue.h>
#include <linux/sysfs.h>
-struct idletimer_tg_attr {
- struct attribute attr;
- ssize_t (*show)(struct kobject *kobj,
- struct attribute *attr, char *buf);
-};
-
struct idletimer_tg {
struct list_head entry;
struct timer_list timer;
struct work_struct work;
struct kobject *kobj;
- struct idletimer_tg_attr attr;
+ struct device_attribute attr;
unsigned int refcnt;
};
@@ -76,15 +70,15 @@ struct idletimer_tg *__idletimer_tg_find_by_label(const char *label)
return NULL;
}
-static ssize_t idletimer_tg_show(struct kobject *kobj, struct attribute *attr,
- char *buf)
+static ssize_t idletimer_tg_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct idletimer_tg *timer;
unsigned long expires = 0;
mutex_lock(&list_mutex);
- timer = __idletimer_tg_find_by_label(attr->name);
+ timer = __idletimer_tg_find_by_label(attr->attr.name);
if (timer)
expires = timer->timer.expires;
--
2.21.0.rc2.261.ga7da99ff1b-goog
^ permalink raw reply related
* Re: [PATCH] appletalk: Fix use-after-free in atalk_proc_exit
From: David Miller @ 2019-02-27 18:14 UTC (permalink / raw)
To: yuehaibing; +Cc: joe, gregkh, linux-kernel, netdev
In-Reply-To: <20190227145900.28808-1-yuehaibing@huawei.com>
From: Yue Haibing <yuehaibing@huawei.com>
Date: Wed, 27 Feb 2019 22:59:00 +0800
> @@ -1925,7 +1925,10 @@ static int __init atalk_init(void)
>
> register_netdevice_notifier(&ddp_notifier);
> aarp_proto_init();
> - atalk_proc_init();
> + rc = atalk_proc_init();
> + if (rc)
> + goto out;
> +
> atalk_register_sysctl();
> out:
This leaves the netdevice notifier registered etc. You need to add a proper
error cleanup code path.
^ permalink raw reply
* Re: [PATCH net-next] Revert "net: sched: route: don't set arg->stop in route4_walk() when empty"
From: David Miller @ 2019-02-27 18:12 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190227134859.18424-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Wed, 27 Feb 2019 15:48:59 +0200
> This reverts commit 3027ff41f67c ("net: sched: route: don't set arg->stop
> in route4_walk() when empty")
>
> Cls API function tcf_proto_is_empty() was changed in commit
> 6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
> to no longer depend on arg->stop to determine that classifier instance is
> empty. Instead, it adds dedicated arg->nonempty field, which makes the fix
> in route classifier no longer necessary.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Looks good, applied, thanks Vlad.
^ permalink raw reply
* Re: [PATCH][net-next] ethtool: Use explicit designated initializers for .cmd
From: David Miller @ 2019-02-27 18:08 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
In-Reply-To: <1551271677-14492-1-git-send-email-lirongqing@baidu.com>
From: Li RongQing <lirongqing@baidu.com>
Date: Wed, 27 Feb 2019 20:47:57 +0800
> Initialize the .cmd member by using a designated struct
> initializer. This fixes warning of missing field initializers,
> and makes code a little easier to read.
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Applied.
^ permalink raw reply
* Re: [PATCH v4] net: phy: Micrel KSZ8061: link failure after cable connect
From: David Miller @ 2019-02-27 18:05 UTC (permalink / raw)
To: T.Rajasingh
Cc: alexander.onnasch, andrew, f.fainelli, hkallweit1, netdev,
linux-kernel
In-Reply-To: <20190227121319.4106-1-T.Rajasingh@landisgyr.com>
From: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
Date: Wed, 27 Feb 2019 17:43:19 +0530
> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf descripes the problem and possible workarounds in
> detail, see below.
> The batch implements workaround 1, which permanently fixes the issue.
...
> Fixes: 7ab59dc15e2f ("drivers/net/phy/micrel_phy: Add support for new PHYs")
> Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
> Signed-off-by: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v4 perf,bpf 14/15] perf: introduce side band thread
From: Song Liu @ 2019-02-27 17:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
Kernel Team, peterz@infradead.org, acme@redhat.com,
jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <20190227132105.GC18893@krava>
> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Feb 25, 2019 at 04:20:18PM -0800, Song Liu wrote:
>
> SNIP
>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 8c902276d4b4..61b87c8111e6 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -19,6 +19,7 @@
>> #include "debug.h"
>> #include "units.h"
>> #include "asm/bug.h"
>> +#include "bpf-event.h"
>> #include <signal.h>
>> #include <unistd.h>
>>
>> @@ -1841,3 +1842,102 @@ struct perf_evsel *perf_evlist__reset_weak_group(struct perf_evlist *evsel_list,
>> }
>> return leader;
>> }
>> +
>> +static struct perf_evlist *sb_evlist;
>> +pthread_t poll_thread;
>
> so some of the things are static and some like poll_args
> you alloced on the stack.. I dont like this interface,
> could we come up with something generic? perhaps
> encapsulated in perf_evlist, like:
I picked global sb_evlist and poll_thread because there
should be only one sb_evlist and one thread polling it.
There might be multiple evsel on the sb_evlist.
I am not sure I understand your suggestion..
> struct perf_evlist {
> ...
> struct {
> pthread_t th;
> int state;
> } thread;
This will not be used by the main perf_evlist, right?
> };
>
> typedef int (perf_evlist__thread_cb_t)(perf_evlist, union perf_event *event,....)
How do we use this callback? We need a way to specify which
events to poll (mmap, bpf_event, ksymbol, etc). How do we do
that with the callback function?
Thanks,
Song
> perf_evlist__start_thread(perf_evlist, perf_evlist__thread_cb_t cb);
> perf_evlist__stop_thread(perf_evlist);
>
>
> jirka
^ permalink raw reply
* [net: PATCH] net: mvpp2: disable link IRQ waiting for IDLE frames
From: Marcin Wojtas @ 2019-02-27 17:47 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, netdev
Cc: davem, linux, antoine.tenart, thomas.petazzoni, gregory.clement,
stefanc, nadavh, mw, jaz, tn
Current version of the driver was configuring XLG MAC
in a way to wait 3 IDLE frames before allowing for the
link-up interrupt to be triggered. This resulted in an
issue, preventing to detect the link change during RX
traffic on the interface. Fix that.
Fixes: 4bb043262878 ("net: mvpp2: phylink support")
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 16066c2..f1378f9 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4532,8 +4532,7 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
ctrl0 |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
ctrl4 &= ~MVPP22_XLG_CTRL4_MACMODSELECT_GMAC;
- ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC |
- MVPP22_XLG_CTRL4_EN_IDLE_CHECK;
+ ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC;
writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4 perf,bpf 15/15] perf, bpf: save information about short living bpf programs
From: Song Liu @ 2019-02-27 17:42 UTC (permalink / raw)
To: Jiri Olsa
Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
Kernel Team, peterz@infradead.org, acme@redhat.com,
jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <20190227132116.GD18893@krava>
> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Feb 25, 2019 at 04:20:19PM -0800, Song Liu wrote:
>
> SNIP
>
>> + btf_id = info_linear->info.btf_id;
>> +
>> + info_node = malloc(sizeof(struct bpf_prog_info_node));
>> + if (info_node) {
>> + info_node->info_linear = info_linear;
>> + perf_env__insert_bpf_prog_info(env, info_node);
Getting bpf_prog_info here.
>> + } else
>> + free(info_linear);
>> +
>> + if (btf_id == 0)
>> + goto out;
>> +
>> + if (btf__get_from_id(btf_id, &btf)) {
>> + pr_debug("%s: failed to get BTF of id %u, aborting\n",
>> + __func__, btf_id);
>> + goto out;
>> + }
>> + perf_env__fetch_btf(env, btf_id, btf);
>
> so is this the main reason we are doing this? getting the btf
> data for bpf prog ids and store them?
We are getting both bpf_prog_info (see above) and btf.
>
> please describe the whole bpf events/features data flow in
> changelog as I asked in previous email
I will add more details to the change log.
Thanks,
Song
>
> thanks,
> jirka
^ permalink raw reply
* [PATCH net 16/16] net/sched: act_vlan: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action vlan pop pass index 90
# tc actions replace action vlan \
> pop goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action vlan
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: vlan pop goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000007974f067 P4D 800000007974f067 PUD 79638067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffff982dfdb83be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff982dfc55db00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff982df97099c0 RDI: ffff982dfc55db00
RBP: ffff982dfdb83c80 R08: ffff982df983fec8 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff982df5aacd00
R13: ffff982df5aacd08 R14: 0000000000000001 R15: ffff982df97099c0
FS: 0000000000000000(0000) GS:ffff982dfdb80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000796d0005 CR4: 00000000001606e0
Call Trace:
<IRQ>
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ip6_finish_output2+0x369/0x590
ip6_finish_output2+0x369/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.35+0x79/0xc0
mld_sendpack+0x16f/0x220
mld_ifc_timer_expire+0x195/0x2c0
? igmp6_timer_handler+0x70/0x70
call_timer_fn+0x2b/0x130
run_timer_softirq+0x3e8/0x440
? enqueue_hrtimer+0x39/0x90
__do_softirq+0xe3/0x2f5
irq_exit+0xf0/0x100
smp_apic_timer_interrupt+0x6c/0x130
apic_timer_interrupt+0xf/0x20
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10
Code: 7b ff ff ff 7f f3 c3 65 48 8b 04 25 00 5c 01 00 f0 80 48 02 20 48 8b 00 a8 08 74 8b eb c1 90 90 90 90 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffa4714038feb8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffff840184f0 RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000001e57d3f387
RBP: 0000000000000003 R08: 001125d9ca39e1eb R09: 0000000000000000
R10: 000000000000027d R11: 000000000009f400 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
? __sched_text_end+0x1/0x1
default_idle+0x1c/0x140
do_idle+0x1c4/0x280
cpu_startup_entry+0x19/0x20
start_secondary+0x1a7/0x200
secondary_startup_64+0xa4/0xb0
Modules linked in: act_vlan veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 snd_hda_codec_generic mbcache crct10dif_pclmul jbd2 snd_hda_intel crc32_pclmul snd_hda_codec ghash_clmulni_intel snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd glue_helper joydev snd_timer virtio_balloon snd pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt virtio_net fb_sys_fops virtio_blk ttm net_failover virtio_console failover ata_piix drm libata crc32c_intel virtio_pci serio_raw virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_vlan_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_vlan.c | 21 ++++++++++++++--
.../tc-testing/tc-tests/actions/vlan.json | 25 +++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 80fd0e238a10..b59e138100dd 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -15,6 +15,7 @@
#include <linux/if_vlan.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_vlan.h>
#include <net/tc_act/tc_vlan.h>
@@ -108,6 +109,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_VLAN_MAX + 1];
struct tcf_vlan_params *p;
struct tc_vlan *parm;
@@ -200,12 +202,16 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
+
v = to_vlan(*a);
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p) {
- tcf_idr_release(*a, bind);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto put_chain;
}
p->tcfv_action = action;
@@ -214,6 +220,8 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
p->tcfv_push_proto = push_proto;
spin_lock_bh(&v->tcf_lock);
+ oldchain = v->tcf_goto_chain;
+ v->tcf_goto_chain = newchain;
v->tcf_action = parm->action;
rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
spin_unlock_bh(&v->tcf_lock);
@@ -223,7 +231,16 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static void tcf_vlan_cleanup(struct tc_action *a)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
index 69ea09eefffc..cc7c7d758008 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
@@ -688,5 +688,30 @@
"teardown": [
"$TC actions flush action vlan"
]
+ },
+ {
+ "id": "e394",
+ "name": "Replace vlan push action with invalid goto chain control",
+ "category": [
+ "actions",
+ "vlan"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action vlan",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action vlan push id 500 pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action vlan push id 500 goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action vlan index 90",
+ "matchPattern": "action order [0-9]+: vlan.*push id 500 protocol 802.1Q priority 0 pass.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action vlan"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 15/16] net/sched: act_tunnel_key: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action tunnel_key set src_ip 10.10.10.1 dst_ip 20.20.2 dst_port 3128 \
> nocsum id 1 pass index 90
# tc actions replace action tunnel_key \
> set src_ip 10.10.10.1 dst_ip 20.20.2 dst_port 3128 nocsum id 1 \
> goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action tunnel_key
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: tunnel_key set
src_ip 10.10.10.1
dst_ip 20.20.2.0
key_id 1
dst_port 3128
nocsum goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000002aba4067 P4D 800000002aba4067 PUD 795f9067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffff9346bdb83be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9346bb795c00 RCX: 0000000000000002
RDX: 0000000000000000 RSI: ffff93466c881700 RDI: 0000000000000246
RBP: ffff9346bdb83c80 R08: ffff9346b3e1e0c8 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff9346b978f000
R13: ffff9346b978f008 R14: 0000000000000001 R15: ffff93466dceeb40
FS: 0000000000000000(0000) GS:ffff9346bdb80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007a6c2002 CR4: 00000000001606e0
Call Trace:
<IRQ>
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ip6_finish_output2+0x369/0x590
ip6_finish_output2+0x369/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.35+0x79/0xc0
mld_sendpack+0x16f/0x220
mld_ifc_timer_expire+0x195/0x2c0
? igmp6_timer_handler+0x70/0x70
call_timer_fn+0x2b/0x130
run_timer_softirq+0x3e8/0x440
? tick_sched_timer+0x37/0x70
__do_softirq+0xe3/0x2f5
irq_exit+0xf0/0x100
smp_apic_timer_interrupt+0x6c/0x130
apic_timer_interrupt+0xf/0x20
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10
Code: 55 ff ff ff 7f f3 c3 65 48 8b 04 25 00 5c 01 00 f0 80 48 02 20 48 8b 00 a8 08 74 8b eb c1 90 90 90 90 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffa48a8038feb8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffffaa8184f0 RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000087 RDI: 0000000000000003
RBP: 0000000000000003 R08: 0011251c6fcfac49 R09: ffff9346b995be00
R10: ffffa48a805e7ce8 R11: 00000000024c38dd R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
? __sched_text_end+0x1/0x1
default_idle+0x1c/0x140
do_idle+0x1c4/0x280
cpu_startup_entry+0x19/0x20
start_secondary+0x1a7/0x200
secondary_startup_64+0xa4/0xb0
Modules linked in: act_tunnel_key veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul crc32_pclmul snd_hda_codec_generic ghash_clmulni_intel mbcache snd_hda_intel jbd2 snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd glue_helper joydev snd_timer snd pcspkr virtio_balloon soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect virtio_net sysimgblt fb_sys_fops ttm net_failover virtio_console virtio_blk failover drm serio_raw crc32c_intel ata_piix virtio_pci floppy virtio_ring libata virtio dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_tunnel_key_init() proved to fix
the above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_tunnel_key.c | 18 ++++++++++++-
.../tc-tests/actions/tunnel_key.json | 25 +++++++++++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index eaa4a9c80898..dfc932fd0ff7 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -17,6 +17,7 @@
#include <net/netlink.h>
#include <net/pkt_sched.h>
#include <net/dst.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_tunnel_key.h>
#include <net/tc_act/tc_tunnel_key.h>
@@ -213,6 +214,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_TUNNEL_KEY_MAX + 1];
struct tcf_tunnel_key_params *params_new;
struct metadata_dst *metadata = NULL;
@@ -353,6 +355,12 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
goto release_tun_meta;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err) {
+ ret = err;
+ exists = true;
+ goto release_tun_meta;
+ }
t = to_tunnel_key(*a);
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
@@ -360,12 +368,14 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
NL_SET_ERR_MSG(extack, "Cannot allocate tunnel key parameters");
ret = -ENOMEM;
exists = true;
- goto release_tun_meta;
+ goto put_chain;
}
params_new->tcft_action = parm->t_action;
params_new->tcft_enc_metadata = metadata;
spin_lock_bh(&t->tcf_lock);
+ oldchain = t->tcf_goto_chain;
+ t->tcf_goto_chain = newchain;
t->tcf_action = parm->action;
rcu_swap_protected(t->params, params_new,
lockdep_is_held(&t->tcf_lock));
@@ -374,9 +384,15 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+
release_tun_meta:
if (metadata)
dst_release(&metadata->dst);
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
index e7e15a7336b6..28453a445fdb 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
@@ -884,5 +884,30 @@
"teardown": [
"$TC actions flush action tunnel_key"
]
+ },
+ {
+ "id": "8242",
+ "name": "Replace tunnel_key set action with invalid goto chain",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action tunnel_key set src_ip 10.10.10.1 dst_ip 20.20.20.2 dst_port 3128 nocsum id 1 pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action tunnel_key set src_ip 10.10.10.2 dst_ip 20.20.20.1 dst_port 3129 id 2 csum goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 90",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 10.10.10.1.*dst_ip 20.20.20.2.*key_id 1.*dst_port 3128.*csum pass.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 14/16] net/sched: act_skbmod: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action skbmod set smac 00:c1:a0:c1:a0:00 pass index 90
# tc actions replace action skbmod \
> set smac 00:c1:a0:c1:a0:00 goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action skbmod
had the following output:
src MAC address <00:c1:a0:c1:a0:00>
src MAC address <00:c1:a0:c1:a0:00>
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: skbmod goto chain 42 set smac 00:c1:a0:c1:a0:00
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000002d5c7067 P4D 800000002d5c7067 PUD 77e16067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffff8987ffd83be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff8987aeb68800 RCX: ffff8987fa263640
RDX: 0000000000000000 RSI: ffff8987f51c8802 RDI: 00000000000000a0
RBP: ffff8987ffd83c80 R08: ffff8987f939bac8 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8987f5c77d00
R13: ffff8987f5c77d08 R14: 0000000000000001 R15: ffff8987f0c29f00
FS: 0000000000000000(0000) GS:ffff8987ffd80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007832c004 CR4: 00000000001606e0
Call Trace:
<IRQ>
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ip6_finish_output2+0x369/0x590
ip6_finish_output2+0x369/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.35+0x79/0xc0
mld_sendpack+0x16f/0x220
mld_ifc_timer_expire+0x195/0x2c0
? igmp6_timer_handler+0x70/0x70
call_timer_fn+0x2b/0x130
run_timer_softirq+0x3e8/0x440
? tick_sched_timer+0x37/0x70
__do_softirq+0xe3/0x2f5
irq_exit+0xf0/0x100
smp_apic_timer_interrupt+0x6c/0x130
apic_timer_interrupt+0xf/0x20
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10
Code: 56 ff ff ff 7f f3 c3 65 48 8b 04 25 00 5c 01 00 f0 80 48 02 20 48 8b 00 a8 08 74 8b eb c1 90 90 90 90 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffa2a1c038feb8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffffa94184f0 RBX: 0000000000000003 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000087 RDI: 0000000000000003
RBP: 0000000000000003 R08: 001123cfc2ba71ac R09: 0000000000000000
R10: 0000000000000000 R11: 00000000000f4240 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
? __sched_text_end+0x1/0x1
default_idle+0x1c/0x140
do_idle+0x1c4/0x280
cpu_startup_entry+0x19/0x20
start_secondary+0x1a7/0x200
secondary_startup_64+0xa4/0xb0
Modules linked in: act_skbmod veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel mbcache jbd2 snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device aesni_intel crypto_simd cryptd glue_helper snd_pcm joydev pcspkr virtio_balloon snd_timer snd i2c_piix4 soundcore nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect virtio_net sysimgblt fb_sys_fops net_failover virtio_console ttm virtio_blk failover drm crc32c_intel serio_raw ata_piix virtio_pci libata virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_skbmod_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_skbmod.c | 26 +++++++++++++++----
.../tc-testing/tc-tests/actions/skbmod.json | 25 ++++++++++++++++++
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 35572d0e4576..dbba5ed873c2 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -16,6 +16,7 @@
#include <linux/rtnetlink.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_skbmod.h>
#include <net/tc_act/tc_skbmod.h>
@@ -86,6 +87,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_SKBMOD_MAX + 1];
struct tcf_skbmod_params *p, *p_old;
struct tc_skbmod *parm;
@@ -154,20 +156,25 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
d = to_skbmod(*a);
p = kzalloc(sizeof(struct tcf_skbmod_params), GFP_KERNEL);
if (unlikely(!p)) {
- tcf_idr_release(*a, bind);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto put_chain;
}
- p->flags = lflags;
- d->tcf_action = parm->action;
-
if (ovr)
spin_lock_bh(&d->tcf_lock);
+
+ p->flags = lflags;
+ oldchain = d->tcf_goto_chain;
+ d->tcf_goto_chain = newchain;
+ d->tcf_action = parm->action;
/* Protected by tcf_lock if overwriting existing action. */
p_old = rcu_dereference_protected(d->skbmod_p, 1);
@@ -187,7 +194,16 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static void tcf_skbmod_cleanup(struct tc_action *a)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
index fe3326e939c1..6eb4c4f97060 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
@@ -392,5 +392,30 @@
"teardown": [
"$TC actions flush action skbmod"
]
+ },
+ {
+ "id": "b651",
+ "name": "Replace skbmod action with invalid goto_chain control",
+ "category": [
+ "actions",
+ "skbmod"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action skbmod",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action skbmod set etype 0x1111 pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action skbmod set etype 0x1111 goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions ls action skbmod",
+ "matchPattern": "action order [0-9]*: skbmod pass set etype 0x1111\\s+index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action skbmod"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 13/16] net/sched: act_skbedit: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action skbedit ptype host pass index 90
# tc actions replace action skbedit \
> ptype host goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action skbedit
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: skbedit ptype host goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 3467 Comm: kworker/3:3 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffb50a81e1fad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9aa47ba4ea00 RCX: 0000000000000001
RDX: 0000000000000000 RSI: ffff9aa469eeb3c0 RDI: ffff9aa47ba4ea00
RBP: ffffb50a81e1fb70 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: ffff9aa47bce0638 R12: ffff9aa4793b0c00
R13: ffff9aa4793b0c08 R14: 0000000000000001 R15: ffff9aa469eeb3c0
FS: 0000000000000000(0000) GS:ffff9aa474780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007360e005 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_skbedit veth ip6table_filter ip6_tables iptable_filter binfmt_misc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ext4 snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep mbcache snd_hda_core jbd2 snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd snd_timer glue_helper snd joydev soundcore pcspkr virtio_balloon i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm virtio_net net_failover drm failover virtio_blk virtio_console ata_piix virtio_pci crc32c_intel serio_raw libata virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_skbedit_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_skbedit.c | 22 +++++++++++++---
.../tc-testing/tc-tests/actions/skbedit.json | 25 +++++++++++++++++++
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 9a8a0f2d4418..5a0d35b2dac3 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -26,6 +26,7 @@
#include <net/ip.h>
#include <net/ipv6.h>
#include <net/dsfield.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_skbedit.h>
#include <net/tc_act/tc_skbedit.h>
@@ -100,6 +101,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct tcf_skbedit_params *params_new;
struct nlattr *tb[TCA_SKBEDIT_MAX + 1];
struct tc_skbedit *parm;
@@ -187,12 +189,13 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
return -EEXIST;
}
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
- if (unlikely(!params_new)) {
- tcf_idr_release(*a, bind);
- return -ENOMEM;
- }
+ if (unlikely(!params_new))
+ goto put_chain;
params_new->flags = flags;
if (flags & SKBEDIT_F_PRIORITY)
@@ -209,6 +212,8 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
params_new->mask = *mask;
spin_lock_bh(&d->tcf_lock);
+ oldchain = d->tcf_goto_chain;
+ d->tcf_goto_chain = newchain;
d->tcf_action = parm->action;
rcu_swap_protected(d->params, params_new,
lockdep_is_held(&d->tcf_lock));
@@ -218,7 +223,16 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
index 5aaf593b914a..ecd96eda7f6a 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
@@ -484,5 +484,30 @@
"teardown": [
"$TC actions flush action skbedit"
]
+ },
+ {
+ "id": "1b2b",
+ "name": "Replace skbedit action with invalid goto_chain control",
+ "category": [
+ "actions",
+ "skbedit"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action skbedit",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action skbedit ptype host pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action skbedit ptype host goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action skbedit",
+ "matchPattern": "action order [0-9]*: skbedit ptype host pass.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action skbedit"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 12/16] net/sched: act_simple: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action simple sdata hello pass index 90
# tc actions replace action simple \
> sdata world goto chain 42 index 90 cookie c1a0c1a0
# tc action show action simple
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: Simple <world>
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000006a6fb067 P4D 800000006a6fb067 PUD 6aed6067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 2 PID: 3241 Comm: kworker/2:0 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffbe6781763ad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9e59bdb80e00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff9e59b4716738 RDI: ffff9e59ab12d140
RBP: ffffbe6781763b70 R08: 0000000000000234 R09: 0000000000aaaaaa
R10: 0000000000000000 R11: ffff9e59b247cd50 R12: ffff9e59b112f100
R13: ffff9e59b112f108 R14: 0000000000000001 R15: ffff9e59ab12d0c0
FS: 0000000000000000(0000) GS:ffff9e59b4700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000006af92004 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_simple veth ip6table_filter ip6_tables iptable_filter binfmt_misc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ext4 snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep mbcache snd_hda_core jbd2 snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd snd_timer glue_helper snd joydev virtio_balloon pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops virtio_net ttm net_failover virtio_console virtio_blk failover drm crc32c_intel serio_raw floppy ata_piix libata virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_simple_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_simple.c | 50 +++++++++++++++----
.../tc-testing/tc-tests/actions/simple.json | 25 ++++++++++
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 640ee5b785dc..ca8ef8378c33 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -18,6 +18,7 @@
#include <linux/rtnetlink.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#define TCA_ACT_SIMP 22
@@ -62,14 +63,28 @@ static int alloc_defdata(struct tcf_defact *d, const struct nlattr *defdata)
return 0;
}
-static void reset_policy(struct tcf_defact *d, const struct nlattr *defdata,
- struct tc_defact *p)
+static int reset_policy(struct tcf_defact *d, const struct nlattr *defdata,
+ struct tc_defact *p, struct tcf_proto *tp,
+ struct netlink_ext_ack *extack)
{
+ struct tcf_chain *newchain = NULL, *oldchain;
+ int err;
+
+ err = tcf_action_check_ctrlact(p->action, tp, &newchain, extack);
+ if (err)
+ return err;
+
spin_lock_bh(&d->tcf_lock);
+ oldchain = d->tcf_goto_chain;
+ d->tcf_goto_chain = newchain;
d->tcf_action = p->action;
memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
spin_unlock_bh(&d->tcf_lock);
+
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
+ return err;
}
static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = {
@@ -83,6 +98,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_DEF_MAX + 1];
struct tc_defact *parm;
struct tcf_defact *d;
@@ -124,27 +140,41 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
}
d = to_defact(*a);
- ret = alloc_defdata(d, tb[TCA_DEF_DATA]);
- if (ret < 0) {
- tcf_idr_release(*a, bind);
- return ret;
- }
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain,
+ extack);
+ if (err < 0)
+ goto release_idr;
+
+ err = alloc_defdata(d, tb[TCA_DEF_DATA]);
+ if (err < 0)
+ goto release_idr;
+
+ oldchain = d->tcf_goto_chain;
+ d->tcf_goto_chain = newchain;
d->tcf_action = parm->action;
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
ret = ACT_P_CREATED;
} else {
d = to_defact(*a);
if (!ovr) {
- tcf_idr_release(*a, bind);
- return -EEXIST;
+ err = -EEXIST;
+ goto release_idr;
}
- reset_policy(d, tb[TCA_DEF_DATA], parm);
+ err = reset_policy(d, tb[TCA_DEF_DATA], parm, tp, extack);
+ if (err)
+ goto release_idr;
}
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
+
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/simple.json b/tools/testing/selftests/tc-testing/tc-tests/actions/simple.json
index e89a7aa4012d..8e8c1ae12260 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/simple.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/simple.json
@@ -126,5 +126,30 @@
"teardown": [
""
]
+ },
+ {
+ "id": "b776",
+ "name": "Replace simple action with invalid goto chain control",
+ "category": [
+ "actions",
+ "simple"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action simple",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action simple sdata \"hello\" pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action simple sdata \"world\" goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action simple",
+ "matchPattern": "action order [0-9]*: Simple <hello>.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action simple"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 11/16] net/sched: act_sample: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action sample rate 1024 group 4 pass index 90
# tc actions replace action sample \
> rate 1024 group 4 goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action sample
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: sample rate 1/1024 group 4 goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 8000000079966067 P4D 8000000079966067 PUD 7987b067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.0.0-rc4.gotochain_crash+ #536
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffbee60033fad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff99d7ae6e3b00 RCX: 00000000e555df9b
RDX: 0000000000000000 RSI: 00000000b0352718 RDI: ffff99d7fda1fcf0
RBP: ffffbee60033fb70 R08: 0000000070731ab1 R09: 0000000000000400
R10: 0000000000000000 R11: ffff99d7ac733838 R12: ffff99d7f3c2be00
R13: ffff99d7f3c2be08 R14: 0000000000000001 R15: ffff99d7f3c2b600
FS: 0000000000000000(0000) GS:ffff99d7fda00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000797de006 CR4: 00000000001606f0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_sample psample veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel mbcache jbd2 snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device aesni_intel crypto_simd snd_pcm cryptd glue_helper snd_timer joydev snd pcspkr virtio_balloon i2c_piix4 soundcore nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect virtio_net sysimgblt fb_sys_fops net_failover ttm failover virtio_blk virtio_console drm ata_piix serio_raw crc32c_intel libata virtio_pci virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_sample_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_sample.c | 20 +++++++++++++--
.../tc-testing/tc-tests/actions/sample.json | 25 +++++++++++++++++++
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index b2154edcb535..1effafc7fd0b 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -22,6 +22,7 @@
#include <linux/tc_act/tc_sample.h>
#include <net/tc_act/tc_sample.h>
#include <net/psample.h>
+#include <net/pkt_cls.h>
#include <linux/if_arp.h>
@@ -41,6 +42,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_SAMPLE_MAX + 1];
struct psample_group *psample_group;
struct tc_sample *parm;
@@ -79,17 +81,22 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
psample_group = psample_group_get(net, psample_group_num);
if (!psample_group) {
- tcf_idr_release(*a, bind);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto put_chain;
}
s = to_sample(*a);
spin_lock_bh(&s->tcf_lock);
+ oldchain = s->tcf_goto_chain;
+ s->tcf_goto_chain = newchain;
s->tcf_action = parm->action;
s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
s->psample_group_num = psample_group_num;
@@ -103,7 +110,16 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static void tcf_sample_cleanup(struct tc_action *a)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json
index 3aca33c00039..27f0acaed880 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json
@@ -584,5 +584,30 @@
"teardown": [
"$TC actions flush action sample"
]
+ },
+ {
+ "id": "0a6e",
+ "name": "Replace sample action with invalid goto chain control",
+ "category": [
+ "actions",
+ "sample"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action sample",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action sample rate 1024 group 4 pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action sample rate 1024 group 7 goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action sample",
+ "matchPattern": "action order [0-9]+: sample rate 1/1024 group 4 pass.*index 90",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action sample"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 10/16] net/sched: act_police: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action police rate 3mbit burst 250k pass index 90
# tc actions replace action police \
> rate 3mbit burst 250k goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action police rate 3mbit burst
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: police 0x5a rate 3Mbit burst 250Kb mtu 2Kb action goto chain 42 overhead 0b
ref 2 bind 1
cookie c1a0c1a0
Then, when crash0 starts transmitting more than 3Mbit/s, the following
kernel crash is observed:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000007a779067 P4D 800000007a779067 PUD 2ad96067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 5032 Comm: netperf Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffb0e04064fa60 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff93bb3322cce0 RCX: 0000000000000005
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff93bb3322cce0
RBP: ffffb0e04064fb00 R08: 0000000000000022 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000001 R12: ffff93bb3beed300
R13: ffff93bb3beed308 R14: 0000000000000001 R15: ffff93bb3b64d000
FS: 00007f0bc6be5740(0000) GS:ffff93bb3db80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000746a8001 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ipt_do_table+0x31c/0x420 [ip_tables]
? ip_finish_output2+0x16f/0x430
ip_finish_output2+0x16f/0x430
? ip_output+0x69/0xe0
ip_output+0x69/0xe0
? ip_forward_options+0x1a0/0x1a0
__tcp_transmit_skb+0x563/0xa40
tcp_write_xmit+0x243/0xfa0
__tcp_push_pending_frames+0x32/0xf0
tcp_sendmsg_locked+0x404/0xd30
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x36/0x40
__sys_sendto+0x10e/0x140
? __sys_connect+0x87/0xf0
? syscall_trace_enter+0x1df/0x2e0
? __audit_syscall_exit+0x216/0x260
__x64_sys_sendto+0x24/0x30
do_syscall_64+0x5b/0x180
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f0bc5ffbafd
Code: 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 8b 05 ae c4 2c 00 85 c0 75 2d 45 31 c9 45 31 c0 4c 63 d1 48 63 ff b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 63 63 2c 00 f7 d8 64 89 02 48
RSP: 002b:00007fffef94b7f8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000004000 RCX: 00007f0bc5ffbafd
RDX: 0000000000004000 RSI: 00000000017e5420 RDI: 0000000000000004
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000004
R13: 00000000017e51d0 R14: 0000000000000010 R15: 0000000000000006
Modules linked in: act_police veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 snd_hda_codec_generic mbcache crct10dif_pclmul jbd2 crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd glue_helper snd_timer snd joydev pcspkr virtio_balloon soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm virtio_blk virtio_net virtio_console net_failover failover crc32c_intel ata_piix libata serio_raw virtio_pci virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_police_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_police.c | 12 +++++++++
.../tc-testing/tc-tests/actions/police.json | 25 +++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index a444dd78a244..1f7b30b645e7 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -21,6 +21,7 @@
#include <linux/slab.h>
#include <net/act_api.h>
#include <net/netlink.h>
+#include <net/pkt_cls.h>
struct tcf_police_params {
int tcfp_result;
@@ -87,6 +88,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
int ret = 0, tcfp_result = TC_ACT_OK, err, size;
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_POLICE_MAX + 1];
struct tc_police *parm;
struct tcf_police *police;
@@ -129,6 +131,9 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
police = to_police(*a);
if (parm->rate.rate) {
@@ -214,6 +219,8 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
if (new->peak_present)
police->tcfp_ptoks = new->tcfp_mtu_ptoks;
spin_unlock_bh(&police->tcfp_lock);
+ oldchain = police->tcf_goto_chain;
+ police->tcf_goto_chain = newchain;
police->tcf_action = parm->action;
rcu_swap_protected(police->params,
new,
@@ -225,11 +232,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
failure:
qdisc_put_rtab(P_tab);
qdisc_put_rtab(R_tab);
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
tcf_idr_release(*a, bind);
return err;
}
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 4086a50a670e..b8268da5adaa 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -739,5 +739,30 @@
"teardown": [
"$TC actions flush action police"
]
+ },
+ {
+ "id": "689e",
+ "name": "Replace police action with invalid goto chain control",
+ "category": [
+ "actions",
+ "police"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action police",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action police rate 3mbit burst 250k drop index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action police rate 3mbit burst 250k goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action police index 90",
+ "matchPattern": "action order [0-9]*: police 0x5a rate 3Mbit burst 250Kb mtu 2Kb action drop",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action police"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 09/16] net/sched: act_pedit: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc filter add dev crash0 egress matchall \
> action pedit ex munge ip ttl set 10 pass index 90
# tc actions replace action pedit \
> ex munge ip ttl set 10 goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action pedit
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: pedit action goto chain 42 keys 1
index 90 ref 2 bind 1
key #0 at ipv4+8: val 0a000000 mask 00ffffff
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffff94a73db03be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff94a6ee4c0700 RCX: 000000000000000a
RDX: 0000000000000000 RSI: ffff94a6ed22c800 RDI: 0000000000000000
RBP: ffff94a73db03c80 R08: ffff94a7386fa4c8 R09: ffff94a73229ea20
R10: 0000000000000000 R11: 0000000000000000 R12: ffff94a6ed22cb00
R13: ffff94a6ed22cb08 R14: 0000000000000001 R15: ffff94a6ed22c800
FS: 0000000000000000(0000) GS:ffff94a73db00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007120e002 CR4: 00000000001606e0
Call Trace:
<IRQ>
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ip6_finish_output2+0x369/0x590
ip6_finish_output2+0x369/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.35+0x79/0xc0
mld_sendpack+0x16f/0x220
mld_ifc_timer_expire+0x195/0x2c0
? igmp6_timer_handler+0x70/0x70
call_timer_fn+0x2b/0x130
run_timer_softirq+0x3e8/0x440
? tick_sched_timer+0x37/0x70
__do_softirq+0xe3/0x2f5
irq_exit+0xf0/0x100
smp_apic_timer_interrupt+0x6c/0x130
apic_timer_interrupt+0xf/0x20
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10
Code: 4e ff ff ff 7f f3 c3 65 48 8b 04 25 00 5c 01 00 f0 80 48 02 20 48 8b 00 a8 08 74 8b eb c1 90 90 90 90 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffab1740387eb8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffffb18184f0 RBX: 0000000000000002 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000087 RDI: 0000000000000002
RBP: 0000000000000002 R08: 000f168fa695f9a9 R09: 0000000000000020
R10: 0000000000000004 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
? __sched_text_end+0x1/0x1
default_idle+0x1c/0x140
do_idle+0x1c4/0x280
cpu_startup_entry+0x19/0x20
start_secondary+0x1a7/0x200
secondary_startup_64+0xa4/0xb0
Modules linked in: act_pedit veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 mbcache jbd2 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep aesni_intel snd_hda_core crypto_simd snd_seq cryptd glue_helper snd_seq_device snd_pcm joydev snd_timer pcspkr virtio_balloon snd soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs qxl ata_generic pata_acpi drm_kms_helper virtio_net net_failover syscopyarea sysfillrect sysimgblt failover virtio_blk fb_sys_fops virtio_console ttm drm crc32c_intel serio_raw ata_piix virtio_pci libata virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_pedit_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_pedit.c | 17 ++++++-
.../tc-testing/tc-tests/actions/pedit.json | 51 +++++++++++++++++++
2 files changed, 66 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 1c7a0db7b466..1eccd5053e70 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -23,6 +23,7 @@
#include <linux/tc_act/tc_pedit.h>
#include <net/tc_act/tc_pedit.h>
#include <uapi/linux/tc_act/tc_pedit.h>
+#include <net/pkt_cls.h>
static unsigned int pedit_net_id;
static struct tc_action_ops act_pedit_ops;
@@ -141,6 +142,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_PEDIT_MAX + 1];
struct tc_pedit_key *keys = NULL;
struct tcf_pedit_key_ex *keys_ex;
@@ -205,6 +207,11 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
goto out_free;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err) {
+ ret = err;
+ goto out_release;
+ }
p = to_pedit(*a);
spin_lock_bh(&p->tcf_lock);
@@ -214,7 +221,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (!keys) {
spin_unlock_bh(&p->tcf_lock);
ret = -ENOMEM;
- goto out_release;
+ goto put_chain;
}
kfree(p->tcfp_keys);
p->tcfp_keys = keys;
@@ -223,6 +230,8 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
memcpy(p->tcfp_keys, parm->keys, ksize);
p->tcfp_flags = parm->flags;
+ oldchain = p->tcf_goto_chain;
+ p->tcf_goto_chain = newchain;
p->tcf_action = parm->action;
kfree(p->tcfp_keys_ex);
@@ -231,14 +240,18 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
spin_unlock_bh(&p->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
out_release:
tcf_idr_release(*a, bind);
out_free:
kfree(keys_ex);
return ret;
-
}
static void tcf_pedit_cleanup(struct tc_action *a)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json b/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
new file mode 100644
index 000000000000..b73ceb9e28b1
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
@@ -0,0 +1,51 @@
+[
+ {
+ "id": "319a",
+ "name": "Add pedit action that mangles IP TTL",
+ "category": [
+ "actions",
+ "pedit"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action pedit",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action pedit ex munge ip ttl set 10",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions ls action pedit",
+ "matchPattern": "action order [0-9]+: pedit action pass keys 1.*index 1 ref.*key #0 at ipv4\\+8: val 0a000000 mask 00ffffff",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action pedit"
+ ]
+ },
+ {
+ "id": "7e67",
+ "name": "Replace pedit action with invalid goto chain",
+ "category": [
+ "actions",
+ "pedit"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action pedit",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action pedit ex munge ip ttl set 10 pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action pedit ex munge ip ttl set 10 goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions ls action pedit",
+ "matchPattern": "action order [0-9]+: pedit action pass keys 1.*index 90 ref.*key #0 at ipv4\\+8: val 0a000000 mask 00ffffff",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action pedit"
+ ]
+ }
+]
--
2.20.1
^ permalink raw reply related
* [PATCH net 08/16] net/sched: act_nat: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action nat ingress 1.18.1.1 1.18.2.2 pass index 90
# tc actions replace action nat \
> ingress 1.18.1.1 1.18.2.2 goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action nat
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: nat ingress 1.18.1.1/32 1.18.2.2 goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000002d180067 P4D 800000002d180067 PUD 7cb8b067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 164 Comm: kworker/3:1 Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffae4500e2fad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9fa52e28c800 RCX: 0000000001011201
RDX: 0000000000000000 RSI: 0000000000000056 RDI: ffff9fa52ca12800
RBP: ffffae4500e2fb70 R08: 0000000000000022 R09: 000000000000000e
R10: 00000000ffffffff R11: 0000000001011201 R12: ffff9fa52cbc9c00
R13: ffff9fa52cbc9c08 R14: 0000000000000001 R15: ffff9fa52ca12780
FS: 0000000000000000(0000) GS:ffff9fa57db80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000073f8c004 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_nat veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel mbcache jbd2 snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd glue_helper snd_timer snd joydev virtio_balloon pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs qxl ata_generic pata_acpi drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm virtio_net virtio_blk net_failover failover virtio_console drm crc32c_intel floppy ata_piix libata virtio_pci virtio_ring virtio serio_raw dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_nat_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_nat.c | 12 +++++++++
.../tc-testing/tc-tests/actions/nat.json | 25 +++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 526c4c99bcce..97aa55b4b268 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -21,6 +21,7 @@
#include <linux/string.h>
#include <linux/tc_act/tc_nat.h>
#include <net/act_api.h>
+#include <net/pkt_cls.h>
#include <net/icmp.h>
#include <net/ip.h>
#include <net/netlink.h>
@@ -42,6 +43,7 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_NAT_MAX + 1];
struct tc_nat *parm;
int ret = 0, err;
@@ -77,6 +79,9 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
} else {
return err;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
p = to_tcf_nat(*a);
spin_lock_bh(&p->tcf_lock);
@@ -85,13 +90,20 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
p->mask = parm->mask;
p->flags = parm->flags;
+ oldchain = p->tcf_goto_chain;
+ p->tcf_goto_chain = newchain;
p->tcf_action = parm->action;
spin_unlock_bh(&p->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_nat_act(struct sk_buff *skb, const struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/nat.json b/tools/testing/selftests/tc-testing/tc-tests/actions/nat.json
index 0080dc2fd41c..bc12c1ccad30 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/nat.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/nat.json
@@ -589,5 +589,30 @@
"teardown": [
"$TC actions flush action nat"
]
+ },
+ {
+ "id": "4b12",
+ "name": "Replace nat action with invalid goto chain control",
+ "category": [
+ "actions",
+ "nat"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action nat",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action nat ingress 1.18.1.1 1.18.2.2 drop index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action nat ingress 1.18.1.1 1.18.2.2 goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action nat index 90",
+ "matchPattern": "action order [0-9]+: nat ingress 1.18.1.1/32 1.18.2.2 drop.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action nat"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 07/16] net/sched: act_connmark: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action connmark pass index 90
# tc actions replace action connmark \
> goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action connmark
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: connmark zone 0 goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 302 Comm: kworker/0:2 Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffff9bea406c3ad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff8c5dfc009f00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff9bea406c3a80 RDI: ffff8c5dfb9d6ec0
RBP: ffff9bea406c3b70 R08: ffff8c5dfda222a0 R09: ffffffff90933c3c
R10: 0000000000000000 R11: 0000000092793f7d R12: ffff8c5df48b3c00
R13: ffff8c5df48b3c08 R14: 0000000000000001 R15: ffff8c5dfb9d6e40
FS: 0000000000000000(0000) GS:ffff8c5dfda00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000062e0e006 CR4: 00000000001606f0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_connmark nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul mbcache crc32_pclmul jbd2 snd_hda_codec_generic ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel snd_timer crypto_simd cryptd snd glue_helper joydev virtio_balloon pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper virtio_net net_failover syscopyarea virtio_blk failover virtio_console sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix crc32c_intel serio_raw libata virtio_pci virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_connmark_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_connmark.c | 24 +++++++++++++++++-
.../tc-testing/tc-tests/actions/connmark.json | 25 +++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 30c4c109c80c..5e02f9f6850a 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -21,6 +21,7 @@
#include <net/netlink.h>
#include <net/pkt_sched.h>
#include <net/act_api.h>
+#include <net/pkt_cls.h>
#include <uapi/linux/tc_act/tc_connmark.h>
#include <net/tc_act/tc_connmark.h>
@@ -101,10 +102,11 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_CONNMARK_MAX + 1];
struct tcf_connmark_info *ci;
struct tc_connmark *parm;
- int ret = 0;
+ int ret = 0, err;
if (!nla)
return -EINVAL;
@@ -129,6 +131,12 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
}
ci = to_connmark(*a);
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain,
+ extack);
+ if (err)
+ goto release_idr;
+ oldchain = ci->tcf_goto_chain;
+ ci->tcf_goto_chain = newchain;
ci->tcf_action = parm->action;
ci->net = net;
ci->zone = parm->zone;
@@ -143,15 +151,29 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain,
+ extack);
+ if (err)
+ goto release_idr;
/* replacing action and zone */
spin_lock_bh(&ci->tcf_lock);
+ oldchain = ci->tcf_goto_chain;
+ ci->tcf_goto_chain = newchain;
ci->tcf_action = parm->action;
ci->zone = parm->zone;
spin_unlock_bh(&ci->tcf_lock);
ret = 0;
+ } else {
+ goto end;
}
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
+end:
return ret;
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json b/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json
index 13147a1f5731..cadde8f41fcd 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json
@@ -287,5 +287,30 @@
"teardown": [
"$TC actions flush action connmark"
]
+ },
+ {
+ "id": "c506",
+ "name": "Replace connmark with invalid goto chain control",
+ "category": [
+ "actions",
+ "connmark"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action connmark",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action connmark pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action connmark goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action connmark index 90",
+ "matchPattern": "action order [0-9]+: connmark zone 0 pass.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action connmark"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 06/16] net/sched: act_mirred: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action mirred ingress mirror dev lo pass
# tc actions replace action mirred \
> ingress mirror dev lo goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action mirred
had the following output:
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: mirred (Ingress Mirror to device lo) goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
Mirror/redirect action on
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 47 Comm: kworker/3:1 Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffa772404b7ad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9c5afc3f4300 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff9c5afdba9380 RDI: 0000000000029380
RBP: ffffa772404b7b70 R08: ffff9c5af7010028 R09: ffff9c5af7010029
R10: 0000000000000000 R11: ffff9c5af94c6a38 R12: ffff9c5af7953000
R13: ffff9c5af7953008 R14: 0000000000000001 R15: ffff9c5af7953d00
FS: 0000000000000000(0000) GS:ffff9c5afdb80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007c514004 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_mirred veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul snd_hda_codec_generic crc32_pclmul snd_hda_intel snd_hda_codec mbcache ghash_clmulni_intel jbd2 snd_hwdep snd_hda_core snd_seq snd_seq_device snd_pcm aesni_intel snd_timer snd crypto_simd cryptd glue_helper soundcore virtio_balloon joydev pcspkr i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops virtio_net ttm virtio_blk net_failover virtio_console failover drm ata_piix crc32c_intel virtio_pci serio_raw libata virtio_ring virtio floppy dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_mirred_init() proved to fix the
above issue. For the same reason, postpone the assignment of tcfa_action
and tcfm_eaction to avoid partial reconfiguration of a mirred rule when
it's replaced by another one that mirrors to a device that does not
exist. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_mirred.c | 23 ++++++++++++++---
.../tc-testing/tc-tests/actions/mirred.json | 25 +++++++++++++++++++
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 69dda57f1097..49f5116d01e6 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -99,6 +99,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
struct nlattr *tb[TCA_MIRRED_MAX + 1];
+ struct tcf_chain *newchain = NULL, *oldchain;
bool mac_header_xmit = false;
struct tc_mirred *parm;
struct tcf_mirred *m;
@@ -158,18 +159,20 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
+
m = to_mirred(*a);
spin_lock_bh(&m->tcf_lock);
- m->tcf_action = parm->action;
- m->tcfm_eaction = parm->eaction;
if (parm->ifindex) {
dev = dev_get_by_index(net, parm->ifindex);
if (!dev) {
spin_unlock_bh(&m->tcf_lock);
- tcf_idr_release(*a, bind);
- return -ENODEV;
+ err = -ENODEV;
+ goto put_chain;
}
mac_header_xmit = dev_is_mac_header_xmit(dev);
rcu_swap_protected(m->tcfm_dev, dev,
@@ -178,6 +181,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
dev_put(dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
}
+ oldchain = m->tcf_goto_chain;
+ m->tcf_goto_chain = newchain;
+ m->tcf_action = parm->action;
+ m->tcfm_eaction = parm->eaction;
spin_unlock_bh(&m->tcf_lock);
if (ret == ACT_P_CREATED) {
@@ -188,7 +195,15 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
tcf_idr_insert(tn, *a);
}
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return ret;
+put_chain:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index db49fd0f8445..6e5fb3d25681 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -434,5 +434,30 @@
"teardown": [
"$TC actions flush action mirred"
]
+ },
+ {
+ "id": "2a9a",
+ "name": "Replace mirred action with invalid goto chain control",
+ "category": [
+ "actions",
+ "mirred"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mirred",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action mirred ingress mirror dev lo drop index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action mirred ingress mirror dev lo goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action mirred index 90",
+ "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) drop.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mirred"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 05/16] net/sched: act_ife: validate the control action inside init()
From: Davide Caratti @ 2019-02-27 17:40 UTC (permalink / raw)
To: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Vlad Buslov
Cc: pabeni, netdev
In-Reply-To: <cover.1551288982.git.dcaratti@redhat.com>
the following script:
# tc qdisc add dev crash0 clsact
# tc filter add dev crash0 egress matchall \
> action ife encode allow mark pass index 90
# tc actions replace action ife \
> encode allow mark goto chain 42 index 90 cookie c1a0c1a0
# tc action show action ife
had the following output:
IFE type 0xED3E
IFE type 0xED3E
Error: Failed to init TC action chain.
We have an error talking to the kernel
total acts 1
action order 0: ife encode action goto chain 42 type 0XED3E
allow mark
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
#PF error: [normal kernel read fault]
PGD 800000007b4e7067 P4D 800000007b4e7067 PUD 7b4e6067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 2 PID: 164 Comm: kworker/2:1 Not tainted 5.0.0-rc4.gotochain_crash+ #533
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:tcf_action_exec+0xb8/0x100
Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 89 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3
RSP: 0018:ffffa6a7c0553ad0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9796ee1bbd00 RCX: 0000000000000001
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffa6a7c0553b70 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: ffff9797385bb038 R12: ffff9796ead9d700
R13: ffff9796ead9d708 R14: 0000000000000001 R15: ffff9796ead9d800
FS: 0000000000000000(0000) GS:ffff97973db00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000007c41e006 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ndisc_next_option+0x50/0x50
? ___neigh_create+0x4d5/0x680
? ip6_finish_output2+0x1b5/0x590
ip6_finish_output2+0x1b5/0x590
? ip6_output+0x68/0x110
ip6_output+0x68/0x110
? nf_hook.constprop.28+0x79/0xc0
ndisc_send_skb+0x248/0x2e0
ndisc_send_ns+0xf8/0x200
? addrconf_dad_work+0x389/0x4b0
addrconf_dad_work+0x389/0x4b0
? __switch_to_asm+0x34/0x70
? process_one_work+0x195/0x380
? addrconf_dad_completed+0x370/0x370
process_one_work+0x195/0x380
worker_thread+0x30/0x390
? process_one_work+0x380/0x380
kthread+0x113/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: act_gact act_meta_mark act_ife dummy veth ip6table_filter ip6_tables iptable_filter binfmt_misc snd_hda_codec_generic ext4 snd_hda_intel snd_hda_codec crct10dif_pclmul mbcache crc32_pclmul jbd2 snd_hwdep snd_hda_core ghash_clmulni_intel snd_seq snd_seq_device snd_pcm snd_timer aesni_intel crypto_simd snd cryptd glue_helper virtio_balloon joydev pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi qxl virtio_net drm_kms_helper virtio_blk net_failover syscopyarea failover sysfillrect virtio_console sysimgblt fb_sys_fops ttm drm crc32c_intel serio_raw ata_piix virtio_pci virtio_ring libata virtio floppy dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_ife]
CR2: 0000000000000000
Validating the control action within tcf_ife_init() proved to fix the
above issue. A TDC selftest is added to verify the correct behavior.
Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_ife.c | 32 ++++++++++++-------
.../tc-testing/tc-tests/actions/ife.json | 25 +++++++++++++++
2 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 9b2eb941e093..4480edb386d7 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -29,6 +29,7 @@
#include <net/net_namespace.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#include <uapi/linux/tc_act/tc_ife.h>
#include <net/tc_act/tc_ife.h>
#include <linux/etherdevice.h>
@@ -472,6 +473,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_IFE_MAX + 1];
struct nlattr *tb2[IFE_META_MAX + 1];
struct tcf_ife_params *p;
@@ -531,6 +533,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
}
ife = to_ife(*a);
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
+
p->flags = parm->flags;
if (parm->flags & IFE_ENCODE) {
@@ -563,13 +569,8 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
if (tb[TCA_IFE_METALST]) {
err = nla_parse_nested(tb2, IFE_META_MAX, tb[TCA_IFE_METALST],
NULL, NULL);
- if (err) {
-metadata_parse_err:
- tcf_idr_release(*a, bind);
- kfree(p);
- return err;
- }
-
+ if (err)
+ goto metadata_parse_err;
err = populate_metalist(ife, tb2, exists, rtnl_held);
if (err)
goto metadata_parse_err;
@@ -581,15 +582,14 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
* going to bail out
*/
err = use_all_metadata(ife, exists);
- if (err) {
- tcf_idr_release(*a, bind);
- kfree(p);
- return err;
- }
+ if (err)
+ goto metadata_parse_err;
}
if (exists)
spin_lock_bh(&ife->tcf_lock);
+ oldchain = ife->tcf_goto_chain;
+ ife->tcf_goto_chain = newchain;
ife->tcf_action = parm->action;
/* protected by tcf_lock when modifying existing action */
rcu_swap_protected(ife->params, p, 1);
@@ -603,6 +603,14 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
tcf_idr_insert(tn, *a);
return ret;
+
+metadata_parse_err:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
+ kfree(p);
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
index 0da3545cabdb..c13a68b98fc7 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
@@ -1060,5 +1060,30 @@
"matchPattern": "action order [0-9]*: ife encode action pipe.*allow prio.*index 4",
"matchCount": "0",
"teardown": []
+ },
+ {
+ "id": "a0e2",
+ "name": "Replace ife encode action with invalid goto chain control",
+ "category": [
+ "actions",
+ "ife"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ife",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action ife encode allow mark pass index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action ife encode allow mark goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action ife index 90",
+ "matchPattern": "action order [0-9]*: ife encode action pass.*type 0[xX]ED3E .*allow mark.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ife"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
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