[prev in list] [next in list] [prev in thread] [next in thread] 

List:       netfilter-devel
Subject:    Re: [PATCH] netfilter: xt_ecn: Add missing hotdrop mark.
From:       Pablo Neira Ayuso <pablo () netfilter ! org>
Date:       2017-07-31 18:20:15
Message-ID: 20170731182015.GA15201 () salvia
[Download RAW message or body]

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!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]