netfilter buglog - Oct 2017 - [Bug 1199] New: nft_set_hash fast lookup broken for 2 byte keys

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

            Bug ID: 1199
           Summary: nft_set_hash fast lookup broken for 2 byte keys
           Product: nftables
           Version: unspecified
          Hardware: x86_64
                OS: All
            Status: NEW
          Severity: major
          Priority: P5
         Component: kernel
          Assignee: pablo at netfilter.org
          Reporter: makovick at gmail.com

Hi,

after an upgrade of the userspace nftables to v0.8, I found that my port sets
suddenly stopped matching. After some searching, I tracked the issue down to
nft_hash_key and nft_hash_lookup_fast - with v0.8, the kernel started
preferring hashes instead of bitsets.

nft_hash_lookup_fast uses jhash_1word, which always uses the hash initializer
appropriate for keys of length == 4. This means it miscomputes the bucket
location for 2-byte keys and the lookup fails. In addition, lookups for 4-byte
keys will probably fail on big endian machines - the byte-wise jhash used
everywhere else reads the u32s as litle endian.

I tried removing the fast version of the hash lookups, and it indeed fixes the
issue.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171028/16de9c81/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

--- Comment #1 from Jindřich Makovička <makovick at gmail.com> ---
When searching for this bug, I also found this piece of code in 
nft_hash_deactivate():

    hlist_for_each_entry(he, &priv->table[hash], node) {
        if (!memcmp(nft_set_ext_key(&this->ext), &elem->key.val,
                set->klen) ||
            nft_set_elem_active(&he->ext, genmask)) {
            nft_set_elem_change_active(net, set, &he->ext);
            return he;
        }
    }

The logical OR looks fishy to me. Shouldn't be && there instead?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171029/22802ce1/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

Florian Westphal <fw at strlen.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fw at strlen.de
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from Florian Westphal <fw at strlen.de> ---
Fixed via

commit 0414c78f14861cb704d6e6888efd53dd36e3bdde
netfilter: nft_set_hash: disable fast_ops for 2-len keys

in nf/net trees.

I submitted a patch to address || vs &&, thanks for reporting!

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171102/516989c2/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

--- Comment #3 from Jindřich Makovička <makovick at gmail.com> ---
Thanks, I think the 4 byte version should be also disabled for BE archs though.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171102/ac08b990/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

--- Comment #4 from Florian Westphal <fw at strlen.de> ---
(In reply to Jindřich Makovička from comment #3)
> Thanks, I think the 4 byte version should be also disabled for BE archs
> though.
Right, I missed the comment wrt. jhash_1word vs. jhash() on big endian.
I will look at it again, but I think that it might not be worth fixing this
so removing the "fast" versions seems better.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171102/96348198/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1199

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |DUPLICATE

--- Comment #5 from Pablo Neira Ayuso <pablo at netfilter.org> ---
Fix for this bug is now available in Linux kernel release 4.13.13.

*** This bug has been marked as a duplicate of bug 1201 ***

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20171115/e4cfa96d/attachment.html>