Patch "netfilter: nft_set_pipapo: do not free live element" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    netfilter: nft_set_pipapo: do not free live element

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     netfilter-nft_set_pipapo-do-not-free-live-element.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit f47c1acc43314a6675a44898f437a4b63e4029ce
Author: Florian Westphal <fw@xxxxxxxxx>
Date:   Wed Apr 10 21:05:13 2024 +0200

    netfilter: nft_set_pipapo: do not free live element
    
    [ Upstream commit 3cfc9ec039af60dbd8965ae085b2c2ccdcfbe1cc ]
    
    Pablo reports a crash with large batches of elements with a
    back-to-back add/remove pattern.  Quoting Pablo:
    
      add_elem("00000000") timeout 100 ms
      ...
      add_elem("0000000X") timeout 100 ms
      del_elem("0000000X") <---------------- delete one that was just added
      ...
      add_elem("00005000") timeout 100 ms
    
      1) nft_pipapo_remove() removes element 0000000X
      Then, KASAN shows a splat.
    
    Looking at the remove function there is a chance that we will drop a
    rule that maps to a non-deactivated element.
    
    Removal happens in two steps, first we do a lookup for key k and return the
    to-be-removed element and mark it as inactive in the next generation.
    Then, in a second step, the element gets removed from the set/map.
    
    The _remove function does not work correctly if we have more than one
    element that share the same key.
    
    This can happen if we insert an element into a set when the set already
    holds an element with same key, but the element mapping to the existing
    key has timed out or is not active in the next generation.
    
    In such case its possible that removal will unmap the wrong element.
    If this happens, we will leak the non-deactivated element, it becomes
    unreachable.
    
    The element that got deactivated (and will be freed later) will
    remain reachable in the set data structure, this can result in
    a crash when such an element is retrieved during lookup (stale
    pointer).
    
    Add a check that the fully matching key does in fact map to the element
    that we have marked as inactive in the deactivation step.
    If not, we need to continue searching.
    
    Add a bug/warn trap at the end of the function as well, the remove
    function must not ever be called with an invisible/unreachable/non-existent
    element.
    
    v2: avoid uneeded temporary variable (Stefano)
    
    Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
    Reported-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
    Reviewed-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
    Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
    Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 58eca26162735..2299ced939c47 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1994,6 +1994,8 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
 		rules_fx = rules_f0;
 
 		nft_pipapo_for_each_field(f, i, m) {
+			bool last = i == m->field_count - 1;
+
 			if (!pipapo_match_field(f, start, rules_fx,
 						match_start, match_end))
 				break;
@@ -2006,16 +2008,18 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
 
 			match_start += NFT_PIPAPO_GROUPS_PADDED_SIZE(f);
 			match_end += NFT_PIPAPO_GROUPS_PADDED_SIZE(f);
-		}
 
-		if (i == m->field_count) {
-			priv->dirty = true;
-			pipapo_drop(m, rulemap);
-			return;
+			if (last && f->mt[rulemap[i].to].e == e) {
+				priv->dirty = true;
+				pipapo_drop(m, rulemap);
+				return;
+			}
 		}
 
 		first_rule += rules_f0;
 	}
+
+	WARN_ON_ONCE(1); /* elem_priv not found */
 }
 
 /**




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux