[PATCH net] net/sched: cls_flower: validate mask pointer after nla_next()

[PATCH net] net/sched: cls_flower: validate mask pointer after nla_next()

[Aohan Mei]

From: Aohan Mei <henrymei@tencent.com>

fl_set_enc_opt() iterates the key's nested tunnel-option attributes
with nla_for_each_attr() while advancing a single mask pointer via
nla_next() at the bottom of each loop, so the mask cursor is driven
by the number of key attributes rather than by the mask's own
attributes.  The nla_ok() added by commit c96adff956191 ("cls_flower:
call nla_ok() before nla_next()") only validates the mask pointer
that was just consumed; the pointer produced by nla_next() is used by
the next iteration (fl_set_geneve_opt() and siblings) without any
validation.

The mask's nested attributes are validated with NL_VALIDATE_LIBERAL,
which merely warns on trailing bytes that do not form a complete
attribute.  A mask carrying one valid attribute plus 1-3 residue
bytes (or a non-aligned attribute length making msk_depth negative)
therefore reaches the next iteration with msk_depth != 0, so neither
the !msk_depth check in fl_set_enc_opt() nor the !depth check in the
per-type helpers fires.  nla_type() then reads past the mask payload
and nla_parse_nested_deprecated() iterates with an nla_len taken
from those bytes, reading well beyond the mask attribute (KASAN:
slab-out-of-bounds read in __nla_validate_parse from fl_change()).

Validate the advanced mask pointer as well: when the mask is not
legitimately exhausted (msk_depth != 0) and the new pointer fails
nla_ok(), reject the filter with -EINVAL.  An exactly exhausted mask
still skips the check, preserving exact-match behaviour for the
remaining key attributes.

Fixes: c96adff95619 ("cls_flower: call nla_ok() before nla_next()")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---
 net/sched/cls_flower.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 0e275b58151c..1cefea571efd 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1703,6 +1703,11 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key,
 			return -EINVAL;
 		}
 		nla_opt_msk = nla_next(nla_opt_msk, &msk_depth);
+
+		if (msk_depth && !nla_ok(nla_opt_msk, msk_depth)) {
+			NL_SET_ERR_MSG(extack, "A mask attribute is invalid");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
-- 
2.43.7

Re: [PATCH net] net/sched: cls_flower: validate mask pointer after nla_next()

[Jamal Hadi Salim]

On Tue, Aug 25, 2026 at 10:51 PM Aohan Mei <ljp1205831794@gmail.com> wrote:
>
> From: Aohan Mei <henrymei@tencent.com>
>
> fl_set_enc_opt() iterates the key's nested tunnel-option attributes
> with nla_for_each_attr() while advancing a single mask pointer via
> nla_next() at the bottom of each loop, so the mask cursor is driven
> by the number of key attributes rather than by the mask's own
> attributes.  The nla_ok() added by commit c96adff956191 ("cls_flower:
> call nla_ok() before nla_next()") only validates the mask pointer
> that was just consumed; the pointer produced by nla_next() is used by
> the next iteration (fl_set_geneve_opt() and siblings) without any
> validation.
>
> The mask's nested attributes are validated with NL_VALIDATE_LIBERAL,
> which merely warns on trailing bytes that do not form a complete
> attribute.  A mask carrying one valid attribute plus 1-3 residue
> bytes (or a non-aligned attribute length making msk_depth negative)
> therefore reaches the next iteration with msk_depth != 0, so neither
> the !msk_depth check in fl_set_enc_opt() nor the !depth check in the
> per-type helpers fires.  nla_type() then reads past the mask payload
> and nla_parse_nested_deprecated() iterates with an nla_len taken
> from those bytes, reading well beyond the mask attribute (KASAN:
> slab-out-of-bounds read in __nla_validate_parse from fl_change()).
>
> Validate the advanced mask pointer as well: when the mask is not
> legitimately exhausted (msk_depth != 0) and the new pointer fails
> nla_ok(), reject the filter with -EINVAL.  An exactly exhausted mask
> still skips the check, preserving exact-match behaviour for the
> remaining key attributes.
>

Hi,
In the future, can you please send a reproducer, such as a simple tdc
test. We test all tc patches.
I think you can easily add a TDC test for this specific patch and it
may even be worth resending to have the tdc test included.
If the reproducer is sensitive, don't cc the liists but send it to maintainers,

The patch looks sane. I was going to suggest using strict mode but
noticed the fixes commit states it intetionaly avoided strict mode
with claim it will break uapi.
One nit: the commit message says "call nla_ok() before nla_next()" in
the Fixes:  but noting that that commit moved nla_next() to the loop
bottom and added the pre-consumption nla_ok(), but the
post-advancement pointer was left unchecked. Your description is
accurate; just double check if you got the right Fixes..

cheers,
jamal
> Fixes: c96adff95619 ("cls_flower: call nla_ok() before nla_next()")
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Cc: stable@vger.kernel.org
> Assisted-by: CodeBuddy:Kimi-K3
> Signed-off-by: Aohan Mei <henrymei@tencent.com>
> ---
>  net/sched/cls_flower.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 0e275b58151c..1cefea571efd 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1703,6 +1703,11 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key,
>                         return -EINVAL;
>                 }
>                 nla_opt_msk = nla_next(nla_opt_msk, &msk_depth);
> +
> +               if (msk_depth && !nla_ok(nla_opt_msk, msk_depth)) {
> +                       NL_SET_ERR_MSG(extack, "A mask attribute is invalid");
> +                       return -EINVAL;
> +               }
>         }
>
>         return 0;
> --
> 2.43.7
>

Re: [PATCH net] net/sched: cls_flower: validate mask pointer after nla_next()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Aug 2026 10:51:20 +0800 you wrote:
> From: Aohan Mei <henrymei@tencent.com>
> 
> fl_set_enc_opt() iterates the key's nested tunnel-option attributes
> with nla_for_each_attr() while advancing a single mask pointer via
> nla_next() at the bottom of each loop, so the mask cursor is driven
> by the number of key attributes rather than by the mask's own
> attributes.  The nla_ok() added by commit c96adff956191 ("cls_flower:
> call nla_ok() before nla_next()") only validates the mask pointer
> that was just consumed; the pointer produced by nla_next() is used by
> the next iteration (fl_set_geneve_opt() and siblings) without any
> validation.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: cls_flower: validate mask pointer after nla_next()
    https://git.kernel.org/netdev/net/c/fee10655709c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html