* [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
* [PATCH net 04/16] net/sched: act_gact: 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 gact pass index 90
# tc actions replace action gact \
> goto chain 42 index 90 cookie c1a0c1a0
# tc actions show action gact
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: gact action goto chain 42
random type none pass val 0
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: 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:ffff8c2434703be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff8c23ed6d7e00 RCX: 000000000000005a
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8c23ed6d7e00
RBP: ffff8c2434703c80 R08: ffff8c243b639ac8 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8c2429e68b00
R13: ffff8c2429e68b08 R14: 0000000000000001 R15: ffff8c2429c5a480
FS: 0000000000000000(0000) GS:ffff8c2434700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000002dc0e005 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: 74 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:ffff9c8640387eb8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffff8b2184f0 RBX: 0000000000000002 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000087 RDI: 0000000000000002
RBP: 0000000000000002 R08: 000eb57882b36cc3 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_gact act_bpf veth ip6table_filter ip6_tables iptable_filter binfmt_misc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_generic ext4 snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core mbcache jbd2 snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd glue_helper virtio_balloon joydev pcspkr 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 virtio_net sysfillrect net_failover virtio_blk sysimgblt fb_sys_fops virtio_console ttm failover drm crc32c_intel serio_raw ata_piix libata floppy virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_bpf]
CR2: 0000000000000000
Validating the control action within tcf_gact_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_gact.c | 13 ++++++++++
.../tc-testing/tc-tests/actions/gact.json | 25 +++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 727bbca9534b..abe1e23a4db3 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -20,6 +20,7 @@
#include <linux/init.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_gact.h>
#include <net/tc_act/tc_gact.h>
@@ -60,6 +61,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_GACT_MAX + 1];
struct tc_gact *parm;
struct tcf_gact *gact;
@@ -116,9 +118,14 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
return err;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
gact = to_gact(*a);
spin_lock_bh(&gact->tcf_lock);
+ oldchain = gact->tcf_goto_chain;
+ gact->tcf_goto_chain = newchain;
gact->tcf_action = parm->action;
#ifdef CONFIG_GACT_PROB
if (p_parm) {
@@ -135,7 +142,13 @@ static int tcf_gact_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;
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
}
static int tcf_gact_act(struct sk_buff *skb, const struct tc_action *a,
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index 89189a03ce3d..814b7a8a478b 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -560,5 +560,30 @@
"teardown": [
"$TC actions flush action gact"
]
+ },
+ {
+ "id": "ca89",
+ "name": "Replace gact action with invalid goto chain control",
+ "category": [
+ "actions",
+ "gact"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action gact",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action pass random determ drop 2 index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action goto chain 42 random determ drop 5 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action gact",
+ "matchPattern": "action order [0-9]*: gact action pass.*random type determ drop val 2.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action gact"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 03/16] net/sched: act_csum: 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 csum icmp pass index 90
# tc actions replace action csum icmp goto chain 42 index 90 \
> cookie c1a0c1a0
# tc actions show action csum
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: csum (icmp) action 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 8000000074692067 P4D 8000000074692067 PUD 2e210067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 0 Comm: swapper/0 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:ffff93153da03be0 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff9314ee40f700 RCX: 0000000000003a00
RDX: 0000000000000000 RSI: ffff931537c87828 RDI: ffff931537c87818
RBP: ffff93153da03c80 R08: 00000000527cffff R09: 0000000000000003
R10: 000000000000003f R11: 0000000000000028 R12: ffff9314edf68400
R13: ffff9314edf68408 R14: 0000000000000001 R15: ffff9314ed67b600
FS: 0000000000000000(0000) GS:ffff93153da00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000073e32003 CR4: 00000000001606f0
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: 66 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:ffffffff9a803e98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: ffffffff99e184f0 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000087 RDI: 0000000000000000
RBP: 0000000000000000 R08: 000eb5c4572376b3 R09: 0000000000000000
R10: ffffa53e806a3ca0 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_kernel+0x49e/0x4be
secondary_startup_64+0xa4/0xb0
Modules linked in: act_csum veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 crct10dif_pclmul crc32_pclmul snd_hda_codec_generic ghash_clmulni_intel snd_hda_intel mbcache snd_hda_codec jbd2 snd_hwdep snd_hda_core 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 virtio_net sysimgblt net_failover fb_sys_fops virtio_console virtio_blk ttm failover drm ata_piix crc32c_intel floppy virtio_pci serio_raw libata virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
CR2: 0000000000000000
Validating the control action within tcf_csum_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_csum.c | 21 ++++++++++++++--
.../tc-testing/tc-tests/actions/csum.json | 25 +++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 1ae120c9ab02..34c0f30dbc83 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -33,6 +33,7 @@
#include <net/sctp/checksum.h>
#include <net/act_api.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_csum.h>
#include <net/tc_act/tc_csum.h>
@@ -50,6 +51,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct tcf_csum_params *params_new;
struct nlattr *tb[TCA_CSUM_MAX + 1];
struct tc_csum *parm;
@@ -87,16 +89,22 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
return err;
}
+ err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (err)
+ goto release_idr;
+
p = to_tcf_csum(*a);
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
- tcf_idr_release(*a, bind);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto put_chain;
}
params_new->update_flags = parm->update_flags;
spin_lock_bh(&p->tcf_lock);
+ oldchain = p->tcf_goto_chain;
+ p->tcf_goto_chain = newchain;
p->tcf_action = parm->action;
rcu_swap_protected(p->params, params_new,
lockdep_is_held(&p->tcf_lock));
@@ -108,7 +116,16 @@ static int tcf_csum_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;
}
/**
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json b/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json
index a022792d392a..ddabb2fbb7c7 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json
@@ -500,5 +500,30 @@
"matchPattern": "^[ \t]+index [0-9]+ ref",
"matchCount": "0",
"teardown": []
+ },
+ {
+ "id": "d128",
+ "name": "Replace csum action with invalid goto chain control",
+ "category": [
+ "actions",
+ "csum"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action csum",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action csum iph index 90"
+ ],
+ "cmdUnderTest": "$TC actions replace action csum iph goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action csum index 90",
+ "matchPattern": "action order [0-9]*: csum \\(iph\\) action pass.*index 90 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action csum"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 02/16] net/sched: act_bpf: 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 bpf bytecode '1,6 0 0 4294967295' pass index 90
# tc actions replace action bpf \
> bytecode '1,6 0 0 4294967295' goto chain 42 index 90 cookie c1a0c1a0
# tc action show action bpf
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: bpf bytecode '1,6 0 0 4294967295' default-action goto chain 42
index 90 ref 2 bind 1
cookie c1a0c1a0
Then, the first packet transmitted by crash0 made the kernel crash:
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:ffffb3a0803dfa90 EFLAGS: 00010246
RAX: 000000002000002a RBX: ffff942b347ada00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffb3a08034d038 RDI: ffff942b347ada00
RBP: ffffb3a0803dfb30 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: ffffb3a0803dfb0c R12: ffff942b3b682b00
R13: ffff942b3b682b08 R14: 0000000000000001 R15: ffff942b3b682f00
FS: 00007f6160a72740(0000) GS:ffff942b3da80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000795a4002 CR4: 00000000001606e0
Call Trace:
tcf_classify+0x58/0x120
__dev_queue_xmit+0x40a/0x890
? ip_finish_output2+0x16f/0x430
ip_finish_output2+0x16f/0x430
? ip_output+0x69/0xe0
ip_output+0x69/0xe0
? ip_forward_options+0x1a0/0x1a0
ip_send_skb+0x15/0x40
raw_sendmsg+0x8e1/0xbd0
? sched_clock+0x5/0x10
? sched_clock_cpu+0xc/0xa0
? try_to_wake_up+0x54/0x480
? ldsem_down_read+0x3f/0x280
? _cond_resched+0x15/0x40
? down_read+0xe/0x30
? copy_termios+0x1e/0x70
? tty_mode_ioctl+0x1b6/0x4c0
? sock_sendmsg+0x36/0x40
sock_sendmsg+0x36/0x40
__sys_sendto+0x10e/0x140
? do_vfs_ioctl+0xa4/0x640
? handle_mm_fault+0xdc/0x210
? 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:0x7f615f7e3c03
Code: 48 8b 0d 90 62 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 9d c3 2c 00 00 75 13 49 89 ca b8 2c 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 34 c3 48 83 ec 08 e8 4b cc 00 00 48 89 04 24
RSP: 002b:00007ffee5d8cc28 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 000055a4f28f1700 RCX: 00007f615f7e3c03
RDX: 0000000000000040 RSI: 000055a4f28f1700 RDI: 0000000000000003
RBP: 00007ffee5d8e340 R08: 000055a4f28ee510 R09: 0000000000000010
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000040
R13: 000055a4f28f16c0 R14: 000055a4f28ef69c R15: 0000000000000080
Modules linked in: act_bpf veth ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 mbcache crct10dif_pclmul jbd2 crc32_pclmul 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 crypto_simd cryptd glue_helper pcspkr joydev 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 virtio_blk virtio_net virtio_console net_failover failover syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm crc32c_intel ata_piix 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_bpf_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_bpf.c | 13 ++++++++++
.../tc-testing/tc-tests/actions/bpf.json | 25 +++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 88a729bdab25..9aa24decffc4 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -17,6 +17,7 @@
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
#include <linux/tc_act/tc_bpf.h>
#include <net/tc_act/tc_bpf.h>
@@ -281,6 +282,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
+ struct tcf_chain *newchain = NULL, *oldchain;
struct nlattr *tb[TCA_ACT_BPF_MAX + 1];
struct tcf_bpf_cfg cfg, old;
struct tc_act_bpf *parm;
@@ -323,6 +325,10 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
return ret;
}
+ ret = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+ if (ret < 0)
+ goto release_idr;
+
is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];
is_ebpf = tb[TCA_ACT_BPF_FD];
@@ -350,6 +356,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
if (cfg.bpf_num_ops)
prog->bpf_num_ops = cfg.bpf_num_ops;
+ oldchain = prog->tcf_goto_chain;
+ prog->tcf_goto_chain = newchain;
prog->tcf_action = parm->action;
rcu_assign_pointer(prog->filter, cfg.filter);
spin_unlock_bh(&prog->tcf_lock);
@@ -361,9 +369,14 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
synchronize_rcu();
tcf_bpf_cfg_cleanup(&old);
}
+ if (oldchain)
+ tcf_chain_put_by_act(oldchain);
return res;
out:
+ if (newchain)
+ tcf_chain_put_by_act(newchain);
+release_idr:
tcf_idr_release(*act, bind);
return ret;
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json b/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json
index 5970cee6d05f..b074ea9b6fe8 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json
@@ -286,5 +286,30 @@
"teardown": [
"$TC action flush action bpf"
]
+ },
+ {
+ "id": "b8a1",
+ "name": "Replace bpf action with invalid goto_chain control",
+ "category": [
+ "actions",
+ "bpf"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action bpf",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC action add action bpf bytecode '1,6 0 0 4294967295' pass index 90"
+ ],
+ "cmdUnderTest": "$TC action replace action bpf bytecode '1,6 0 0 4294967295' goto chain 42 index 90 cookie c1a0c1a0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC action list action bpf",
+ "matchPattern": "action order [0-9]*: bpf.* default-action pass.*index 90",
+ "matchCount": "1",
+ "teardown": [
+ "$TC action flush action bpf"
+ ]
}
]
--
2.20.1
^ permalink raw reply related
* [PATCH net 01/16] net/sched: prepare TC actions to properly validate the control action
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>
- add tcf_action_check_ctrlact(), and pass a pointer to struct tcf_proto
in each actions's init() handler, to allow validation of 'goto chain'
control action.
- remove code that validates the control action after a successful call
to the action's init() handler, and replace it with a test that forbids
addition of actions having 'goto_chain' and no chain handle at the same
time.
This disallows 'goto_chain' on actions that don't initialize it properly
in their init() handler, i.e. calling tcf_action_check_ctrlact() after
successful IDR reservation and then assigning 'tcf_goto_chain' and
'tcf_action' consistently.
By doing this, the kernel does not leak anymore refcounts when a valid
'goto chain' handle is replaced in TC actions, causing kmemleak splats
like the following one:
# tc chain add dev dd0 chain 42 ingress protocol ip flower \
> ip_proto tcp action drop
# tc chain add dev dd0 chain 43 ingress protocol ip flower \
> ip_proto udp action drop
# tc filter add dev dd0 ingress matchall \
> action gact goto chain 42 index 66
# tc filter replace dev dd0 ingress matchall \
> action gact goto chain 43 index 66
# echo scan >/sys/kernel/debug/kmemleak
<...>
unreferenced object 0xffff93c0ee09f000 (size 1024):
comm "tc", pid 2565, jiffies 4295339808 (age 65.426s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 08 00 06 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<000000009b63f92d>] tc_ctl_chain+0x3d2/0x4c0
[<00000000683a8d72>] rtnetlink_rcv_msg+0x263/0x2d0
[<00000000ddd88f8e>] netlink_rcv_skb+0x4a/0x110
[<000000006126a348>] netlink_unicast+0x1a0/0x250
[<00000000b3340877>] netlink_sendmsg+0x2c1/0x3c0
[<00000000a25a2171>] sock_sendmsg+0x36/0x40
[<00000000f19ee1ec>] ___sys_sendmsg+0x280/0x2f0
[<00000000d0422042>] __sys_sendmsg+0x5e/0xa0
[<000000007a6c61f9>] do_syscall_64+0x5b/0x180
[<00000000ccd07542>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[<0000000013eaa334>] 0xffffffffffffffff
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>
---
include/net/act_api.h | 7 +++-
net/sched/act_api.c | 84 ++++++++++++++++++++++----------------
net/sched/act_bpf.c | 2 +-
net/sched/act_connmark.c | 1 +
net/sched/act_csum.c | 2 +-
net/sched/act_gact.c | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_ipt.c | 11 ++---
net/sched/act_mirred.c | 1 +
net/sched/act_nat.c | 3 +-
net/sched/act_pedit.c | 2 +-
net/sched/act_police.c | 1 +
net/sched/act_sample.c | 2 +-
net/sched/act_simple.c | 2 +-
net/sched/act_skbedit.c | 1 +
net/sched/act_skbmod.c | 1 +
net/sched/act_tunnel_key.c | 1 +
net/sched/act_vlan.c | 2 +-
18 files changed, 77 insertions(+), 50 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index dbc795ec659e..f21830b7753a 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -50,6 +50,7 @@ struct tc_action {
#define tcf_qstats common.tcfa_qstats
#define tcf_rate_est common.tcfa_rate_est
#define tcf_lock common.tcfa_lock
+#define tcf_goto_chain common.goto_chain
/* Update lastuse only if needed, to avoid dirtying a cache line.
* We use a temp variable to avoid fetching jiffies twice.
@@ -90,7 +91,7 @@ struct tc_action_ops {
int (*lookup)(struct net *net, struct tc_action **a, u32 index);
int (*init)(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act, int ovr,
- int bind, bool rtnl_held,
+ int bind, bool rtnl_held, struct tcf_proto *tp,
struct netlink_ext_ack *extack);
int (*walk)(struct net *, struct sk_buff *,
struct netlink_callback *, int,
@@ -181,6 +182,10 @@ int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
+int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
+ struct tcf_chain **handle,
+ struct netlink_ext_ack *extack);
+
#endif /* CONFIG_NET_CLS_ACT */
static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index d4b8355737d8..ae44eaa85df8 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -28,18 +28,6 @@
#include <net/act_api.h>
#include <net/netlink.h>
-static int tcf_action_goto_chain_init(struct tc_action *a, struct tcf_proto *tp)
-{
- u32 chain_index = a->tcfa_action & TC_ACT_EXT_VAL_MASK;
-
- if (!tp)
- return -EINVAL;
- a->goto_chain = tcf_chain_get_by_act(tp->chain->block, chain_index);
- if (!a->goto_chain)
- return -ENOMEM;
- return 0;
-}
-
static void tcf_action_goto_chain_fini(struct tc_action *a)
{
tcf_chain_put_by_act(a->goto_chain);
@@ -71,6 +59,48 @@ static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
call_rcu(&old->rcu, tcf_free_cookie_rcu);
}
+int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
+ struct tcf_chain **handle,
+ struct netlink_ext_ack *extack)
+{
+ int opcode = TC_ACT_EXT_OPCODE(action), ret = -EINVAL;
+ u32 chain_index;
+
+ if (!opcode)
+ ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0;
+ else if (opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC)
+ ret = 0;
+ if (ret) {
+ NL_SET_ERR_MSG(extack, "invalid control action");
+ goto end;
+ }
+
+ if (TC_ACT_EXT_CMP(action, TC_ACT_GOTO_CHAIN)) {
+ chain_index = action & TC_ACT_EXT_VAL_MASK;
+ if (!tp) {
+ ret = -EINVAL;
+ NL_SET_ERR_MSG(extack,
+ "can't use goto_chain with NULL proto");
+ goto end;
+ }
+ if (!handle) {
+ ret = -EINVAL;
+ NL_SET_ERR_MSG(extack,
+ "can't put goto_chain on NULL handle");
+ goto end;
+ }
+ *handle = tcf_chain_get_by_act(tp->chain->block, chain_index);
+ if (!*handle) {
+ ret = -ENOMEM;
+ NL_SET_ERR_MSG(extack,
+ "can't allocate goto_chain handle");
+ }
+ }
+end:
+ return ret;
+}
+EXPORT_SYMBOL(tcf_action_check_ctrlact);
+
/* XXX: For standalone actions, we don't need a RCU grace period either, because
* actions are always connected to filters and filters are already destroyed in
* RCU callbacks, so after a RCU grace period actions are already disconnected
@@ -800,15 +830,6 @@ static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
return c;
}
-static bool tcf_action_valid(int action)
-{
- int opcode = TC_ACT_EXT_OPCODE(action);
-
- if (!opcode)
- return action <= TC_ACT_VALUE_MAX;
- return opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC;
-}
-
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
@@ -890,10 +911,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
/* backward compatibility for policer */
if (name == NULL)
err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, ovr, bind,
- rtnl_held, extack);
+ rtnl_held, tp, extack);
else
err = a_o->init(net, nla, est, &a, ovr, bind, rtnl_held,
- extack);
+ tp, extack);
if (err < 0)
goto err_mod;
@@ -907,19 +928,12 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (err != ACT_P_CREATED)
module_put(a_o->owner);
- if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
- err = tcf_action_goto_chain_init(a, tp);
- if (err) {
- tcf_action_destroy_1(a, bind);
- NL_SET_ERR_MSG(extack, "Failed to init TC action chain");
- return ERR_PTR(err);
- }
- }
-
- if (!tcf_action_valid(a->tcfa_action)) {
+ if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN) &&
+ !a->goto_chain) {
+ NL_SET_ERR_MSG(extack, "can't use goto chain with NULL chain");
tcf_action_destroy_1(a, bind);
- NL_SET_ERR_MSG(extack, "Invalid control action value");
- return ERR_PTR(-EINVAL);
+ err = -EINVAL;
+ return ERR_PTR(err);
}
return a;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index c7633843e223..88a729bdab25 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -278,7 +278,7 @@ static void tcf_bpf_prog_fill_cfg(const struct tcf_bpf *prog,
static int tcf_bpf_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act,
int replace, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
struct nlattr *tb[TCA_ACT_BPF_MAX + 1];
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 8475913f2070..30c4c109c80c 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -97,6 +97,7 @@ static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = {
static int tcf_connmark_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3dc25b7806d7..1ae120c9ab02 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -46,7 +46,7 @@ static struct tc_action_ops act_csum_ops;
static int tcf_csum_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind, bool rtnl_held,
+ int bind, bool rtnl_held, struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index b61c20ebb314..727bbca9534b 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -57,7 +57,7 @@ static const struct nla_policy gact_policy[TCA_GACT_MAX + 1] = {
static int tcf_gact_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
struct nlattr *tb[TCA_GACT_MAX + 1];
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 30b63fa23ee2..9b2eb941e093 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -469,7 +469,7 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
static int tcf_ife_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
struct nlattr *tb[TCA_IFE_MAX + 1];
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index faa1addf89b3..13998231bed5 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -97,7 +97,8 @@ static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = {
static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- const struct tc_action_ops *ops, int ovr, int bind)
+ const struct tc_action_ops *ops, int ovr, int bind,
+ struct tcf_proto *tp)
{
struct tc_action_net *tn = net_generic(net, id);
struct nlattr *tb[TCA_IPT_MAX + 1];
@@ -205,20 +206,20 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
static int tcf_ipt_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind, bool rtnl_held,
+ int bind, bool rtnl_held, struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
return __tcf_ipt_init(net, ipt_net_id, nla, est, a, &act_ipt_ops, ovr,
- bind);
+ bind, tp);
}
static int tcf_xt_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind, bool unlocked,
+ int bind, bool unlocked, struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
return __tcf_ipt_init(net, xt_net_id, nla, est, a, &act_xt_ops, ovr,
- bind);
+ bind, tp);
}
static int tcf_ipt_act(struct sk_buff *skb, const struct tc_action *a,
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index c8cf4d10c435..69dda57f1097 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -94,6 +94,7 @@ static struct tc_action_ops act_mirred_ops;
static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index c5c1e23add77..526c4c99bcce 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -38,7 +38,8 @@ static const struct nla_policy nat_policy[TCA_NAT_MAX + 1] = {
static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
struct tc_action **a, int ovr, int bind,
- bool rtnl_held, struct netlink_ext_ack *extack)
+ bool rtnl_held, struct tcf_proto *tp,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
struct nlattr *tb[TCA_NAT_MAX + 1];
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 2b372a06b432..1c7a0db7b466 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -138,7 +138,7 @@ static int tcf_pedit_key_ex_dump(struct sk_buff *skb,
static int tcf_pedit_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
struct nlattr *tb[TCA_PEDIT_MAX + 1];
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index ec8ec55e0fe8..a444dd78a244 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -83,6 +83,7 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
static int tcf_police_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
int ret = 0, tcfp_result = TC_ACT_OK, err, size;
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 1a0c682fd734..b2154edcb535 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -37,7 +37,7 @@ static const struct nla_policy sample_policy[TCA_SAMPLE_MAX + 1] = {
static int tcf_sample_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind, bool rtnl_held,
+ int bind, bool rtnl_held, struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 902957beceb3..640ee5b785dc 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -80,7 +80,7 @@ static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = {
static int tcf_simp_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
struct nlattr *tb[TCA_DEF_MAX + 1];
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index cfceed28c333..9a8a0f2d4418 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -96,6 +96,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 59710a183bd3..35572d0e4576 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -82,6 +82,7 @@ static const struct nla_policy skbmod_policy[TCA_SKBMOD_MAX + 1] = {
static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 3f943de9a2c9..eaa4a9c80898 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -209,6 +209,7 @@ static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
static int tunnel_key_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 93fdaf707313..80fd0e238a10 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -105,7 +105,7 @@ static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
static int tcf_vlan_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
int ovr, int bind, bool rtnl_held,
- struct netlink_ext_ack *extack)
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
struct nlattr *tb[TCA_VLAN_MAX + 1];
--
2.20.1
^ permalink raw reply related
* [PATCH net 00/16] validate the control action with all the other parameters
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
currently, the kernel checks for bad values of the control action in
tcf_action_init_1(), after a successful call to the action's init()
function. This causes three bad behaviors:
1. the "half configuration"
if the action is overwritten, the new configuration data are
applied successfully - but the value of the control action remains
the previous one.
2. the "refcount leak that makes kmemleak complain"
when a valid 'goto chain' action is overwritten with another 'goto chain'
action, the kernel leaks two refcounts.
3. the "kernel crash in the traffic plane"
when an action is overwritten with an invalid 'goto chain' action,
packets hitting the new rule will trigger a NULL pointer dereference.
all the above three problems can be fixed if we validate the control
action in each action's init() handler, the same way as we are already
doing for all the other parameters.
Changes since RFC:
- include a fix for all TC actions
- add a selftest for each TC action
- squash fix for refcount leaks into a single patch, the first in the
series, thanks to Cong Wang
- ensure that chain refcount is released without tcfa_lock held, thanks
to Vlad Buslov
Notes:
- act_ipt didn't need any fix, as the control action is constantly equal
to TC_ACT_OK.
- the selftest for act_simple fails because userspace tc backend for
'simple' does not parse the control action correctly (and hardcodes it
to TC_ACT_PIPE).
Davide Caratti (16):
net/sched: prepare TC actions to properly validate the control action
net/sched: act_bpf: validate the control action inside init()
net/sched: act_csum: validate the control action inside init()
net/sched: act_gact: validate the control action inside init()
net/sched: act_ife: validate the control action inside init()
net/sched: act_mirred: validate the control action inside init()
net/sched: act_connmark: validate the control action inside init()
net/sched: act_nat: validate the control action inside init()
net/sched: act_pedit: validate the control action inside init()
net/sched: act_police: validate the control action inside init()
net/sched: act_sample: validate the control action inside init()
net/sched: act_simple: validate the control action inside init()
net/sched: act_skbedit: validate the control action inside init()
net/sched: act_skbmod: validate the control action inside init()
net/sched: act_tunnel_key: validate the control action inside init()
net/sched: act_vlan: validate the control action inside init()
include/net/act_api.h | 7 +-
net/sched/act_api.c | 84 +++++++++++--------
net/sched/act_bpf.c | 15 +++-
net/sched/act_connmark.c | 25 +++++-
net/sched/act_csum.c | 23 ++++-
net/sched/act_gact.c | 15 +++-
net/sched/act_ife.c | 34 +++++---
net/sched/act_ipt.c | 11 +--
net/sched/act_mirred.c | 24 +++++-
net/sched/act_nat.c | 15 +++-
net/sched/act_pedit.c | 19 ++++-
net/sched/act_police.c | 13 +++
net/sched/act_sample.c | 22 ++++-
net/sched/act_simple.c | 52 +++++++++---
net/sched/act_skbedit.c | 23 ++++-
net/sched/act_skbmod.c | 27 ++++--
net/sched/act_tunnel_key.c | 19 ++++-
net/sched/act_vlan.c | 23 ++++-
.../tc-testing/tc-tests/actions/bpf.json | 25 ++++++
.../tc-testing/tc-tests/actions/connmark.json | 25 ++++++
.../tc-testing/tc-tests/actions/csum.json | 25 ++++++
.../tc-testing/tc-tests/actions/gact.json | 25 ++++++
.../tc-testing/tc-tests/actions/ife.json | 25 ++++++
.../tc-testing/tc-tests/actions/mirred.json | 25 ++++++
.../tc-testing/tc-tests/actions/nat.json | 25 ++++++
.../tc-testing/tc-tests/actions/pedit.json | 51 +++++++++++
.../tc-testing/tc-tests/actions/police.json | 25 ++++++
.../tc-testing/tc-tests/actions/sample.json | 25 ++++++
.../tc-testing/tc-tests/actions/simple.json | 25 ++++++
.../tc-testing/tc-tests/actions/skbedit.json | 25 ++++++
.../tc-testing/tc-tests/actions/skbmod.json | 25 ++++++
.../tc-tests/actions/tunnel_key.json | 25 ++++++
.../tc-testing/tc-tests/actions/vlan.json | 25 ++++++
33 files changed, 757 insertions(+), 95 deletions(-)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
--
2.20.1
^ permalink raw reply
* Re: [PATCH v4 perf,bpf 06/15] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Song Liu @ 2019-02-27 17:38 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: <20190227132123.GE18893@krava>
> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Feb 25, 2019 at 04:20:10PM -0800, Song Liu wrote:
>> bpf_prog_info contains information necessary to annotate bpf programs.
>> This patch saves bpf_prog_info for bpf programs loaded in the system.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> tools/perf/util/bpf-event.c | 32 +++++++++++++-
>> tools/perf/util/bpf-event.h | 7 ++-
>> tools/perf/util/env.c | 85 +++++++++++++++++++++++++++++++++++++
>> tools/perf/util/env.h | 17 ++++++++
>> 4 files changed, 139 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
>> index ff7ee149ec46..ce81b2c43a51 100644
>> --- a/tools/perf/util/bpf-event.c
>> +++ b/tools/perf/util/bpf-event.c
>> @@ -10,6 +10,7 @@
>> #include "debug.h"
>> #include "symbol.h"
>> #include "machine.h"
>> +#include "env.h"
>> #include "session.h"
>>
>> #define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
>> @@ -54,17 +55,28 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> struct bpf_event *bpf_event = &event->bpf_event;
>> struct bpf_prog_info_linear *info_linear;
>> struct perf_tool *tool = session->tool;
>> + struct bpf_prog_info_node *info_node;
>> struct bpf_prog_info *info;
>> struct btf *btf = NULL;
>> bool has_btf = false;
>> + struct perf_env *env;
>> u32 sub_prog_cnt, i;
>> int err = 0;
>> u64 arrays;
>>
>> + /*
>> + * for perf-record and perf-report use header.env;
>> + * otherwise, use global perf_env.
>> + */
>> + env = session->data ? &session->header.env : &perf_env;
>> +
>> arrays = 1UL << BPF_PROG_INFO_JITED_KSYMS;
>> arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
>> arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
>> arrays |= 1UL << BPF_PROG_INFO_PROG_TAGS;
>> + arrays |= 1UL << BPF_PROG_INFO_JITED_INSNS;
>> + arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
>> + arrays |= 1UL << BPF_PROG_INFO_JITED_LINE_INFO;
>>
>> info_linear = bpf_program__get_prog_info_linear(fd, arrays);
>> if (IS_ERR_OR_NULL(info_linear)) {
>> @@ -153,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> machine, process);
>> }
>>
>> - /* Synthesize PERF_RECORD_BPF_EVENT */
>> if (opts->bpf_event) {
>> + /* Synthesize PERF_RECORD_BPF_EVENT */
>> *bpf_event = (struct bpf_event){
>> .header = {
>> .type = PERF_RECORD_BPF_EVENT,
>> @@ -167,6 +179,24 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
>> memset((void *)event + event->header.size, 0, machine->id_hdr_size);
>> event->header.size += machine->id_hdr_size;
>> +
>> + /* save bpf_prog_info to env */
>
> why do we save this to perf_env in here? we just
> synthesize the same data as event, so report and
> top will read it and fill perf_env again, right?
The synthesized events are same as PERF_RECORD_BPF_EVENT
PROG_LOAD. We need to process them and fill perf_env as
soon as we get PROG_LOAD. Otherwise, the program might
unload before we process them.
>
> could you please explain the whole flow of the
> bpf events and its respective data in perf_env
> and put it into the changelog
I will add more information to the change log.
>
>> + info_node = malloc(sizeof(struct bpf_prog_info_node));
>> +
>> + /*
>> + * Do not bail out for !info_node, as we still want to
>> + * call perf_tool__process_synth_event()
>
> well, we are out of memory, so I dont think perf_tool__process_synth_event
> will get too far.. also the perf_env data would be inconsistent with what
> you store as event.. how can that work?
It is OK we have PERF_RECORD_BPF_EVENT but not related
bpf_prog_info and btf. The perf.data file will show
a BPF program was loaded, but we won't be able to do
annotation. Does this make sense?
Thanks,
Song
>
> thanks,
> jirka
>
>> + */
>> + if (info_node) {
>> + info_node->info_linear = info_linear;
>> + perf_env__insert_bpf_prog_info(env, info_node);
>> + info_linear = NULL;
>> + }
>> +
>
> SNIP
^ permalink raw reply
* Re: [PATCH v1] enc28j60: Correct description of debug module parameter
From: David Miller @ 2019-02-27 17:35 UTC (permalink / raw)
To: andriy.shevchenko; +Cc: netdev
In-Reply-To: <20190227104535.15504-1-andriy.shevchenko@linux.intel.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Wed, 27 Feb 2019 13:45:35 +0300
> The netif_msg_init() API takes on input the amount of bits to be set. The
> description of debug parameter in the enc28j60 module is misleading in this
> sense and passing 0xffff does not give an expected behaviour.
>
> Fix the description of debug module parameter to show what exactly is expected.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH v1] net: dev: Use unsigned integer as an argument to left-shift
From: David Miller @ 2019-02-27 17:34 UTC (permalink / raw)
To: andriy.shevchenko; +Cc: netdev
In-Reply-To: <20190227103726.15163-1-andriy.shevchenko@linux.intel.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Wed, 27 Feb 2019 13:37:26 +0300
> 1 << 31 is Undefined Behaviour according to the C standard.
> Use U type modifier to avoid theoretical overflow.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thanks Andy.
^ permalink raw reply
* [PATCH v2 0/2] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
From: pisa @ 2019-02-27 17:26 UTC (permalink / raw)
To: devicetree, mkl, linux-can
Cc: wg, davem, robh+dt, mark.rutland, netdev, linux-kernel,
martin.jerabek01, ondrej.ille, Pavel Pisa
From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
The patch update, added separate patch to add Czech Technical University
to the vendors list. Change ctu_can_fd_0 instance string to lowercase
(originally uppercase because build from official abbreviations).
Clarify that clock-names are not used for now but can be used in future.
Pavel Pisa (2):
dt-bindings: vendor-prefix: add prefix for Czech Technical University
in Prague.
dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
The corresponding sources for testing program from user-space
and for Linux kernel driver for platform and PCIe IP core
integrations are available there
https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core/tree/master/driver/
.../devicetree/bindings/net/can/ctu,ctucanfd.txt | 108 +++++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 109 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
--
2.11.0
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: vendor-prefix: add prefix for Czech Technical University in Prague.
From: pisa @ 2019-02-27 17:26 UTC (permalink / raw)
To: devicetree, mkl, linux-can
Cc: wg, davem, robh+dt, mark.rutland, netdev, linux-kernel,
martin.jerabek01, ondrej.ille, Pavel Pisa
In-Reply-To: <cover.1551287365.git.pisa@cmp.felk.cvut.cz>
From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 389508584f48..07de8df03c57 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -86,6 +86,7 @@ crane Crane Connectivity Solutions
creative Creative Technology Ltd
crystalfontz Crystalfontz America, Inc.
csky Hangzhou C-SKY Microsystems Co., Ltd
+ctu Czech Technical University in Prague
cubietech Cubietech, Ltd.
cypress Cypress Semiconductor Corporation
cznic CZ.NIC, z.s.p.o.
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/2] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
From: pisa @ 2019-02-27 17:26 UTC (permalink / raw)
To: devicetree, mkl, linux-can
Cc: wg, davem, robh+dt, mark.rutland, netdev, linux-kernel,
martin.jerabek01, ondrej.ille, Pavel Pisa
In-Reply-To: <cover.1551287365.git.pisa@cmp.felk.cvut.cz>
From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
.../devicetree/bindings/net/can/ctu,ctucanfd.txt | 108 +++++++++++++++++++++
1 file changed, 108 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
diff --git a/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
new file mode 100644
index 000000000000..fea9c08b79ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
@@ -0,0 +1,108 @@
+Memory mapped CTU CAN FD open-source IP core
+
+The core sources and documentation on project page
+
+ https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core
+ http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf
+
+Integration in Xilinx Zynq SoC based system together with
+OpenCores SJA1000 compatible controllers
+
+ https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top
+
+Martin Jerabek's dimploma thesis with integration and testing
+framework description
+
+ https://dspace.cvut.cz/bitstream/handle/10467/80366/F3-DP-2019-Jerabek-Martin-Jerabek-thesis-2019-canfd.pdf
+
+Required properties:
+
+- compatible : should be one of "ctu,ctucanfd", "ctu,canfd-2".
+ The "canfd-2" has been reserved for older revision of the IP core.
+ The revision can be read from the IP core register as well.
+
+- reg = <(baseaddr) (size)> : specify mapping into physical address
+ space of the processor system.
+
+- interrupts : property with a value describing the interrupt source
+ required for the CTU CAN FD. For Zynq SoC system format is
+ <(is_spi) (number) (type)> where is_spi defines if it is SPI
+ (shared peripheral) interrupt, the second number is translated
+ to the vector by addition of 32 on Zynq-7000 systems and type
+ is IRQ_TYPE_LEVEL_HIGH (4) for Zynq.
+
+- interrupt-parent = <&interrupt-controller-phandle> :
+ is required for Zynq SoC to find map interrupt
+ to the correct controller
+
+- clocks: phandle of reference clock (100 MHz is appropriate
+ for FPGA implementation on Zynq-7000 system).
+
+Optional properties:
+
+- clock-names: not used in actual design but if more clocks are used
+ by cores then "can_clk" would be clock source name for the clocks
+ used to define CAN time-quanta.
+
+Example when integrated to Zynq-7000 system DTS:
+
+ / {
+ /* ... */
+ amba: amba {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+
+ ctu_can_fd_0: ctu_can_fd@43c30000 {
+ compatible = "ctu,ctucanfd";
+ interrupt-parent = <&intc>;
+ interrupts = <0 30 4>;
+ clocks = <&clkc 15>;
+ reg = <0x43c30000 0x10000>;
+ };
+ };
+ };
+
+
+Example when used as DTS overlay on Zynq-7000 system:
+
+
+// Device Tree Example: Full Reconfiguration without Bridges
+/dts-v1/;
+/plugin/;
+
+/ {
+ fragment@0 {
+ target-path = "/fpga-full";
+
+ __overlay__ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ firmware-name = "system.bit.bin";
+ };
+ };
+
+ fragment@1 {
+ target-path = "/amba";
+ __overlay__ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ctu_can_fd_0: ctu_can_fd@43c30000 {
+ compatible = "ctu,ctucanfd";
+ interrupt-parent = <&intc>;
+ interrupts = <0 30 4>;
+ clocks = <&clkc 15>;
+ reg = <0x43c30000 0x10000>;
+ };
+ ctu_can_fd_1: ctu_can_fd@43c70000 {
+ compatible = "ctu,ctucanfd";
+ interrupt-parent = <&intc>;
+ interrupts = <0 31 4>;
+ clocks = <&clkc 15>;
+ reg = <0x43c70000 0x10000>;
+ };
+ };
+ };
+};
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] bnxt_en: Drop oversize TX packets to prevent errors.
From: David Miller @ 2019-02-27 17:30 UTC (permalink / raw)
To: michael.chan; +Cc: netdev
In-Reply-To: <1551257933-13270-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Wed, 27 Feb 2019 03:58:53 -0500
> There have been reports of oversize UDP packets being sent to the
> driver to be transmitted, causing error conditions. The issue is
> likely caused by the dst of the SKB switching between 'lo' with
> 64K MTU and the hardware device with a smaller MTU. Patches are
> being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
> issue.
>
> In the meantime, add a quick length check in the driver to prevent
> the error. The driver uses the TX packet size as index to look up an
> array to setup the TX BD. The array is large enough to support all MTU
> sizes supported by the driver. The oversize TX packet causes the
> driver to index beyond the array and put garbage values into the
> TX BD. Add a simple check to prevent this.
>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>
> David, I think this should be queued for stable as well.
Applied and queued up for -stable.
Thanks for following up on this Michael.
^ permalink raw reply
* Re: [PATCH v4 perf,bpf 07/15] perf, bpf: save bpf_prog_info information as headers to perf.data
From: Song Liu @ 2019-02-27 17:28 UTC (permalink / raw)
To: Jiri Olsa
Cc: Netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Peter Zijlstra, Arnaldo Carvalho de Melo, Jiri Olsa,
Namhyung Kim
In-Reply-To: <20190227132155.GI18893@krava>
> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Feb 25, 2019 at 04:20:11PM -0800, Song Liu wrote:
>
> SNIP
>
>> + if (info_len > sizeof(struct bpf_prog_info)) {
>> + pr_warning("detected invalid bpf_prog_info\n");
>> + goto out;
>> + }
>> +
>> + info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
>> + data_len);
>> + if (!info_linear)
>> + goto out;
>> + info_linear->info_len = sizeof(struct bpf_prog_info);
>> + info_linear->data_len = data_len;
>> + if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
>> + goto out;
>> + if (__do_read(ff, &info_linear->info, info_len))
>> + goto out;
>> + if (info_len < sizeof(struct bpf_prog_info))
>> + memset(((void *)(&info_linear->info)) + info_len, 0,
>> + sizeof(struct bpf_prog_info) - info_len);
>> +
>> + if (__do_read(ff, info_linear->data, data_len))
>> + goto out;
>> +
>> + /* endian mismatch, drop the info, continue */
>
> so there's no way we can swap the data? why?
>
> jirka
Yes, we can swap the data. OTOH, the set is big enough. How about we
implement the swap later? Running perf-report on a system of with
different endianness is a relatively rare case.
Thanks,
Song
>> + if (ff->ph->needs_swap) {
>> + free(info_linear);
>> + continue;
>> + }
>
> SNIP
^ permalink raw reply
* Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
From: Jakub Kicinski @ 2019-02-27 17:23 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190227124135.GB10571@nanopsycho>
On Wed, 27 Feb 2019 13:41:35 +0100, Jiri Pirko wrote:
> Wed, Feb 27, 2019 at 01:23:27PM CET, jiri@resnulli.us wrote:
> >Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
> >>Current port flavours cover simple switches and DSA. Add PF
> >>and VF flavours to cover "switchdev" SR-IOV NICs.
> >>
> >>Example devlink user space output:
> >>
> >>$ devlink port
> >>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
> >>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
> >>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
> >>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
> >
> >Wait a second, howcome pf and vfs have the same PCI address?
>
> Oh, I think you have these as eswitch port representors. Confusing...
FWIW I don't like the word representor, its a port. We don't call
physical ports "representors" even though from ASIC's point of view
they are exactly the same.
^ permalink raw reply
* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Neil Horman @ 2019-02-27 17:13 UTC (permalink / raw)
To: Xin Long; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <CADvbK_cFyjd5T-rrZTBaZCNU0RVJMHd_ouuCE-kKpSWoMpFJog@mail.gmail.com>
On Wed, Feb 27, 2019 at 08:53:26PM +0800, Xin Long wrote:
> On Tue, Feb 26, 2019 at 8:29 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Tue, Feb 26, 2019 at 12:15:54AM +0800, Xin Long wrote:
> > > On Mon, Feb 25, 2019 at 10:08 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > On Mon, Feb 25, 2019 at 09:20:44PM +0800, Xin Long wrote:
> > > > > On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > > >
> > > > > > On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > > > > > > sctp_hdr(skb) only works when skb->transport_header is set properly.
> > > > > > >
> > > > > > > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> > > > > > >
> > > > > > > skb->transport_header is not guaranteed to be right value for sctp.
> > > > > > > It will cause to fail to check the checksum for sctp packets.
> > > > > > >
> > > > > > > So fix it by using offset, which is always right in all places.
> > > > > > >
> > > > > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > > > > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > > ---
> > > > > > > include/net/sctp/checksum.h | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > > > > > > index 32ee65a..1c6e6c0 100644
> > > > > > > --- a/include/net/sctp/checksum.h
> > > > > > > +++ b/include/net/sctp/checksum.h
> > > > > > > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> > > > > > > static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> > > > > > > unsigned int offset)
> > > > > > > {
> > > > > > > - struct sctphdr *sh = sctp_hdr(skb);
> > > > > > > + struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> > > > > > > const struct skb_checksum_ops ops = {
> > > > > > > .update = sctp_csum_update,
> > > > > > > .combine = sctp_csum_combine,
> > > > > > > --
> > > > > > > 2.1.0
> > > > > > >
> > > > > > >
> > > > > > Shouldn't you use skb_set_transport_header and skb_transport_header here?
> > > > > you mean:
> > > > > skb_set_transport_header(skb, offset);
> > > > > sh = sctp_hdr(skb);
> > > > > ?
> > > > >
> > > > > There's no place counting on here to set transport_header.
> > > > > It will be a kinda redundant job, yet skb is 'const'.
> > > > >
> > > > I'm not sure what you mean by "theres no place counting here". We have the
> > > > transport header offset, and you're doing the exact same computation that that
> > > > function does. It seems like we should use it in case the underlying
> > > > implementation changes.
> > > 1. skb_set_transport_header() and sctp_hdr() are like:
> > > skb->transport_header = skb->data - skb->head;
> > > skb->transport_header += offset
> > > sh = skb->head + skb->transport_header;
> > >
> > > 2. in this patch:
> > > sh = (struct sctphdr *)(skb->data + offset); only
> > >
> > > I think the 2nd one is better.
> > >
> > > I feel it's weird to set transport_header here if it's only for
> > > sctp_hdr(skb) in here.
> > >
> > > As for "underlying implementation changes", I don't know exactly the case
> > > but there are quite a few places doing things like:
> > > *hdr = (struct *hdr *)(skb->data + hdroff);
> > >
> > > I'd think it's safe. no?
> > >
> > Safe, yes, it just doesn't seem right. I know you've pointed out several places
> > below that rapidly compute transport offsets in a one-off fashion, but at this
> > same time, the other primary transports (tcp and udp), all seems to use the
> > transport header to do their work (linearizing as necessecary, which sctp also
> > does in sctp_rcv, at least in most cases).
> > > >
> > > > I understand what you are saying regarding the use of a const variable there,
> > > > but perhaps thats an argument for removing the const storage classifier. Better
> > > > still, it would be good to figure out why all paths to this function don't
> > > > already set the transport header offset to begin with (addressing your redundant
> > > > comment)
> > > The issue was reported when going to nf_conntrack by br_netfilter's
> > > bridge-nf-call-iptables.
> > > As you can see on nf_conntrack_in() path, even iphdr is got by:
> > > iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
> > > It's impossible to set skb->transport_header when we're not sure iphdr
> > > in linearized memory.
> > >
> > But if the skb isn't linearized, computing the transport header manually isn't
> > going to help you anyway. You can see that in skb_header_pointer. If the
> > offset they are trying to get to is outside the bounds of the length of the skb
> > (i.e. the fragmented case), it calls skb_copy_bits to linearize the needed
> > segment. It seems we should be doing something simmilar. In most cases we are
> > already linearized from sctp_rcv (possibly all, I need to think about that). All
> > I'm really saying is that by using the skb apis we insulate ourselves from
> > potential changes in how skbs might work in the future. I'm not strictly bound
> > to setting the transport header, but we should definately be getting the
> > transport header via the skb utility functions wherever possible.
> Okay, I will change to fix it with the below patch if you agree.
> I've confirmed this won't affect netfilter.
>
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c
> b/net/netfilter/nf_conntrack_proto_sctp.c
> index d53e3e7..6b53cd2 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -343,7 +343,9 @@ static bool sctp_error(struct sk_buff *skb,
> logmsg = "nf_ct_sctp: failed to read header ";
> goto out_invalid;
> }
> - sh = (const struct sctphdr *)(skb->data + dataoff);
> + /* sctp_compute_cksum() depends on correct transport header */
> + skb_set_transport_header(skb, dataoff);
> + sh = sctp_hdr(skb);
>
Thank you, that looks much better to me.
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [BUG] net/sched : qlen can not really be per cpu ?
From: Eric Dumazet @ 2019-02-27 16:59 UTC (permalink / raw)
To: Cong Wang; +Cc: John Fastabend, Networking, Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <d075c0f5-c2b5-6ea8-f09a-7af5ba4ce05b@gmail.com>
On 02/26/2019 04:56 PM, Eric Dumazet wrote:
>
>
> On 02/26/2019 03:51 PM, Cong Wang wrote:
>> On Tue, Feb 26, 2019 at 3:19 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>
>>>
>>>
>>> On 02/25/2019 10:42 PM, Eric Dumazet wrote:
>>>> HTB + pfifo_fast as a leaf qdisc hits badly the following warning in htb_activate() :
>>>>
>>>> WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen);
>>>>
>>>> This is because pfifo_fast does not update sch->q.qlen, but per cpu counters.
>>>> So cl->leaf.q->q.qlen is zero.
>>>>
>>>> HFSC, CBQ, DRR, QFQ have the same problem.
>>>>
>>>> Any ideas how we can fix this ?
>>>
>>> What about something simple for stable ?
>>> ( I yet have to boot/test this )
>>
>> Is merely updating qlen sufficient for fixing it?
>>
>> I thought it is because of the lack of qdisc_tree_reduce_backlog()
>> in pfifo_fast.
>
> It does not seem to be the qdisc_tree_reduce_backlog() thing.
>
> HTB, HFSC, CBQ, DRR, QFQ only peek at their children 'qlen' to decide if there
> is at least one packet in them.
>
> The backlog is only reported for dumps, but the actual backlog value is not used in data path.
>
>
Another way to fix this would be to have a shadow version of pfifo_fast, which
basically would be the old version of it, that would be automatically selected
when used as a child of another qdisc (except mq/mqprio of course)
This seems not a stable candidate though.
^ permalink raw reply
* Re: [PATCH] bpf: drop refcount if bpf_map_new_fd() fails in map_create()
From: Martin Lau @ 2019-02-27 16:52 UTC (permalink / raw)
To: zerons
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1551278185-5219-1-git-send-email-sironhide0null@gmail.com>
On Wed, Feb 27, 2019 at 10:36:25PM +0800, zerons wrote:
> In bpf/syscall.c, map_create() first set map->usercnt to 1, a file descriptor is
> supposed to return to userspace. When bpf_map_new_fd() fails, drop the refcount.
Thanks for the patch.
Please add a Fixes tag for bug fix in the future.
Fixes: bd5f5f4ecb78 ("bpf: Add BPF_MAP_GET_FD_BY_ID")
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [RFC] nasty corner case in unix_dgram_sendmsg()
From: Jason Baron @ 2019-02-27 16:45 UTC (permalink / raw)
To: Al Viro; +Cc: Rainer Weikusat, netdev
In-Reply-To: <20190226235912.GL2217@ZenIV.linux.org.uk>
On 2/26/19 6:59 PM, Al Viro wrote:
> On Tue, Feb 26, 2019 at 03:35:39PM -0500, Jason Baron wrote:
>
>>> I understand what the unix_dgram_peer_wake_me() is doing; I understand
>>> what unix_dgram_poll() is using it for. What I do not understand is
>>> what's the point of doing that in unix_dgram_sendmsg()...
>>>
>>
>> Hi,
>>
>> So the unix_dgram_peer_wake_me() in unix_dgram_sendmsg() is there for
>> epoll in edge-triggered mode. In that case, we want to ensure that if
>> -EAGAIN is returned a subsequent epoll_wait() is not stuck indefinitely.
>> Probably could use a comment...
>
> *owwww*
>
> Let me see if I've got it straight - you want the forwarding rearmed,
> so that it would match the behaviour of ep_poll_callback() (i.e.
> removing only when POLLFREE is passed)? Looks like an odd way to
> do it, if that's what's happening...
If unix_dgram_sendmsg() return -EAGAIN in this case, then a subsequent call
to poll()/select()/epoll_wait() is normally going to do the forwarding rearm
via unix_dgram_poll() (unless its already writeable). However, in the
special case of epoll with edge-trigger, the call to epoll_wait does not
call unix_dgram_poll() and thus the re-arm has to happen in
unix_dgram_sendmsg().
>
> While we are at it, why disarm a forwarder upon noticing that peer
> is dead? Wouldn't it be simpler to move that
> wake_up_interruptible_all(&u->peer_wait);
> in unix_release_sock() to just before
> unix_state_unlock(sk);
> a line prior? Then anyone seeing SOCK_DEAD on (locked) peer
> would be guaranteed that all forwarders are gone...
>
The condition we are checking here is unix_recvq_full(), so even if
the wakeup happens under the lock, we could end up waking up the
waiter that still sees unix_recvq_full() because the skb's aren't
freed until *after* the wakeup call. The race is described here:
51f7e95 af_unix: ensure POLLOUT on remote close() for connected dgram socket
Note, that I did have an earlier version of that patch that moved
the wake up call (instead of checking for SOCK_DEAD), see:
https://patchwork.ozlabs.org/patch/944593/
However, I thought that the explicit check for SOCK_DEAD made things
more explicit. IE we don't wait on a SOCK_DEAD socket.
> Another fun question about the same dgram sendmsg:
> if (unix_peer(sk) == other) {
> unix_peer(sk) = NULL;
> unix_dgram_peer_wake_disconnect_wakeup(sk, other);
>
> unix_state_unlock(sk);
>
> unix_dgram_disconnected(sk, other);
>
> ... and we are holding any locks at the last line. What happens
> if we have thread A doing
> decide which address to talk to
> connect(fd, that address)
> send request over fd (with send(2) or write(2))
> read reply from fd (recv(2) or read(2))
> in a loop, with thread B doing explicit sendto(2) over the same
> socket?
>
> Suppose B happens to send to the last server thread A was talking
> to and finds it just closed (e.g. because the last request from
> A had been "shut down", which server has honoured). B gets ECONNREFUSED,
> as it ought to, but it can also ends up disrupting the next exchange
> of A.
>
> Shouldn't we rather extract the skbs from that queue *before*
> dropping sk->lock? E.g. move them to a temporary queue, and flush
> that queue after we'd unlocked sk...
>
If I understand your concern, B drops the lock as above and then
A does a connect() to somewhere else and then B drops skbs from the
new source. Looks plausible. I think in general, A and B would probably
be co-ordinating if they are both reading/writing the same socket,
but I think it probably would make sense to fix this case. Note that,
unix_dgram_disconnected() is also called in unix_dgram_connect() after
the lock is dropped so that would need a similar fix.
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH V2] samples: bpf: fix: broken sample regarding removed function
From: Daniel Borkmann @ 2019-02-27 16:38 UTC (permalink / raw)
To: Daniel T. Lee, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <20190227075226.4409-1-danieltimlee@gmail.com>
On 02/27/2019 08:52 AM, Daniel T. Lee wrote:
> Currently, running sample "task_fd_query" and "tracex3" occurs the
> following error. On kernel v5.0-rc* this sample will be unavailable
> due to the removal of function 'blk_start_request' at commit "a1ce35f".
> (function removed, as "Single Queue IO scheduler" no longer exists)
>
> $ sudo ./task_fd_query
> failed to create kprobe 'blk_start_request' error 'No such file or
> directory'
>
> This commit will change the function 'blk_start_request' to
> 'blk_mq_start_request' to fix the broken sample.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Applied, thanks! (Also please make sure to test your changes in future
ahead of submission to save valuable reviewer time.)
^ permalink raw reply
* Re: [PATCH v3 iproute2-next 05/11] devlink: Add devlink health show command
From: David Ahern @ 2019-02-27 16:36 UTC (permalink / raw)
To: Aya Levin; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha
In-Reply-To: <1551012378-29167-6-git-send-email-ayal@mellanox.com>
On 2/24/19 5:46 AM, Aya Levin wrote:
> Add devlink health show command which displays status and configuration
> info on a specific reporter on a device or dump the info on all
> reporters on all devices. Add helper functions to display status and
> dump's time stamp.
> Example:
> $ devlink health show pci/0000:00:09.0 reporter tx
> pci/0000:00:09.0:
> name tx
> state healthy error 0 recover 1 last_dump_date 2019-02-14 last_dump_time 10:10:10 grace_period 600 auto_recover true
> $ devlink health show pci/0000:00:09.0 reporter tx -jp
> {
> "health":{
> "pci/0000:00:0a.0":[
> {
> "name":"tx",
> "state":"healthy",
> "error":0,
> "recover":1,
> "last_dump_date":"2019-Feb-14",
> "last_dump_time":"10:10:10",
> "grace_period":600,
> "auto_recover":true
> }
> ]
> }
>
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> devlink/devlink.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 184 insertions(+), 1 deletion(-)
>
This one no longer applies. Please respin.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 0/4] bpf: per program stats
From: Daniel Borkmann @ 2019-02-27 16:32 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>
On 02/25/2019 11:28 PM, Alexei Starovoitov wrote:
> Introduce per program stats to monitor the usage BPF
>
> v2->v3:
> - rename to run_time_ns/run_cnt everywhere
>
> v1->v2:
> - fixed u64 stats on 32-bit archs. Thanks Eric
> - use more verbose run_time_ns in json output as suggested by Andrii
> - refactored prog_alloc and clarified behavior of stats in subprogs
>
> Alexei Starovoitov (4):
> bpf: enable program stats
> bpf: expose program stats via bpf_prog_info
> tools/bpf: sync bpf.h into tools
> tools/bpftool: recognize bpf_prog_info run_time_ns and run_cnt
>
> include/linux/bpf.h | 9 +++++
> include/linux/filter.h | 20 +++++++++-
> include/uapi/linux/bpf.h | 2 +
> kernel/bpf/core.c | 31 ++++++++++++++-
> kernel/bpf/syscall.c | 39 ++++++++++++++++++-
> kernel/bpf/verifier.c | 7 +++-
> kernel/sysctl.c | 34 ++++++++++++++++
> .../bpftool/Documentation/bpftool-prog.rst | 4 +-
> tools/bpf/bpftool/prog.c | 7 ++++
> tools/include/uapi/linux/bpf.h | 2 +
> 10 files changed, 148 insertions(+), 7 deletions(-)
>
Applied, thanks!
^ permalink raw reply
* RE: [PATCH net 4/4] tls: Fix tls_device receive
From: Vakul Garg @ 2019-02-27 16:28 UTC (permalink / raw)
To: Boris Pismenny, Dave Watson
Cc: Aviad Yehezkel, john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org, Eran Ben Elisha
In-Reply-To: <e35a912b-7f09-92f9-8f3c-bbf0def3eae0@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Wednesday, February 27, 2019 8:54 PM
> To: Vakul Garg <vakul.garg@nxp.com>; Dave Watson
> <davejwatson@fb.com>
> Cc: Aviad Yehezkel <aviadye@mellanox.com>; john.fastabend@gmail.com;
> daniel@iogearbox.net; netdev@vger.kernel.org; Eran Ben Elisha
> <eranbe@mellanox.com>
> Subject: Re: [PATCH net 4/4] tls: Fix tls_device receive
>
>
>
> On 2/27/2019 5:08 AM, Vakul Garg wrote:
> >
> >
> >> -----Original Message-----
> >> From: Dave Watson <davejwatson@fb.com>
> >> Sent: Wednesday, February 27, 2019 2:05 AM
> >> To: Boris Pismenny <borisp@mellanox.com>
> >> Cc: aviadye@mellanox.com; john.fastabend@gmail.com;
> >> daniel@iogearbox.net; Vakul Garg <vakul.garg@nxp.com>;
> >> netdev@vger.kernel.org; eranbe@mellanox.com
> >> Subject: Re: [PATCH net 4/4] tls: Fix tls_device receive
> >>
> >> On 02/26/19 02:12 PM, Boris Pismenny wrote:
> >>> Currently, the receive function fails to handle records already
> >>> decrypted by the device due to the commit mentioned below.
> >>>
> >>> This commit advances the TLS record sequence number and prepares the
> >>> context to handle the next record.
> >>>
> >>> Fixes: fedf201e1296 ("net: tls: Refactor control message handling on
> >>> recv")
> >>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> >>> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> >>> ---
> >>> net/tls/tls_sw.c | 15 +++++++--------
> >>> 1 file changed, 7 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> >>> f515cd7e984e..85da10182d8d 100644
> >>> --- a/net/tls/tls_sw.c
> >>> +++ b/net/tls/tls_sw.c
> >>> @@ -1481,18 +1481,17 @@ static int decrypt_skb_update(struct sock
> >>> *sk, struct sk_buff *skb,
> >>>
> >>> return err;
> >>> }
> >>> -
> >>> - rxm->full_len -= padding_length(ctx, tls_ctx, skb);
> >>> -
> >>> - rxm->offset += prot->prepend_size;
> >>> - rxm->full_len -= prot->overhead_size;
> >>> - tls_advance_record_sn(sk, &tls_ctx->rx, version);
> >>> - ctx->decrypted = true;
> >>> - ctx->saved_data_ready(sk);
> >>> } else {
> >>> *zc = false;
> >>> }
> >>>
> >>> + rxm->full_len -= padding_length(ctx, tls_ctx, skb);
> >>> + rxm->offset += prot->prepend_size;
> >>> + rxm->full_len -= prot->overhead_size;
> >>> + tls_advance_record_sn(sk, &tls_ctx->rx, version);
> >>> + ctx->decrypted = true;
> >>> + ctx->saved_data_ready(sk);
> >>> +
> >>> return err;
> >>> }
> >>
> >> This breaks the tls.control_msg test:
> >>
> >> [ RUN ] tls.control_msg
> >> tls.c:764:tls.control_msg:Expected memcmp(buf, test_str, send_len)
> >> (18446744073709551614) == 0 (0)
> >> tls.c:777:tls.control_msg:Expected memcmp(buf, test_str, send_len)
> >> (18446744073709551614) == 0 (0)
> >> tls.control_msg: Test failed at step #8
> >>
> >> So either control message handling needs to only call
> >> decrypt_skb_update once, or we need a new flag or something to handle
> >> the device case
> >
> > I prefer to remove variable 'decrypted' in context.
> > This is no longer required as we already have an rx_list in context for
> storing decrypted records.
> > So for any record which we decrypted but did not return to user space
> > (e.g. for the case when user used recv() and it lead to decryption of
> > non-data record), we should it in rx_list.
> >
>
> IMO this is inappropriate here, because packets decrypted by tls_device are
> ready to be received, and there is no reason to bounce them through the
> rx_list.
My point was about preventing tls_sw_recvmsg() from calling decrypt_skb_update()
with an already decrypted record. The test case failed because an already decrypted record
got dequeued and passed to decrypt_skb_update() from tls_sw_recvmsg().
For packets decrypted by device, a check using skb->decrypted should be enough.
For now, I think your patch is ok.
I can submit a simplification patch for removing 'decrypted' from tls context later.
^ permalink raw reply
* Re: [PATCH iproute2-next] devlink: add support for updating device flash
From: David Ahern @ 2019-02-27 16:26 UTC (permalink / raw)
To: Jakub Kicinski, jiri, stephen; +Cc: oss-drivers, netdev
In-Reply-To: <20190226202014.23626-1-jakub.kicinski@netronome.com>
On 2/26/19 1:20 PM, Jakub Kicinski wrote:
> Add new command for updating flash of devices via devlink API.
> Example:
>
> $ cp flash-boot.bin /lib/firmware/
> $ devlink dev flash pci/0000:05:00.0 file flash-boot.bin
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> devlink/devlink.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> man/man8/devlink-dev.8 | 32 +++++++++++++++++++++++++
> 2 files changed, 86 insertions(+)
>
applied to iproute2-next after updating the headers. Thanks
^ permalink raw reply
* Re: [PATCH iproute2-next] rdma: Add the prefix for driver attributes
From: David Ahern @ 2019-02-27 16:26 UTC (permalink / raw)
To: Leon Romanovsky, Lijun Ou, Steve Wise
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20190227064151.25445-1-leon@kernel.org>
On 2/26/19 11:41 PM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> There is a need to distinguish between driver vs. general exposed
> attributes. The most common use case is to expose some internal
> garbage under extremely common and sexy name, e.g. pi, ci e.t.c
>
> In order to achieve that, we will add "drv_" prefix to all strings
> which were received through RDMA_NLDEV_ATTR_DRIVER_* attributes.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>a
> ---
> rdma/utils.c | 34 ++++++++++++++++++++++------------
> 1 file changed, 22 insertions(+), 12 deletions(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox