* [PATCH net v2] openvswitch: Fix conntrack cache with timeout
@ 2019-08-22 20:17 Yi-Hung Wei
2019-08-23 6:53 ` Pravin Shelar
2019-08-25 21:49 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Yi-Hung Wei @ 2019-08-22 20:17 UTC (permalink / raw)
To: netdev, pshelar; +Cc: Yi-Hung Wei
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@intel.com>
Signed-off-by: Yi-Hung Wei <yihung.wei@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
@@ -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);
+
}
if (helper) {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
2019-08-22 20:17 [PATCH net v2] openvswitch: Fix conntrack cache with timeout Yi-Hung Wei
@ 2019-08-23 6:53 ` Pravin Shelar
2019-08-23 16:40 ` Yi-Hung Wei
2019-08-25 21:49 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Pravin Shelar @ 2019-08-23 6:53 UTC (permalink / raw)
To: Yi-Hung Wei; +Cc: Linux Kernel Network Developers
On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung.wei@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 <lkp@intel.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@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
> @@ -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?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
2019-08-23 6:53 ` Pravin Shelar
@ 2019-08-23 16:40 ` Yi-Hung Wei
2019-08-25 16:53 ` Pravin Shelar
0 siblings, 1 reply; 5+ messages in thread
From: Yi-Hung Wei @ 2019-08-23 16:40 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers
On Thu, Aug 22, 2019 at 11:51 PM Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung.wei@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 <lkp@intel.com>
> > Signed-off-by: Yi-Hung Wei <yihung.wei@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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
2019-08-23 16:40 ` Yi-Hung Wei
@ 2019-08-25 16:53 ` Pravin Shelar
0 siblings, 0 replies; 5+ messages in thread
From: Pravin Shelar @ 2019-08-25 16:53 UTC (permalink / raw)
To: Yi-Hung Wei; +Cc: Linux Kernel Network Developers
On Fri, Aug 23, 2019 at 9:40 AM Yi-Hung Wei <yihung.wei@gmail.com> wrote:
>
> On Thu, Aug 22, 2019 at 11:51 PM Pravin Shelar <pshelar@ovn.org> wrote:
> >
> > On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung.wei@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 <lkp@intel.com>
> > > Signed-off-by: Yi-Hung Wei <yihung.wei@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().
>
Sounds good.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Thanks,
Pravin.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
2019-08-22 20:17 [PATCH net v2] openvswitch: Fix conntrack cache with timeout Yi-Hung Wei
2019-08-23 6:53 ` Pravin Shelar
@ 2019-08-25 21:49 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2019-08-25 21:49 UTC (permalink / raw)
To: yihung.wei; +Cc: netdev, pshelar
From: Yi-Hung Wei <yihung.wei@gmail.com>
Date: Thu, 22 Aug 2019 13:17:50 -0700
> 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@intel.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-25 21:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-22 20:17 [PATCH net v2] openvswitch: Fix conntrack cache with timeout Yi-Hung Wei
2019-08-23 6:53 ` Pravin Shelar
2019-08-23 16:40 ` Yi-Hung Wei
2019-08-25 16:53 ` Pravin Shelar
2019-08-25 21:49 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox