netdev - Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <69957353-7fe0-9faa-4ddd-1ac44d5386a5@thelounge.net>
Date:   Fri, 5 Feb 2021 12:33:27 +0100
From:   Reindl Harald <h.reindl@...lounge.net>
To:     Pablo Neira Ayuso <pablo@...filter.org>,
        netfilter-devel@...r.kernel.org
Cc:     davem@...emloft.net, netdev@...r.kernel.org, kuba@...nel.org
Subject: Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update
 deleted entry


thank you for adressing that issue - maybe GRO can be enabled and wasn't 
involved at all

"Reap only entries which won't be updated" sounds for me like the could 
be some optimization: i mean when you first update and then check what 
can be reaped the recently updated entry would not match to begin with

Am 05.02.21 um 01:17 schrieb Pablo Neira Ayuso:
> From: Jozsef Kadlecsik <kadlec@...l.kfki.hu>
> 
> When both --reap and --update flag are specified, there's a code
> path at which the entry to be updated is reaped beforehand,
> which then leads to kernel crash. Reap only entries which won't be
> updated.
> 
> Fixes kernel bugzilla #207773.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=207773
> Reported-by: Reindl Harald <h.reindl@...lounge.net>
> Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
> Signed-off-by: Jozsef Kadlecsik <kadlec@...filter.org>
> Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>
> ---
>   net/netfilter/xt_recent.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> index 606411869698..0446307516cd 100644
> --- a/net/netfilter/xt_recent.c
> +++ b/net/netfilter/xt_recent.c
> @@ -152,7 +152,8 @@ static void recent_entry_remove(struct recent_table *t, struct recent_entry *e)
>   /*
>    * Drop entries with timestamps older then 'time'.
>    */
> -static void recent_entry_reap(struct recent_table *t, unsigned long time)
> +static void recent_entry_reap(struct recent_table *t, unsigned long time,
> +			      struct recent_entry *working, bool update)
>   {
>   	struct recent_entry *e;
>   
> @@ -161,6 +162,12 @@ static void recent_entry_reap(struct recent_table *t, unsigned long time)
>   	 */
>   	e = list_entry(t->lru_list.next, struct recent_entry, lru_list);
>   
> +	/*
> +	 * Do not reap the entry which are going to be updated.
> +	 */
> +	if (e == working && update)
> +		return;
> +
>   	/*
>   	 * The last time stamp is the most recent.
>   	 */
> @@ -303,7 +310,8 @@ recent_mt(const struct sk_buff *skb, struct xt_action_param *par)
>   
>   		/* info->seconds must be non-zero */
>   		if (info->check_set & XT_RECENT_REAP)
> -			recent_entry_reap(t, time);
> +			recent_entry_reap(t, time, e,
> +				info->check_set & XT_RECENT_UPDATE && ret);
>   	}
>   
>   	if (info->check_set & XT_RECENT_SET ||