On Thu, Aug 22, 2019 at 11:51 PM Pravin Shelar <pshe...@ovn.org> wrote:
>
> On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung....@gmail.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 <l...@intel.com>
> > Signed-off-by: Yi-Hung Wei <yihung....@gmail.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
> > @@ -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?
Hi Pravin,
Thanks for your review. I am not sure if
nf_ct_timeout_find(ct_info.ct) will return NULL in this case.
We only run into this statement when ct_info.timeout[0] is set, and it
is only set in parse_ct() when CONFIG_NF_CONNTRACK_TIMEOUT is
configured. Also, in this else condition the timeout extension is
supposed to be set properly by nf_ct_set_timeout().
Am I missing something?
Thanks,
-Yi-Hung
