[PATCH net+iproute2 0/2] nbyte, cmp and text filter fixups

STALE3126d

Revision v1 of 2 in this series.

7 messages, 3 authors, 2018-01-24 · open the first message on its own page

[PATCH net+iproute2 0/2] nbyte, cmp and text filter fixups

From: Wolfgang Bumiller <hidden>
Date: 2018-01-18 10:32:39

The iproute2 part allows the the actual use of the already existing
quoted string parsing.

The kernel side fixes an oob read in em_nbyte and allows 'layer 0' in
cmp and nbyte (and em_text whose existence surprised me given that I did
not see it exposed via iproute2) to actually match layer 0 rather than
being the same as specifying layer 1.

I seem to have stumbled upon a layer of dust (says git-blame).
Trying to match mac addresses I felt that the examples found online
using the 'u32' filter were rather inconvenient, particularly given
that there's the 'nbyte' filter around that could just memcmp the
entire a byte sequence at once.

Wolfgang Bumiller (1; 2):
  tc/lexer: let quotes actually start strings

 tc/emp_ematch.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

  net: sched: em_nbyte: don't add the data offset twice
  net_sched: fix TCF_LAYER_LINK case in tcf_get_base_ptr

 include/net/pkt_cls.h | 2 +-
 net/sched/em_nbyte.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.11.0

[PATCH net 1/2] net: sched: em_nbyte: don't add the data offset twice

From: Wolfgang Bumiller <hidden>
Date: 2018-01-18 10:32:39

'ptr' is shifted by the offset and then validated,
the memcmp should not add it a second time.

Signed-off-by: Wolfgang Bumiller <redacted>
---
 net/sched/em_nbyte.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/em_nbyte.c b/net/sched/em_nbyte.c
index df3110d69585..07c10bac06a0 100644
--- a/net/sched/em_nbyte.c
+++ b/net/sched/em_nbyte.c
@@ -47,15 +47,15 @@ static int em_nbyte_match(struct sk_buff *skb, struct tcf_ematch *em,
 	unsigned char *ptr = tcf_get_base_ptr(skb, nbyte->hdr.layer);
 
 	ptr += nbyte->hdr.off;
 
 	if (!tcf_valid_offset(skb, ptr, nbyte->hdr.len))
 		return 0;
 
-	return !memcmp(ptr + nbyte->hdr.off, nbyte->pattern, nbyte->hdr.len);
+	return !memcmp(ptr, nbyte->pattern, nbyte->hdr.len);
 }
 
 static struct tcf_ematch_ops em_nbyte_ops = {
 	.kind	  = TCF_EM_NBYTE,
 	.change	  = em_nbyte_change,
 	.match	  = em_nbyte_match,
 	.owner	  = THIS_MODULE,
-- 
2.11.0

[PATCH net 2/2] net_sched: fix TCF_LAYER_LINK case in tcf_get_base_ptr

From: Wolfgang Bumiller <hidden>
Date: 2018-01-18 10:32:39

TCF_LAYER_LINK and TCF_LAYER_NETWORK returned the same pointer as
skb->data points to the network header.
Use skb_mac_header instead.

Signed-off-by: Wolfgang Bumiller <redacted>
---
Alternatively this could return skb->head directly, but
'sk_buff->mac_header' is documented as 'Link layer header' and this
seemed more clear. Since on the first read I thought "it looks fine"
while in fact skb->head comes before skb->data, so this seems less
confusing.

 include/net/pkt_cls.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 8e08b6da72f3..753ac9361154 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -522,7 +522,7 @@ static inline unsigned char * tcf_get_base_ptr(struct sk_buff *skb, int layer)
 {
 	switch (layer) {
 		case TCF_LAYER_LINK:
-			return skb->data;
+			return skb_mac_header(skb);
 		case TCF_LAYER_NETWORK:
 			return skb_network_header(skb);
 		case TCF_LAYER_TRANSPORT:
-- 
2.11.0

[PATCH iproute2] tc/lexer: let quotes actually start strings

From: Wolfgang Bumiller <hidden>
Date: 2018-01-18 10:32:40

The lexer will go with the longest match, so previously
the starting double quotes of a string would be swallowed by
the [^ \t\r\n()]+ pattern leaving the user no way to
actually use strings with escape sequences.
Fix this by not allowing this case to start with double
quotes.

Signed-off-by: Wolfgang Bumiller <redacted>
---
 tc/emp_ematch.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/emp_ematch.l b/tc/emp_ematch.l
index dc106759..d7a99304 100644
--- a/tc/emp_ematch.l
+++ b/tc/emp_ematch.l
@@ -137,7 +137,7 @@
 ")"					{
 						return yylval.i = *yytext;
 					}
-[^ \t\r\n()]+				{
+[^" \t\r\n()][^ \t\r\n()]*		{
 						yylval.b = bstr_alloc(yytext);
 						if (yylval.b == NULL)
 							return ERROR;
-- 
2.11.0

Re: [PATCH net+iproute2 0/2] nbyte, cmp and text filter fixups

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-01-18 10:59:42

Thu, Jan 18, 2018 at 11:32:33AM CET, w.bumiller@proxmox.com wrote:
The iproute2 part allows the the actual use of the already existing
quoted string parsing.
Makes no sense to send iproute2 patch which is not related with the
kernel ones in the same set. Please send it separatelly.
The kernel side fixes an oob read in em_nbyte and allows 'layer 0' in
cmp and nbyte (and em_text whose existence surprised me given that I did
not see it exposed via iproute2) to actually match layer 0 rather than
being the same as specifying layer 1.

I seem to have stumbled upon a layer of dust (says git-blame).
Trying to match mac addresses I felt that the examples found online
using the 'u32' filter were rather inconvenient, particularly given
that there's the 'nbyte' filter around that could just memcmp the
entire a byte sequence at once.

Wolfgang Bumiller (1; 2):
 tc/lexer: let quotes actually start strings

tc/emp_ematch.l | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

 net: sched: em_nbyte: don't add the data offset twice
 net_sched: fix TCF_LAYER_LINK case in tcf_get_base_ptr
"net: sched:" or "net_sched:"? - please, try to be consistent

Re: [PATCH net 1/2] net: sched: em_nbyte: don't add the data offset twice

From: David Miller <davem@davemloft.net>
Date: 2018-01-24 19:53:13

From: Wolfgang Bumiller <redacted>
Date: Thu, 18 Jan 2018 11:32:35 +0100
'ptr' is shifted by the offset and then validated,
the memcmp should not add it a second time.

Signed-off-by: Wolfgang Bumiller <redacted>
Applied.

Re: [PATCH net 2/2] net_sched: fix TCF_LAYER_LINK case in tcf_get_base_ptr

From: David Miller <davem@davemloft.net>
Date: 2018-01-24 19:53:27

From: Wolfgang Bumiller <redacted>
Date: Thu, 18 Jan 2018 11:32:36 +0100
TCF_LAYER_LINK and TCF_LAYER_NETWORK returned the same pointer as
skb->data points to the network header.
Use skb_mac_header instead.

Signed-off-by: Wolfgang Bumiller <redacted>
Applied with Subject changed to be more consistent "net: sched:"
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help