mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Zhiqi Zhang <zhiqizhangg@gmail.com>
Cc: linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Finke Lennart <lfinke@student.ethz.ch>,
	netfilter-devel@vger.kernel.org
Subject: Re: [PATCH] lib/ts_fsm: preserve token types when exporting patterns
Date: Sat, 31 Oct 2026 20:22:11 -0700	[thread overview]
Message-ID: <20261031202211.22f18bca7819dbe2d107d17d@linux-foundation.org> (raw)
In-Reply-To: <20261031052307.23533-1-zhiqizhangg@gmail.com>

On Sat, 31 Oct 2026 13:23:07 +0800 Zhiqi Zhang <zhiqizhangg@gmail.com> wrote:

> fsm_init() replaces each token type with an internal character-class mask.
> fsm_get_pattern() then returns these masks as if they were public token
> types. Exporting a pattern and preparing another FSM configuration from
> it can therefore change the matching behaviour or fail with -EINVAL.
> 
> For example, a single TS_FSM_DIGIT token (type 2) becomes the _D mask (4).
> Preparing the exported pattern interprets 4 as TS_FSM_PRINT, so the new
> configuration accepts "A" while the original rejects it. The text ematch
> dump path uses textsearch_get_pattern() to return patterns to userspace.
> 
> Keep the public token types in the configuration and look up their masks
> in match_token(). This preserves the exported representation without
> adding another pattern allocation. Initialization still validates every
> type before it can be used to index token_map.
> 
> Add KUnit coverage for exporting and recreating all token types and a
> sequence with a repeated character class. Also check that exporting and
> recreating the pattern leaves the original configuration usable.
> 

The main user of textsearch appears to be netfilter, so please let's
keep the relevant developers in the loop.

netfilter peeps: I'm seeing a small number of textsearch patches lately
and I'm totally unfamiliar with it.  Can you please propose a
MAINTAINERS patch?

hp2:/usr/src/mm> scripts/get_maintainer.pl -f --nogit --nogit-fallback --no-rolestats lib/ts_bm.c lib/ts_fsm.c lib/ts_kmp.c  
Andrew Morton <akpm@linux-foundation.org>
linux-kernel@vger.kernel.org

Lonely!




Zhiqi, a thing which is missing from the changelog, and it's really
important, is a description of the userspace-visible runtime effects of
the bug.

> I identified this issue. OpenAI Codex helped confirm it and drafted the
> fix, KUnit tests and changelog. I reviewed the fix, built it and verified
> that the tests passed.

I'm suspecting "no such effects are known".  But that isn't enough -
once the issue is out there, LLM-using bad actors might be able to find
ways of exploiting the bug, so please discuss that aspect also.

My LLM suggested

: The userspace consequence is worth spelling out: dumping and restoring
: a traffic-control rule can silently change which packets it matches, or
: fail to restore the rule with EINVAL.

Also, and fyi, it told me

: Related finding: BM/KMP also export transformed patterns. With
: TS_IGNORECASE, repeated folding of 0xff changes it again
: (ff -> df -> bf). That's a separate API roundtrip problem, but I found
: no current userspace dump/restore path exposing it: em_text doesn't
: enable that flag, and xt_string retains the original pattern.

Also, regarding your patch:

: One small test improvement: after copying the exported pattern, add
: KUNIT_EXPECT_MEMEQ(test, exported, pattern, len). That directly checks
: the exported representation as well as its matching behavior.

And I'm thinking that a cc:stable is justified here.

Thanks.

> ---
> Validation in isolated QEMU TCG guests:
> - ARM64 GCC 13.3: the same KUnit test and config change from 2/15 passing
>   before the fix to 15/15 passing afterwards.
> - ARM32 GCC 13.3 and ARM64 Clang 18.1.3: 15/15 passing.
> - An ARM64 rtnetlink smoke test creates a basic classifier with a text
>   ematch, dumps it, restores its payload, and dumps it again. The DIGIT
>   token changes 2 -> 4 -> 151 before the fix, and stays 2 afterwards.
> - W=1 for the changed objects: no warnings. Sparse reports no diagnostics
>   for these objects (an unchanged ARM64 vDSO object has a declaration
>   warning). Strict checkpatch passes after suppressing the new-file
>   MAINTAINERS reminder.
> 
> Packet traffic, module loading and throughput were not tested. Resolving
> masks in match_token() adds a lookup on nonliteral matches.
> 
>  lib/Kconfig.debug                |  14 ++++
>  lib/tests/Makefile               |   1 +
>  lib/tests/textsearch_fsm_kunit.c | 137 +++++++++++++++++++++++++++++++
>  lib/ts_fsm.c                     |   7 +-
>  4 files changed, 153 insertions(+), 6 deletions(-)
>  create mode 100644 lib/tests/textsearch_fsm_kunit.c
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2d22638ec..416d033b0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2531,6 +2531,20 @@ config STRING_HELPERS_KUNIT_TEST
>  	depends on KUNIT
>  	default KUNIT_ALL_TESTS
>  
> +config TEXTSEARCH_FSM_KUNIT_TEST
> +	tristate "KUnit tests for FSM text search" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	select TEXTSEARCH
> +	select TEXTSEARCH_FSM
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Test that exporting and recreating an FSM text search configuration
> +	  preserves its matching behaviour. Exercise each public token type
> +	  and a sequence of tokens with a repeated character class. Check
> +	  that exporting also leaves the original configuration usable.
> +
> +	  If unsure, say N.
> +
>  config STRING_KUNIT_BENCH
>         bool "Benchmark string functions at runtime"
>         depends on STRING_KUNIT_TEST
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index df0537e1b..fa47ef5d3 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -54,6 +54,7 @@ CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
>  obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
>  obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
>  obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
> +obj-$(CONFIG_TEXTSEARCH_FSM_KUNIT_TEST) += textsearch_fsm_kunit.o
>  obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
>  obj-$(CONFIG_UTIL_MACROS_KUNIT) += util_macros_kunit.o
>  obj-$(CONFIG_RATELIMIT_KUNIT_TEST) += test_ratelimit.o
> diff --git a/lib/tests/textsearch_fsm_kunit.c b/lib/tests/textsearch_fsm_kunit.c
> new file mode 100644
> index 000000000..bbf944ef8
> --- /dev/null
> +++ b/lib/tests/textsearch_fsm_kunit.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Tests for exporting and recreating FSM text search configurations. */
> +
> +#include <kunit/test.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/textsearch.h>
> +#include <linux/textsearch_fsm.h>
> +
> +struct fsm_test_case {
> +	const char *name;
> +	u16 type;
> +	const char *match;
> +	const char *miss;
> +};
> +
> +static const struct fsm_test_case fsm_test_cases[] = {
> +	{ "specific", TS_FSM_SPECIFIC, "x", "y" },
> +	{ "wildcard", TS_FSM_WILDCARD, "A", "" },
> +	{ "digit", TS_FSM_DIGIT, "7", "A" },
> +	{ "xdigit", TS_FSM_XDIGIT, "f", "g" },
> +	{ "print", TS_FSM_PRINT, "A", "\n" },
> +	{ "alpha", TS_FSM_ALPHA, "A", "7" },
> +	{ "alnum", TS_FSM_ALNUM, "7", "!" },
> +	{ "ascii", TS_FSM_ASCII, "A", "\200" },
> +	{ "cntrl", TS_FSM_CNTRL, "\n", "A" },
> +	{ "graph", TS_FSM_GRAPH, "!", " " },
> +	{ "lower", TS_FSM_LOWER, "a", "A" },
> +	{ "upper", TS_FSM_UPPER, "A", "a" },
> +	{ "punct", TS_FSM_PUNCT, "!", "A" },
> +	{ "space", TS_FSM_SPACE, " ", "A" },
> +};
> +
> +static void fsm_case_to_desc(const struct fsm_test_case *params, char *desc)
> +{
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", params->name);
> +}
> +
> +KUNIT_ARRAY_PARAM(fsm, fsm_test_cases, fsm_case_to_desc);
> +
> +static void fsm_destroy_config(void *conf)
> +{
> +	textsearch_destroy(conf);
> +}
> +
> +static void fsm_expect_matches(struct kunit *test, struct ts_config *conf,
> +			       const struct fsm_test_case *params)
> +{
> +	struct ts_state state = {};
> +	unsigned int pos;
> +
> +	pos = textsearch_find_continuous(conf, &state, params->match,
> +					 strlen(params->match));
> +	KUNIT_EXPECT_EQ(test, pos, 0U);
> +	pos = textsearch_find_continuous(conf, &state, params->miss,
> +					 strlen(params->miss));
> +	KUNIT_EXPECT_EQ(test, pos, UINT_MAX);
> +}
> +
> +static void fsm_check_roundtrip(struct kunit *test,
> +				const struct ts_fsm_token *pattern,
> +				unsigned int len,
> +				const struct fsm_test_case *params)
> +{
> +	struct ts_fsm_token *exported;
> +	struct ts_config *conf, *recreated;
> +	int err;
> +
> +	conf = textsearch_prepare("fsm", pattern, len, GFP_KERNEL, TS_AUTOLOAD);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conf);
> +	err = kunit_add_action_or_reset(test, fsm_destroy_config, conf);
> +	KUNIT_ASSERT_EQ(test, err, 0);
> +	fsm_expect_matches(test, conf, params);
> +
> +	KUNIT_ASSERT_EQ(test, textsearch_get_pattern_len(conf), len);
> +	exported = kunit_kmalloc(test, len, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, exported);
> +	memcpy(exported, textsearch_get_pattern(conf), len);
> +
> +	/* Exporting a pattern must preserve the existing search behaviour. */
> +	fsm_expect_matches(test, conf, params);
> +
> +	recreated = textsearch_prepare("fsm", exported, len, GFP_KERNEL,
> +				       TS_AUTOLOAD);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, recreated,
> +					 "exported token: type=%u recur=%u value=%u",
> +					 exported->type, exported->recur,
> +					 exported->value);
> +	err = kunit_add_action_or_reset(test, fsm_destroy_config, recreated);
> +	KUNIT_ASSERT_EQ(test, err, 0);
> +	fsm_expect_matches(test, recreated, params);
> +	fsm_expect_matches(test, conf, params);
> +}
> +
> +static void fsm_test_pattern_roundtrip(struct kunit *test)
> +{
> +	const struct fsm_test_case *params = test->param_value;
> +	struct ts_fsm_token pattern = {
> +		.type = params->type,
> +		.recur = TS_FSM_SINGLE,
> +		.value = 'x',
> +	};
> +
> +	fsm_check_roundtrip(test, &pattern, sizeof(pattern), params);
> +}
> +
> +static void fsm_test_sequence_roundtrip(struct kunit *test)
> +{
> +	static const struct ts_fsm_token pattern[] = {
> +		{ TS_FSM_DIGIT, TS_FSM_MULTI, 0 },
> +		{ TS_FSM_SPECIFIC, TS_FSM_SINGLE, ':' },
> +		{ TS_FSM_UPPER, TS_FSM_SINGLE, 0 },
> +	};
> +	static const struct fsm_test_case params = {
> +		.match = "77:A",
> +		.miss = "77:a",
> +	};
> +
> +	fsm_check_roundtrip(test, pattern, sizeof(pattern), &params);
> +}
> +
> +static struct kunit_case fsm_kunit_test_cases[] = {
> +	KUNIT_CASE_PARAM(fsm_test_pattern_roundtrip, fsm_gen_params),
> +	KUNIT_CASE(fsm_test_sequence_roundtrip),
> +	{}
> +};
> +
> +static struct kunit_suite fsm_test_suite = {
> +	.name = "textsearch-fsm",
> +	.test_cases = fsm_kunit_test_cases,
> +};
> +
> +kunit_test_suite(fsm_test_suite);
> +
> +MODULE_DESCRIPTION("FSM text search configuration tests");
> +MODULE_LICENSE("GPL");
> diff --git a/lib/ts_fsm.c b/lib/ts_fsm.c
> index 053615f4f..8725ea6f0 100644
> --- a/lib/ts_fsm.c
> +++ b/lib/ts_fsm.c
> @@ -126,7 +126,7 @@ _W|_L,         _W|_L,        _W|_L,        _W|_L};		/* 252-255 */
>  static inline int match_token(struct ts_fsm_token *t, u8 d)
>  {
>  	if (t->type)
> -		return (token_lookup_tbl[d] & t->type) != 0;
> +		return (token_lookup_tbl[d] & token_map[t->type]) != 0;
>  	else
>  		return t->value == d;
>  }
> @@ -288,11 +288,6 @@ static struct ts_config *fsm_init(const void *pattern, unsigned int len,
>  	fsm->ntokens = ntokens;
>  	memcpy(fsm->tokens, pattern, len);
>  
> -	for (i = 0; i < fsm->ntokens; i++) {
> -		struct ts_fsm_token *t = &fsm->tokens[i];
> -		t->type = token_map[t->type];
> -	}
> -
>  	return conf;
>  
>  errout:
> 
> base-commit: 2a500a04d6f969318c05f46aedb6e73fc1963a00
> -- 
> 2.54.0 (Apple Git-157)

  reply	other threads:[~2026-11-01  3:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-31  5:23 Zhiqi Zhang
2026-11-01  3:22 ` Andrew Morton [this message]
2026-11-01  8:17   ` Zhiqi Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261031202211.22f18bca7819dbe2d107d17d@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=lfinke@student.ethz.ch \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=zhiqizhangg@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®