nft: Dubious code in get_set_decompose() of src/segtree.c

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

 



Hi Pablo,

When dealing with a covscan report for nft, I was pointed at the loop's
else-clause of get_set_decompose() as it overwrites 'left' without
freeing it first. The code in question is this:

| list_for_each_entry_safe(i, next, &set->init->expressions, list) {
|         if (i->flags & EXPR_F_INTERVAL_END && left) {
|                 list_del(&left->list);
|                 list_del(&i->list);
|                 mpz_sub_ui(i->key->value, i->key->value, 1);
|                 new = range_expr_alloc(&internal_location, left, i);
|                 compound_expr_add(new_init, new);
|                 left = NULL;
|         } else {
|                 if (left) {
|                         left = get_set_interval_end(table,
|                                                     set->handle.set,
|                                                     left);
|                         compound_expr_add(new_init, left);
|                 }
|                 left = i;
|         }
| }

IIUC, the else-clause should be hit for non-interval-end items, and the
call to get_set_interval_end() happens if we have two consecutive
non-interval-end items. I tried to trigger it, but failed, hence I
wonder if this situation is possible at all. Do you perhaps remember why
you put the code like this when implementing 'get element' command
(commit a43cc8d53096d)?

Thanks, Phil



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux