[net,1/6] lib/ts_bm: reset initial match offset for every block of text

Commit Message

Pablo Neira Ayuso June 27, 2023, 6:52 a.m. UTC

From: Jeremy Sowden <[e-mail]>

The `shift` variable which indicates the offset in the string at which
to start matching the pattern is initialized to `bm->patlen - 1`, but it
is not reset when a new block is retrieved.  This means the implemen-
tation may start looking at later and later positions in each successive
block and miss occurrences of the pattern at the beginning.  E.g.,
consider a HTTP packet held in a non-linear skb, where the HTTP request
line occurs in the second block:

  [... 52 bytes of packet headers ...]
  GET /bmtest HTTP/1.1\r\nHost: www.example.com\r\n\r\n

and the pattern is "GET /bmtest".

Once the first block comprising the packet headers has been examined,
`shift` will be pointing to somewhere near the end of the block, and so
when the second block is examined the request line at the beginning will
be missed.

Reinitialize the variable for each new block.

Fixes: 8082e4ed0a61 ("[LIB]: Boyer-Moore extension for textsearch infrastructure strike #2")
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1390
Signed-off-by: Jeremy Sowden <[e-mail]>
Signed-off-by: Pablo Neira Ayuso <[e-mail]>
---
 lib/ts_bm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

[e-mail] June 27, 2023, 11 a.m. UTC | #1

Hello:

This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <[e-mail]>:

On Tue, 27 Jun 2023 08:52:59 +0200 you wrote:
> From: Jeremy Sowden <[e-mail]>
> 
> The `shift` variable which indicates the offset in the string at which
> to start matching the pattern is initialized to `bm->patlen - 1`, but it
> is not reset when a new block is retrieved.  This means the implemen-
> tation may start looking at later and later positions in each successive
> block and miss occurrences of the pattern at the beginning.  E.g.,
> consider a HTTP packet held in a non-linear skb, where the HTTP request
> line occurs in the second block:
> 
> [...]

Here is the summary with links:
  - [net,1/6] lib/ts_bm: reset initial match offset for every block of text
    https://git.kernel.org/netdev/net/c/6f67fbf8192d
  - [net,2/6] netfilter: conntrack: dccp: copy entire header to stack buffer, not just basic one
    https://git.kernel.org/netdev/net/c/ff0a3a7d52ff
  - [net,3/6] linux/netfilter.h: fix kernel-doc warnings
    https://git.kernel.org/netdev/net/c/f18e7122cc73
  - [net,4/6] netfilter: nf_conntrack_sip: fix the ct_sip_parse_numerical_param() return value.
    https://git.kernel.org/netdev/net/c/f188d3008748
  - [net,5/6] netfilter: nf_tables: unbind non-anonymous set if rule construction fails
    https://git.kernel.org/netdev/net/c/3e70489721b6
  - [net,6/6] netfilter: nf_tables: fix underflow in chain reference counter
    https://git.kernel.org/netdev/net/c/b389139f12f2

You are awesome, thank you!

Patch

diff --git a/lib/ts_bm.c b/lib/ts_bm.c
index 1f2234221dd1..c8ecbf74ef29 100644
--- a/lib/ts_bm.c
+++ b/lib/ts_bm.c
@@ -60,10 +60,12 @@  static unsigned int bm_find(struct ts_config *conf, struct ts_state *state)
 	struct ts_bm *bm = ts_config_priv(conf);
 	unsigned int i, text_len, consumed = state->offset;
 	const u8 *text;
-	int shift = bm->patlen - 1, bs;
+	int bs;
 	const u8 icase = conf->flags & TS_IGNORECASE;
 
 	for (;;) {
+		int shift = bm->patlen - 1;
+
 		text_len = conf->get_next_block(consumed, &text, conf, state);
 
 		if (unlikely(text_len == 0))