netdev - Re: [PATCH net v2] ovs: add recursion limit to ovs_vport

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] [day] [month] [year] [list]

Message-ID: <CAOrHB_A+eXb648odrGZ4Z_kehi+zRcPk9UOpbhSA-CbioS+JAw@mail.gmail.com>
Date:	Thu, 14 Jan 2016 22:30:18 -0800
From:	pravin shelar <pshelar@....org>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	ovs dev <dev@...nvswitch.org>
Subject: Re: [PATCH net v2] ovs: add recursion limit to ovs_vport_receive

On Thu, Jan 14, 2016 at 3:05 PM, Hannes Frederic Sowa
<hannes@...essinduktion.org> wrote:
> It was seen that defective configurations of openvswitch could overwrite
> the STACK_END_MAGIC and cause a hard crash of the kernel because of too
> many recursions within ovs.
>
> This problem arises due to the high stack usage of openvswitch. The rest
> of the kernel is fine with the current limit of 10 (RECURSION_LIMIT).
> Thus add an extra recursion limit counter for ovs_vport_receive until
> parts of the stack usage is moved to percpu scratch space.
>
> Cc: Pravin Shelar <pshelar@....org>
> Signed-off-by: Hannes Frederic Sowa <hannes@...essinduktion.org>
> ---
> v2) add preemption protection
>
>  net/openvswitch/vport.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 31cbc8c5c7db82..238fe435ca5877 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -426,6 +426,9 @@ u32 ovs_vport_find_upcall_portid(const struct vport *vport, struct sk_buff *skb)
>         return ids->ids[ids_index];
>  }
>
> +static DEFINE_PER_CPU(int, ovs_recursion);
> +static const int ovs_recursion_limit = 8;
> +

I am not sure if 8 is right limit. In some cases, like ipsec, nfs, we
could have really deep stack, So to be conservative, the limit could
be 5.

>  /**
>   *     ovs_vport_receive - pass up received packet to the datapath for processing
>   *
> @@ -442,6 +445,15 @@ int ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
>         struct sw_flow_key key;
>         int error;
>
> +       preempt_disable();
> +       if (__this_cpu_inc_return(ovs_recursion) > ovs_recursion_limit) {
> +               net_crit_ratelimited("ovs: recursion limit reached on datapath %s, probable configuration error\n",
> +                                    ovs_dp_name(vport->dp));
> +               error = -ENETDOWN;
> +               kfree_skb(skb);
> +               goto out;
> +       }
> +
There is already execution level counter in ovs_execute_actions(). We
can use that to check for the limit rather than adding another
counter.