lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 9 Jul 2018 00:28:54 +0000
From:   Jianbo Liu <jianbol@...lanox.com>
To:     "Zahari, Doychev" <zahari.doychev@...el.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jiri@...nulli.us,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v2 net-next 5/5] net/sched: flower: Add supprt for
 matching on QinQ vlan headers

The 07/08/2018 12:27, Zahari, Doychev wrote:
> On Fri, Jul 06, 2018 at 05:38:16AM +0000, Jianbo Liu wrote:
> > As support dissecting of QinQ inner and outer vlan headers, user can
> > add rules to match on QinQ vlan headers.
> > 
> > Signed-off-by: Jianbo Liu <jianbol@...lanox.com>
> > Acked-by: Jiri Pirko <jiri@...lanox.com>
> > ---
> >  include/uapi/linux/pkt_cls.h |  4 +++
> >  net/sched/cls_flower.c       | 65 ++++++++++++++++++++++++++++++++++----------
> >  2 files changed, 55 insertions(+), 14 deletions(-)
> > 
> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> > index 84e4c1d..c4262d9 100644
> > --- a/include/uapi/linux/pkt_cls.h
> > +++ b/include/uapi/linux/pkt_cls.h
> > @@ -469,6 +469,10 @@ enum {
> >  	TCA_FLOWER_KEY_IP_TTL,		/* u8 */
> >  	TCA_FLOWER_KEY_IP_TTL_MASK,	/* u8 */
> >  
> > +	TCA_FLOWER_KEY_CVLAN_ID,	/* be16 */
> > +	TCA_FLOWER_KEY_CVLAN_PRIO,	/* u8   */
> > +	TCA_FLOWER_KEY_CVLAN_ETH_TYPE,	/* be16 */
> > +
> >  	__TCA_FLOWER_MAX,
> >  };
> >  
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index e93b13d..487a152 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
> > @@ -35,6 +35,7 @@ struct fl_flow_key {
> >  	struct flow_dissector_key_basic basic;
> >  	struct flow_dissector_key_eth_addrs eth;
> >  	struct flow_dissector_key_vlan vlan;
> > +	struct flow_dissector_key_vlan cvlan;
> >  	union {
> >  		struct flow_dissector_key_ipv4_addrs ipv4;
> >  		struct flow_dissector_key_ipv6_addrs ipv6;
> > @@ -449,6 +450,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> >  	[TCA_FLOWER_KEY_IP_TOS_MASK]	= { .type = NLA_U8 },
> >  	[TCA_FLOWER_KEY_IP_TTL]		= { .type = NLA_U8 },
> >  	[TCA_FLOWER_KEY_IP_TTL_MASK]	= { .type = NLA_U8 },
> > +	[TCA_FLOWER_KEY_CVLAN_ID]	= { .type = NLA_U16 },
> > +	[TCA_FLOWER_KEY_CVLAN_PRIO]	= { .type = NLA_U8 },
> > +	[TCA_FLOWER_KEY_CVLAN_ETH_TYPE]	= { .type = NLA_U16 },
> >  };
> >  
> >  static void fl_set_key_val(struct nlattr **tb,
> > @@ -501,19 +505,20 @@ static int fl_set_key_mpls(struct nlattr **tb,
> >  
> >  static void fl_set_key_vlan(struct nlattr **tb,
> >  			    __be16 ethertype,
> > +			    int vlan_id_key, int vlan_prio_key,
> >  			    struct flow_dissector_key_vlan *key_val,
> >  			    struct flow_dissector_key_vlan *key_mask)
> >  {
> >  #define VLAN_PRIORITY_MASK	0x7
> >  
> > -	if (tb[TCA_FLOWER_KEY_VLAN_ID]) {
> > +	if (tb[vlan_id_key]) {
> >  		key_val->vlan_id =
> > -			nla_get_u16(tb[TCA_FLOWER_KEY_VLAN_ID]) & VLAN_VID_MASK;
> > +			nla_get_u16(tb[vlan_id_key]) & VLAN_VID_MASK;
> >  		key_mask->vlan_id = VLAN_VID_MASK;
> >  	}
> > -	if (tb[TCA_FLOWER_KEY_VLAN_PRIO]) {
> > +	if (tb[vlan_prio_key]) {
> >  		key_val->vlan_priority =
> > -			nla_get_u8(tb[TCA_FLOWER_KEY_VLAN_PRIO]) &
> > +			nla_get_u8(tb[vlan_prio_key]) &
> >  			VLAN_PRIORITY_MASK;
> >  		key_mask->vlan_priority = VLAN_PRIORITY_MASK;
> >  	}
> > @@ -596,11 +601,25 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> >  		ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
> >  
> >  		if (eth_type_vlan(ethertype)) {
> > -			fl_set_key_vlan(tb, ethertype, &key->vlan, &mask->vlan);
> > -			fl_set_key_val(tb, &key->basic.n_proto,
> > -				       TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > -				       &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
> > -				       sizeof(key->basic.n_proto));
> > +			fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID,
> > +					TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan,
> > +					&mask->vlan);
> > +
> > +			ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]);
> 
> Should we not check the nl attribute here? I am getting a crash when issuing the
> command below:
> 
> tc filter add dev ens6 ingress prio 1 protocol 802.1q \
> 	flower vlan_id 100 action vlan push id 200
> 

I'll send a fix for it. Thanks.

> [ 5015.082140] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
> [ 5015.085725] PGD 0 P4D 0
> [ 5015.086338] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 5015.087234] CPU: 0 PID: 277 Comm: tc Not tainted 4.18.0-rc3-ARCH+ #843
> [ 5015.088536] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org 04/01/2014
> [ 5015.090900] RIP: 0010:fl_change+0x108a/0x1d00
> [ 5015.091774] Code: 88 97 05 01 00 00 80 8c 24 cd 00 00 00 70 66 41 89 87 06 01 00 00 48 8b 83 c8 00 00 00 b9 ff ff ff
> ff 66 89 8c 24 ce 00 00 00 <0f> b7 40 04 66 3d 81 00 74 0a 66 3d 88 a8 0f 85 23 f3 ff ff 48 8b
> [ 5015.095507] RSP: 0018:ffffc900004cb7b0 EFLAGS: 00010246
> [ 5015.096199] RAX: 0000000000000000 RBX: ffff88003ce06c00 RCX: 00000000ffffffff
> [ 5015.097121] RDX: 0000000000000000 RSI: ffff88003bcdaafe RDI: ffff88003ce06c00
> [ 5015.098034] RBP: ffffc900004cb858 R08: 0000000000000007 R09: 0000000000000006
> [ 5015.099102] R10: ffffffff822e66e0 R11: 0000000000000000 R12: ffff88003bcdaae0
> [ 5015.100173] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88003bcdaa00
> [ 5015.101245] FS:  00007f6d8ff66c00(0000) GS:ffff88003f600000(0000) knlGS:0000000000000000
> [ 5015.102447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 5015.103297] CR2: 0000000000000004 CR3: 000000003c0a2000 CR4: 00000000000006f0
> [ 5015.104348] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 5015.105333] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 5015.106246] Call Trace:
> [ 5015.106585]  tc_new_tfilter+0x3fa/0x640
> [ 5015.107090]  rtnetlink_rcv_msg+0x126/0x390
> [ 5015.107626]  ? rtnl_calcit.isra.13+0x110/0x110
> [ 5015.108311]  netlink_rcv_skb+0x4c/0x120
> [ 5015.108898]  netlink_unicast+0x196/0x240
> [ 5015.109496]  netlink_sendmsg+0x1f7/0x3c0
> [ 5015.110093]  sock_sendmsg+0x19/0x30
> [ 5015.110626]  ___sys_sendmsg+0x295/0x2f0
> [ 5015.111211]  ? filemap_map_pages+0x19d/0x350
> [ 5015.111863]  ? __handle_mm_fault+0xed2/0x1240
> [ 5015.112517]  __sys_sendmsg+0x57/0xa0
> [ 5015.113057]  do_syscall_64+0x4e/0x100
> [ 5015.113612]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 5015.114361] RIP: 0033:0x7f6d8eecd984
> [ 5015.114917] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 48 8d 05 41 1e 2c 00 8b 00 85 c0
> 75 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 41 89 d4 55 48 89 f5 53
> [ 5015.117748] RSP: 002b:00007ffe110cfce8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 5015.118895] RAX: ffffffffffffffda RBX: 000000005b41e4c8 RCX: 00007f6d8eecd984
> [ 5015.119968] RDX: 0000000000000000 RSI: 00007ffe110cfd50 RDI: 0000000000000003
> [ 5015.121041] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
> [ 5015.122107] R10: fffffffffffff78c R11: 0000000000000246 R12: 0000000000000001
> [ 5015.123156] R13: 000055d3bc89c760 R14: 0000000000010081 R15: 00007ffe110d4008
> [ 5015.124205] Modules linked in: joydev mousedev bochs_drm ppdev nls_iso8859_1 ttm nls_cp437 vfat fat drm_kms_helper
> evdev input_leds led_class pcspkr psmouse mac_hid drm parport_pc syscopyarea parport sysfillrect sysimgblt fb_sys_fops
> qemu_fw_cfg intel_agp intel_gtt ia
> [ 5015.129877] CR2: 0000000000000004
> [ 5015.130630] ---[ end trace e68994d35a01ff0f ]---
> 
> Is is possible to check for both TPIDs 802.1ad and 802.1q when the vlan_ethtype
> is not set?
> 
> Regards
> Zahari
> 
> > +			if (eth_type_vlan(ethertype)) {
> > +				fl_set_key_vlan(tb, ethertype,
> > +						TCA_FLOWER_KEY_CVLAN_ID,
> > +						TCA_FLOWER_KEY_CVLAN_PRIO,
> > +						&key->cvlan, &mask->cvlan);
> > +				fl_set_key_val(tb, &key->basic.n_proto,
> > +					       TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> > +					       &mask->basic.n_proto,
> > +					       TCA_FLOWER_UNSPEC,
> > +					       sizeof(key->basic.n_proto));
> > +			} else {
> > +				key->basic.n_proto = ethertype;
> > +				mask->basic.n_proto = cpu_to_be16(~0);
> > +			}
> >  		} else {
> >  			key->basic.n_proto = ethertype;
> >  			mask->basic.n_proto = cpu_to_be16(~0);
> > @@ -826,6 +845,8 @@ static void fl_init_dissector(struct fl_flow_mask *mask)
> >  	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> >  			     FLOW_DISSECTOR_KEY_VLAN, vlan);
> >  	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> > +			     FLOW_DISSECTOR_KEY_CVLAN, cvlan);
> > +	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> >  			     FLOW_DISSECTOR_KEY_ENC_KEYID, enc_key_id);
> >  	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> >  			     FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS, enc_ipv4);
> > @@ -1201,6 +1222,7 @@ static int fl_dump_key_ip(struct sk_buff *skb,
> >  }
> >  
> >  static int fl_dump_key_vlan(struct sk_buff *skb,
> > +			    int vlan_id_key, int vlan_prio_key,
> >  			    struct flow_dissector_key_vlan *vlan_key,
> >  			    struct flow_dissector_key_vlan *vlan_mask)
> >  {
> > @@ -1209,13 +1231,13 @@ static int fl_dump_key_vlan(struct sk_buff *skb,
> >  	if (!memchr_inv(vlan_mask, 0, sizeof(*vlan_mask)))
> >  		return 0;
> >  	if (vlan_mask->vlan_id) {
> > -		err = nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ID,
> > +		err = nla_put_u16(skb, vlan_id_key,
> >  				  vlan_key->vlan_id);
> >  		if (err)
> >  			return err;
> >  	}
> >  	if (vlan_mask->vlan_priority) {
> > -		err = nla_put_u8(skb, TCA_FLOWER_KEY_VLAN_PRIO,
> > +		err = nla_put_u8(skb, vlan_prio_key,
> >  				 vlan_key->vlan_priority);
> >  		if (err)
> >  			return err;
> > @@ -1310,13 +1332,28 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
> >  	if (fl_dump_key_mpls(skb, &key->mpls, &mask->mpls))
> >  		goto nla_put_failure;
> >  
> > -	if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
> > +	if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_VLAN_ID,
> > +			     TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan, &mask->vlan))
> >  		goto nla_put_failure;
> >  
> > -	if (mask->vlan.vlan_tpid &&
> > -	    nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, key->basic.n_proto))
> > +	if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_CVLAN_ID,
> > +			     TCA_FLOWER_KEY_CVLAN_PRIO,
> > +			     &key->cvlan, &mask->cvlan) ||
> > +	    (mask->cvlan.vlan_tpid &&
> > +	     nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > +			 key->cvlan.vlan_tpid)))
> >  		goto nla_put_failure;
> >  
> > +	if (mask->cvlan.vlan_tpid) {
> > +		if (nla_put_be16(skb, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> > +				 key->basic.n_proto))
> > +			goto nla_put_failure;
> > +	} else if (mask->vlan.vlan_tpid) {
> > +		if (nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > +				 key->basic.n_proto))
> > +			goto nla_put_failure;
> > +	}
> > +
> >  	if ((key->basic.n_proto == htons(ETH_P_IP) ||
> >  	     key->basic.n_proto == htons(ETH_P_IPV6)) &&
> >  	    (fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> > -- 
> > 2.9.5
> > 

-- 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ