Re: [PATCH] net/sched: fix cls_flow SKUID/SKGID userns mask

[Paolo Abeni <pabeni@redhat.com>]

On 11/3/26 18:32, shaos wrote:
> Hello,
> 
> flow_change() checks FLOW_KEY_SKUID and FLOW_KEY_SKGID as bit values,
> although they are enum indices and TCA_FLOW_KEYS is a bitmask.  The
> current expression tests bits 0 through 3 instead of bits 14 and 15.
> 
> A process with CAP_NET_ADMIN in a non-initial user namespace can
> therefore install SKUID/SKGID filters despite the user-namespace gate.
> Those keys use initial-user-namespace credentials.  If another user's
> locally generated traffic traverses the attacker's qdisc, the filter
> can expose a property of that UID through class packet counters.  The
> same typo also rejects some ordinary flow keys.  The demonstrated
> cross-namespace inference requires such foreign traffic to reach the
> attacker-owned qdisc; this does not disclose an arbitrary UID or grant
> privileges.
> 
> HEAD commit:    c03c1a14507dfaf8b856c2608f9ac216021fc222
> Git tree:       30fd2ef82912c26cf3a58ad0ba6afc375fa851ac
> Source version: Linux 7.3
> Compiler:       gcc 11.4.0, GNU ld 2.38
> 
> The console log, full kernel config, and reproducer are available from
> one fixed-revision Gist:
> 
> Console output:
> https://gist.githubusercontent.com/shaos26/a5571f1bfcd37c68c7270d296f1c12c0/raw/b3acb75e61d1cb703c7488ce0400651cd8c84b16/console-output.txt
> Kernel config:
> https://gist.githubusercontent.com/shaos26/a5571f1bfcd37c68c7270d296f1c12c0/raw/b3acb75e61d1cb703c7488ce0400651cd8c84b16/kernel-config.txt
> PoC source:
> https://gist.githubusercontent.com/shaos26/a5571f1bfcd37c68c7270d296f1c12c0/raw/b3acb75e61d1cb703c7488ce0400651cd8c84b16/poc.c
> 
> Clone this Gist and the exact Linux source revision, then build the
> kernel with the attached config and run the PoC as PID 1 in an
> initramfs:
> 
>      git clone \
>        https://gist.github.com/shaos26/a5571f1bfcd37c68c7270d296f1c12c0.git \
>        cls-flow-repro
>      cd cls-flow-repro
>      git clone \
>        https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git \
>        linux
>      git -C linux checkout c03c1a14507dfaf8b856c2608f9ac216021fc222
>      cp kernel-config.txt linux/.config
>      make -C linux olddefconfig
>      make -C linux -j"$(nproc)" bzImage
>      gcc -static -O2 -Wall -Wextra -Werror -o init poc.c
>      mkdir -p initramfs/dev initramfs/proc initramfs/sys
>      cp init initramfs/init
>      (cd initramfs && find . -print0 | \
>       cpio --null -ov --format=newc | gzip -1 > ../flow-oracle.cpio.gz)
>      timeout 180 qemu-system-x86_64 -m 768M -smp 2 \
>        -kernel linux/arch/x86/boot/bzImage -initrd flow-oracle.cpio.gz \
>        -append 'console=ttyS0 panic=-1 nokaslr rdinit=/init oracle_uid=424243' \
>        -nographic -no-reboot > /tmp/flow-oracle-positive.log 2>&1
> 
> Change oracle_uid to 424242 for the even-UID control.  Run the same
> command with the corrected-mask kernel to check the fixed behavior.
> 
> The vulnerable kernel accepted both restricted filters in two fresh
> boots.  UID 424243 produced a 7/10 packet split between bands 1 and 2;
> changing only the target to even UID 424242 put all 17 packets in band
> 1.  The corrected-mask kernel rejected SKUID and SKGID with -EOPNOTSUPP,
> accepted the ordinary SRC key, and showed all 17 packets in band 2.
> The fixed-kernel harness prints ORACLE_RESULT=FAIL intentionally,
> because its predicate expects the vulnerable behavior.
> 
> The guard was added by commit a6c6796c7127
> ("userns: Convert cls_flow to work with user namespaces enabled").
> Use shifted enum values when checking the TCA_FLOW_KEYS bitmask:

The changelog is not really suitable for inclusion. Please rewrite it
avoid colloquial language, omitting the whole repro stuff and using
imperative mood consistently.
> 
> Fixes: a6c6796c7127 ("userns: Convert cls_flow to work with user namespaces enabled")
> Signed-off-by: shaos <shawnq1@163.com>

We need a real full name in the SoB.

> ---
>   net/sched/cls_flow.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
> --- a/net/sched/cls_flow.c
> +++ b/net/sched/cls_flow.c
> @@ -433,7 +433,8 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
>   		if (fls(keymask) - 1 > FLOW_KEY_MAX)
>   			return -EOPNOTSUPP;
>   
> -		if ((keymask & (FLOW_KEY_SKUID|FLOW_KEY_SKGID)) &&
> +		if ((keymask & ((1U << FLOW_KEY_SKUID) |
> +				(1U << FLOW_KEY_SKGID))) &&

BIT()?

>   		    sk_user_ns(NETLINK_CB(in_skb).sk) != &init_user_ns)
>   			return -EOPNOTSUPP;
>   	}