netdev - Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout

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: <CAOrHB_A6Hn9o=8uzHQTp=cttMQsf=dYpobvq7C7_W398sw8UJA@mail.gmail.com>
Date:   Thu, 22 Aug 2019 23:53:48 -0700
From:   Pravin Shelar <pshelar@....org>
To:     Yi-Hung Wei <yihung.wei@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout

On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung.wei@...il.com> wrote:
>
> This patch addresses a conntrack cache issue with timeout policy.
> Currently, we do not check if the timeout extension is set properly in the
> cached conntrack entry.  Thus, after packet recirculate from conntrack
> action, the timeout policy is not applied properly.  This patch fixes the
> aforementioned issue.
>
> Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
> Reported-by: kbuild test robot <lkp@...el.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@...il.com>
> ---
> v1->v2: Fix rcu dereference issue reported by kbuild test robot.
> ---
>  net/openvswitch/conntrack.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 848c6eb55064..4d7896135e73 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -67,6 +67,7 @@ struct ovs_conntrack_info {
>         struct md_mark mark;
>         struct md_labels labels;
>         char timeout[CTNL_TIMEOUT_NAME_MAX];
> +       struct nf_ct_timeout *nf_ct_timeout;
>  #if IS_ENABLED(CONFIG_NF_NAT)
>         struct nf_nat_range2 range;  /* Only present for SRC NAT and DST NAT. */
>  #endif
> @@ -697,6 +698,14 @@ static bool skb_nfct_cached(struct net *net,
>                 if (help && rcu_access_pointer(help->helper) != info->helper)
>                         return false;
>         }
> +       if (info->nf_ct_timeout) {
> +               struct nf_conn_timeout *timeout_ext;
> +
> +               timeout_ext = nf_ct_timeout_find(ct);
> +               if (!timeout_ext || info->nf_ct_timeout !=
> +                   rcu_dereference(timeout_ext->timeout))
> +                       return false;
> +       }
>         /* Force conntrack entry direction to the current packet? */
>         if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
>                 /* Delete the conntrack entry if confirmed, else just release
> @@ -1657,6 +1666,10 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr,
>                                       ct_info.timeout))
>                         pr_info_ratelimited("Failed to associated timeout "
>                                             "policy `%s'\n", ct_info.timeout);
> +               else
> +                       ct_info.nf_ct_timeout = rcu_dereference(
> +                               nf_ct_timeout_find(ct_info.ct)->timeout);
Is this dereference safe from NULL pointer?