* [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it
@ 2021-04-11 8:13 Roi Dayan
2021-04-11 10:58 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Roi Dayan @ 2021-04-11 8:13 UTC (permalink / raw)
To: netdev
Cc: Roi Dayan, Pablo Neira Ayuso, Oz Shlomo, Paul Blakey, Saeed Mahameed
It could be dst_cache was not set so check it's not null before using
it.
Fixes: 8b9229d15877 ("netfilter: flowtable: dst_check() from garbage collector path")
Signed-off-by: Roi Dayan <roid@nvidia.com>
---
net/netfilter/nf_flow_table_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 76573bae6664..e426077aaed1 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -410,6 +410,8 @@ static bool flow_offload_stale_dst(struct flow_offload_tuple *tuple)
if (tuple->xmit_type == FLOW_OFFLOAD_XMIT_NEIGH ||
tuple->xmit_type == FLOW_OFFLOAD_XMIT_XFRM) {
dst = tuple->dst_cache;
+ if (!dst)
+ return false;
if (!dst_check(dst, tuple->dst_cookie))
return true;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it
2021-04-11 8:13 [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it Roi Dayan
@ 2021-04-11 10:58 ` Pablo Neira Ayuso
2021-04-12 8:26 ` Roi Dayan
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-04-11 10:58 UTC (permalink / raw)
To: Roi Dayan; +Cc: netdev, Oz Shlomo, Paul Blakey, Saeed Mahameed
[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]
Hi Roi,
On Sun, Apr 11, 2021 at 11:13:34AM +0300, Roi Dayan wrote:
> It could be dst_cache was not set so check it's not null before using
> it.
Could you give a try to this fix?
net/sched/act_ct.c leaves the xmit_type as FLOW_OFFLOAD_XMIT_UNSPEC
since it does not cache a route.
Thanks.
> Fixes: 8b9229d15877 ("netfilter: flowtable: dst_check() from garbage collector path")
> Signed-off-by: Roi Dayan <roid@nvidia.com>
> ---
> net/netfilter/nf_flow_table_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index 76573bae6664..e426077aaed1 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -410,6 +410,8 @@ static bool flow_offload_stale_dst(struct flow_offload_tuple *tuple)
> if (tuple->xmit_type == FLOW_OFFLOAD_XMIT_NEIGH ||
> tuple->xmit_type == FLOW_OFFLOAD_XMIT_XFRM) {
> dst = tuple->dst_cache;
> + if (!dst)
> + return false;
> if (!dst_check(dst, tuple->dst_cookie))
> return true;
> }
> --
> 2.26.2
>
[-- Attachment #2: fix.patch --]
[-- Type: text/x-diff, Size: 1013 bytes --]
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 583b327d8fc0..9b42c6523b4d 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -90,7 +90,8 @@ enum flow_offload_tuple_dir {
#define FLOW_OFFLOAD_DIR_MAX IP_CT_DIR_MAX
enum flow_offload_xmit_type {
- FLOW_OFFLOAD_XMIT_NEIGH = 0,
+ FLOW_OFFLOAD_XMIT_UNSPEC = 0,
+ FLOW_OFFLOAD_XMIT_NEIGH,
FLOW_OFFLOAD_XMIT_XFRM,
FLOW_OFFLOAD_XMIT_DIRECT,
};
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 76573bae6664..ea23a36dc14e 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -130,6 +130,9 @@ static int flow_offload_fill_route(struct flow_offload *flow,
flow_tuple->dst_cache = dst;
flow_tuple->dst_cookie = flow_offload_dst_cookie(flow_tuple);
break;
+ case FLOW_OFFLOAD_XMIT_UNSPEC:
+ WARN_ON_ONCE(1);
+ break;
}
flow_tuple->xmit_type = route->tuple[dir].xmit_type;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it
2021-04-11 10:58 ` Pablo Neira Ayuso
@ 2021-04-12 8:26 ` Roi Dayan
2021-04-12 11:42 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Roi Dayan @ 2021-04-12 8:26 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netdev, Oz Shlomo, Paul Blakey, Saeed Mahameed
On 2021-04-11 1:58 PM, Pablo Neira Ayuso wrote:
> Hi Roi,
>
> On Sun, Apr 11, 2021 at 11:13:34AM +0300, Roi Dayan wrote:
>> It could be dst_cache was not set so check it's not null before using
>> it.
>
> Could you give a try to this fix?
>
> net/sched/act_ct.c leaves the xmit_type as FLOW_OFFLOAD_XMIT_UNSPEC
> since it does not cache a route.
>
> Thanks.
>
what do you mean? FLOW_OFFLOAD_XMIT_UNSPEC doesn't exists so default 0
is set.
do you suggest adding that enum option as 0?
this is the current xmit_type enum
enum flow_offload_xmit_type {
FLOW_OFFLOAD_XMIT_NEIGH = 0,
FLOW_OFFLOAD_XMIT_XFRM,
FLOW_OFFLOAD_XMIT_DIRECT,
};
>> Fixes: 8b9229d15877 ("netfilter: flowtable: dst_check() from garbage collector path")
>> Signed-off-by: Roi Dayan <roid@nvidia.com>
>> ---
>> net/netfilter/nf_flow_table_core.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
>> index 76573bae6664..e426077aaed1 100644
>> --- a/net/netfilter/nf_flow_table_core.c
>> +++ b/net/netfilter/nf_flow_table_core.c
>> @@ -410,6 +410,8 @@ static bool flow_offload_stale_dst(struct flow_offload_tuple *tuple)
>> if (tuple->xmit_type == FLOW_OFFLOAD_XMIT_NEIGH ||
>> tuple->xmit_type == FLOW_OFFLOAD_XMIT_XFRM) {
>> dst = tuple->dst_cache;
>> + if (!dst)
>> + return false;
>> if (!dst_check(dst, tuple->dst_cookie))
>> return true;
>> }
>> --
>> 2.26.2
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it
2021-04-12 8:26 ` Roi Dayan
@ 2021-04-12 11:42 ` Pablo Neira Ayuso
2021-04-13 7:58 ` Roi Dayan
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-04-12 11:42 UTC (permalink / raw)
To: Roi Dayan; +Cc: netdev, Oz Shlomo, Paul Blakey, Saeed Mahameed
On Mon, Apr 12, 2021 at 11:26:35AM +0300, Roi Dayan wrote:
>
>
> On 2021-04-11 1:58 PM, Pablo Neira Ayuso wrote:
> > Hi Roi,
> >
> > On Sun, Apr 11, 2021 at 11:13:34AM +0300, Roi Dayan wrote:
> > > It could be dst_cache was not set so check it's not null before using
> > > it.
> >
> > Could you give a try to this fix?
> >
> > net/sched/act_ct.c leaves the xmit_type as FLOW_OFFLOAD_XMIT_UNSPEC
> > since it does not cache a route.
> >
> > Thanks.
> >
>
> what do you mean? FLOW_OFFLOAD_XMIT_UNSPEC doesn't exists so default 0
> is set.
>
> do you suggest adding that enum option as 0?
Yes. This could be FLOW_OFFLOAD_XMIT_TC instead if you prefer.
enum flow_offload_xmit_type {
FLOW_OFFLOAD_XMIT_TC = 0,
FLOW_OFFLOAD_XMIT_NEIGH,
FLOW_OFFLOAD_XMIT_XFRM,
FLOW_OFFLOAD_XMIT_DIRECT,
};
so there is no need to check for no route in the
FLOW_OFFLOAD_XMIT_NEIGH case (it's assumed this type always has a
route).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it
2021-04-12 11:42 ` Pablo Neira Ayuso
@ 2021-04-13 7:58 ` Roi Dayan
0 siblings, 0 replies; 5+ messages in thread
From: Roi Dayan @ 2021-04-13 7:58 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netdev, Oz Shlomo, Paul Blakey, Saeed Mahameed
On 2021-04-12 2:42 PM, Pablo Neira Ayuso wrote:
> On Mon, Apr 12, 2021 at 11:26:35AM +0300, Roi Dayan wrote:
>>
>>
>> On 2021-04-11 1:58 PM, Pablo Neira Ayuso wrote:
>>> Hi Roi,
>>>
>>> On Sun, Apr 11, 2021 at 11:13:34AM +0300, Roi Dayan wrote:
>>>> It could be dst_cache was not set so check it's not null before using
>>>> it.
>>>
>>> Could you give a try to this fix?
>>>
>>> net/sched/act_ct.c leaves the xmit_type as FLOW_OFFLOAD_XMIT_UNSPEC
>>> since it does not cache a route.
>>>
>>> Thanks.
>>>
>>
>> what do you mean? FLOW_OFFLOAD_XMIT_UNSPEC doesn't exists so default 0
>> is set.
>>
>> do you suggest adding that enum option as 0?
>
> Yes. This could be FLOW_OFFLOAD_XMIT_TC instead if you prefer.
>
> enum flow_offload_xmit_type {
> FLOW_OFFLOAD_XMIT_TC = 0,
> FLOW_OFFLOAD_XMIT_NEIGH,
> FLOW_OFFLOAD_XMIT_XFRM,
> FLOW_OFFLOAD_XMIT_DIRECT,
> };
>
> so there is no need to check for no route in the
> FLOW_OFFLOAD_XMIT_NEIGH case (it's assumed this type always has a
> route).
>
thanks Pablo. were not sure I wanted to touch the enum.
I prefer unspec actually as you suggested initially.
it works fine by adding the enum.
i'll submit v2 with this suggestion.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-13 7:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-11 8:13 [PATCH net-next 1/1] netfilter: flowtable: Make sure dst_cache is valid before using it Roi Dayan
2021-04-11 10:58 ` Pablo Neira Ayuso
2021-04-12 8:26 ` Roi Dayan
2021-04-12 11:42 ` Pablo Neira Ayuso
2021-04-13 7:58 ` Roi Dayan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.