Skip to content

Commit ee51577

Browse files
committed
netfilter: nf_set_pipapo: fix initial map fill
jira VULN-42212 cve CVE-2024-57947 commit-author Florian Westphal <[email protected]> commit 791a615 The initial buffer has to be inited to all-ones, but it must restrict it to the size of the first field, not the total field size. After each round in the map search step, the result and the fill map are swapped, so if we have a set where f->bsize of the first element is smaller than m->bsize_max, those one-bits are leaked into future rounds result map. This makes pipapo find an incorrect matching results for sets where first field size is not the largest. Followup patch adds a test case to nft_concat_range.sh selftest script. Thanks to Stefano Brivio for pointing out that we need to zero out the remainder explicitly, only correcting memset() argument isn't enough. Fixes: 3c4287f ("nf_tables: Add set type for arbitrary concatenation of ranges") Reported-by: Yi Chen <[email protected]> Cc: Stefano Brivio <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Reviewed-by: Stefano Brivio <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 791a615) Signed-off-by: Marcin Wcisło <[email protected]>
1 parent 931e6a8 commit ee51577

File tree

3 files changed

+29
-6
lines changed

3 files changed

+29
-6
lines changed

net/netfilter/nft_set_pipapo.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ bool nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
432432
res_map = *raw_cpu_ptr(m->scratch) + (map_index ? m->bsize_max : 0);
433433
fill_map = *raw_cpu_ptr(m->scratch) + (map_index ? 0 : m->bsize_max);
434434

435-
memset(res_map, 0xff, m->bsize_max * sizeof(*res_map));
435+
pipapo_resmap_init(m, res_map);
436436

437437
nft_pipapo_for_each_field(f, i, m) {
438438
bool last = i == m->field_count - 1;
@@ -538,7 +538,7 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
538538
goto out;
539539
}
540540

541-
memset(res_map, 0xff, m->bsize_max * sizeof(*res_map));
541+
pipapo_resmap_init(m, res_map);
542542

543543
nft_pipapo_for_each_field(f, i, m) {
544544
bool last = i == m->field_count - 1;

net/netfilter/nft_set_pipapo.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,25 @@ static u64 pipapo_estimate_size(const struct nft_set_desc *desc)
279279
return size;
280280
}
281281

282+
/**
283+
* pipapo_resmap_init() - Initialise result map before first use
284+
* @m: Matching data, including mapping table
285+
* @res_map: Result map
286+
*
287+
* Initialize all bits covered by the first field to one, so that after
288+
* the first step, only the matching bits of the first bit group remain.
289+
*
290+
* If other fields have a large bitmap, set remainder of res_map to 0.
291+
*/
292+
static inline void pipapo_resmap_init(const struct nft_pipapo_match *m, unsigned long *res_map)
293+
{
294+
const struct nft_pipapo_field *f = m->f;
295+
int i;
296+
297+
for (i = 0; i < f->bsize; i++)
298+
res_map[i] = ULONG_MAX;
299+
300+
for (i = f->bsize; i < m->bsize_max; i++)
301+
res_map[i] = 0ul;
302+
}
282303
#endif /* _NFT_SET_PIPAPO_H */

net/netfilter/nft_set_pipapo_avx2.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,7 @@ static int nft_pipapo_avx2_lookup_8b_16(unsigned long *map, unsigned long *fill,
10391039

10401040
/**
10411041
* nft_pipapo_avx2_lookup_slow() - Fallback function for uncommon field sizes
1042+
* @mdata: Matching data, including mapping table
10421043
* @map: Previous match result, used as initial bitmap
10431044
* @fill: Destination bitmap to be filled with current match result
10441045
* @f: Field, containing lookup and mapping tables
@@ -1054,7 +1055,8 @@ static int nft_pipapo_avx2_lookup_8b_16(unsigned long *map, unsigned long *fill,
10541055
* Return: -1 on no match, rule index of match if @last, otherwise first long
10551056
* word index to be checked next (i.e. first filled word).
10561057
*/
1057-
static int nft_pipapo_avx2_lookup_slow(unsigned long *map, unsigned long *fill,
1058+
static int nft_pipapo_avx2_lookup_slow(const struct nft_pipapo_match *mdata,
1059+
unsigned long *map, unsigned long *fill,
10581060
const struct nft_pipapo_field *f,
10591061
int offset, const u8 *pkt,
10601062
bool first, bool last)
@@ -1065,7 +1067,7 @@ static int nft_pipapo_avx2_lookup_slow(unsigned long *map, unsigned long *fill,
10651067
lt += offset * NFT_PIPAPO_LONGS_PER_M256;
10661068

10671069
if (first)
1068-
memset(map, 0xff, bsize * sizeof(*map));
1070+
pipapo_resmap_init(mdata, map);
10691071

10701072
for (i = offset; i < bsize; i++) {
10711073
if (f->bb == 8)
@@ -1189,7 +1191,7 @@ bool nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
11891191
} else if (f->groups == 16) {
11901192
NFT_SET_PIPAPO_AVX2_LOOKUP(8, 16);
11911193
} else {
1192-
ret = nft_pipapo_avx2_lookup_slow(res, fill, f,
1194+
ret = nft_pipapo_avx2_lookup_slow(m, res, fill, f,
11931195
ret, rp,
11941196
first, last);
11951197
}
@@ -1205,7 +1207,7 @@ bool nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
12051207
} else if (f->groups == 32) {
12061208
NFT_SET_PIPAPO_AVX2_LOOKUP(4, 32);
12071209
} else {
1208-
ret = nft_pipapo_avx2_lookup_slow(res, fill, f,
1210+
ret = nft_pipapo_avx2_lookup_slow(m, res, fill, f,
12091211
ret, rp,
12101212
first, last);
12111213
}

0 commit comments

Comments
 (0)