* [PATCH] netfilter: xt_ecn: Add missing hotdrop mark.
@ 2017-07-29 10:33 Taehee Yoo
2017-07-31 18:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 3+ messages in thread
From: Taehee Yoo @ 2017-07-29 10:33 UTC (permalink / raw)
To: pablo, fw, netfilter-devel; +Cc: ap420073
If the netfilter can't get L4 header, the netfilter
marks hotdrop value. then {ip, ip6, arp, eb}t_do_table() drops
that packet immediately. but xt_ecn doesn't mark hotdrop value.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
net/netfilter/xt_ecn.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
index 3c831a8..c58db1d 100644
--- a/net/netfilter/xt_ecn.c
+++ b/net/netfilter/xt_ecn.c
@@ -37,8 +37,10 @@ static bool match_tcp(const struct sk_buff *skb, struct xt_action_param *par)
* be good citizens.
*/
th = skb_header_pointer(skb, par->thoff, sizeof(_tcph), &_tcph);
- if (th == NULL)
+ if (!th) {
+ par->hotdrop = true;
return false;
+ }
if (einfo->operation & XT_ECN_OP_MATCH_ECE) {
if (einfo->invert & XT_ECN_OP_MATCH_ECE) {
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: xt_ecn: Add missing hotdrop mark.
2017-07-29 10:33 [PATCH] netfilter: xt_ecn: Add missing hotdrop mark Taehee Yoo
@ 2017-07-31 18:20 ` Pablo Neira Ayuso
2017-08-01 2:57 ` Taehee Yoo
0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2017-07-31 18:20 UTC (permalink / raw)
To: Taehee Yoo; +Cc: fw, netfilter-devel
On Sat, Jul 29, 2017 at 07:33:00PM +0900, Taehee Yoo wrote:
> If the netfilter can't get L4 header, the netfilter
> marks hotdrop value. then {ip, ip6, arp, eb}t_do_table() drops
> that packet immediately. but xt_ecn doesn't mark hotdrop value.
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
> net/netfilter/xt_ecn.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
> index 3c831a8..c58db1d 100644
> --- a/net/netfilter/xt_ecn.c
> +++ b/net/netfilter/xt_ecn.c
> @@ -37,8 +37,10 @@ static bool match_tcp(const struct sk_buff *skb, struct xt_action_param *par)
> * be good citizens.
> */
> th = skb_header_pointer(skb, par->thoff, sizeof(_tcph), &_tcph);
> - if (th == NULL)
> + if (!th) {
> + par->hotdrop = true;
> return false;
> + }
Is it that we always - consistenly - drop packets that has no
information that we need.
I would say it's better to do this via policy, it's more flexible,
rather than assuming that accessing a packet that doesn't contain the
information that we need means a drop.
Another concern for me regarding this is the fact that probably this
has been the default behaviour for long time, if that's the case, I
would be reluctant to change this at this point.
Let me know, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: xt_ecn: Add missing hotdrop mark.
2017-07-31 18:20 ` Pablo Neira Ayuso
@ 2017-08-01 2:57 ` Taehee Yoo
0 siblings, 0 replies; 3+ messages in thread
From: Taehee Yoo @ 2017-08-01 2:57 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, Netfilter Developer Mailing List
2017-08-01 3:20 GMT+09:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Sat, Jul 29, 2017 at 07:33:00PM +0900, Taehee Yoo wrote:
>> If the netfilter can't get L4 header, the netfilter
>> marks hotdrop value. then {ip, ip6, arp, eb}t_do_table() drops
>> that packet immediately. but xt_ecn doesn't mark hotdrop value.
>>
>> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
>> ---
>> net/netfilter/xt_ecn.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
>> index 3c831a8..c58db1d 100644
>> --- a/net/netfilter/xt_ecn.c
>> +++ b/net/netfilter/xt_ecn.c
>> @@ -37,8 +37,10 @@ static bool match_tcp(const struct sk_buff *skb, struct xt_action_param *par)
>> * be good citizens.
>> */
>> th = skb_header_pointer(skb, par->thoff, sizeof(_tcph), &_tcph);
>> - if (th == NULL)
>> + if (!th) {
>> + par->hotdrop = true;
>> return false;
>> + }
>
> Is it that we always - consistenly - drop packets that has no
> information that we need.
>
Below match modules are mark hotdrop value when they can not get L4 header.
xt_tcpudp, xt_dccp, xt_sctp, xt_multiport, xt_tcpmss, xt_esp,
xt_ipcomp, xt_hashlimit, ipt_ah,
ip6t_rt, ip6t_frag, ip6t_mh, ip6t_hbh.
And Below match modules are do not mark hotdrop value when they can
not get L4 header.
xt_l2tp, xt_ecn, xt_osf.
So, we do not always mark hotdrop value.
> I would say it's better to do this via policy, it's more flexible,
> rather than assuming that accessing a packet that doesn't contain the
> information that we need means a drop.
>
I agree with your mention.
Already target functions drop packets If they can not get what we need.
So, It's working well even now.
> Another concern for me regarding this is the fact that probably this
> has been the default behaviour for long time, if that's the case, I
> would be reluctant to change this at this point.
>
Yes, It has been the default for long time. so, I agree with your decision.
> Let me know, thanks!
Thank you for your review!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-01 2:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-29 10:33 [PATCH] netfilter: xt_ecn: Add missing hotdrop mark Taehee Yoo
2017-07-31 18:20 ` Pablo Neira Ayuso
2017-08-01 2:57 ` Taehee Yoo
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.