netdev - Re: [PATCH net] net/sched: flower: Ensure both minimum and maximum ports are specified

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM0EoMmo_96JkDzoS3GgnHxTCSiz5UnfkB6PYjEq01oMBU68wA@mail.gmail.com>
Date: Tue, 11 Jul 2023 17:13:08 -0400
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Ido Schimmel <idosch@...dia.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org, 
	edumazet@...gle.com, pabeni@...hat.com, xiyou.wangcong@...il.com, 
	jiri@...nulli.us, amritha.nambiar@...el.com, petrm@...dia.com
Subject: Re: [PATCH net] net/sched: flower: Ensure both minimum and maximum
 ports are specified

On Tue, Jul 11, 2023 at 3:08 AM Ido Schimmel <idosch@...dia.com> wrote:
>
> The kernel does not currently validate that both the minimum and maximum
> ports of a port range are specified. This can lead user space to think
> that a filter matching on a port range was successfully added, when in
> fact it was not. For example, with a patched (buggy) iproute2 that only
> sends the minimum port, the following commands do not return an error:
>
>  # tc filter add dev swp1 ingress pref 1 proto ip flower ip_proto udp src_port 100-200 action pass
>
>  # tc filter add dev swp1 ingress pref 1 proto ip flower ip_proto udp dst_port 100-200 action pass
>
>  # tc filter show dev swp1 ingress
>  filter protocol ip pref 1 flower chain 0
>  filter protocol ip pref 1 flower chain 0 handle 0x1
>    eth_type ipv4
>    ip_proto udp
>    not_in_hw
>          action order 1: gact action pass
>           random type none pass val 0
>           index 1 ref 1 bind 1
>
>  filter protocol ip pref 1 flower chain 0 handle 0x2
>    eth_type ipv4
>    ip_proto udp
>    not_in_hw
>          action order 1: gact action pass
>           random type none pass val 0
>           index 2 ref 1 bind 1
>
> Fix by returning an error unless both ports are specified:
>
>  # tc filter add dev swp1 ingress pref 1 proto ip flower ip_proto udp src_port 100-200 action pass
>  Error: Both min and max source ports must be specified.
>  We have an error talking to the kernel
>
>  # tc filter add dev swp1 ingress pref 1 proto ip flower ip_proto udp dst_port 100-200 action pass
>  Error: Both min and max destination ports must be specified.
>  We have an error talking to the kernel
>
> Fixes: 5c72299fba9d ("net: sched: cls_flower: Classify packets using port ranges")
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> Reviewed-by: Petr Machata <petrm@...dia.com>

Acked-by: Jamal Hadi Salim <jhs@...atatu.com>

cheers,
jamal
> ---
>  net/sched/cls_flower.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 56065cc5a661..f2b0bc4142fe 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -812,6 +812,16 @@ static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
>                        TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_range.tp_max.src,
>                        TCA_FLOWER_UNSPEC, sizeof(key->tp_range.tp_max.src));
>
> +       if (mask->tp_range.tp_min.dst != mask->tp_range.tp_max.dst) {
> +               NL_SET_ERR_MSG(extack,
> +                              "Both min and max destination ports must be specified");
> +               return -EINVAL;
> +       }
> +       if (mask->tp_range.tp_min.src != mask->tp_range.tp_max.src) {
> +               NL_SET_ERR_MSG(extack,
> +                              "Both min and max source ports must be specified");
> +               return -EINVAL;
> +       }
>         if (mask->tp_range.tp_min.dst && mask->tp_range.tp_max.dst &&
>             ntohs(key->tp_range.tp_max.dst) <=
>             ntohs(key->tp_range.tp_min.dst)) {
> --
> 2.40.1
>