netdev - Re: [PATCH v2 net] net: sched: correct flower port blocking

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: <20200216.191837.828352407289487240.davem@davemloft.net>
Date:   Sun, 16 Feb 2020 19:18:37 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     jbaron@...mai.com
Cc:     jiri@...nulli.us, xiyou.wangcong@...il.com, jhs@...atatu.com,
        netdev@...r.kernel.org, soukjin.bae@...sung.com,
        edumazet@...gle.com
Subject: Re: [PATCH v2 net] net: sched: correct flower port blocking

From: David Miller <davem@...emloft.net>
Date: Sun, 16 Feb 2020 19:08:12 -0800 (PST)

> From: Jason Baron <jbaron@...mai.com>
> Date: Fri, 14 Feb 2020 11:20:24 -0500
> 
>> tc flower rules that are based on src or dst port blocking are sometimes
>> ineffective due to uninitialized stack data. __skb_flow_dissect() extracts
>> ports from the skb for tc flower to match against. However, the port
>> dissection is not done when when the FLOW_DIS_IS_FRAGMENT bit is set in
>> key_control->flags. All callers of __skb_flow_dissect(), zero-out the
>> key_control field except for fl_classify() as used by the flower
>> classifier. Thus, the FLOW_DIS_IS_FRAGMENT may be set on entry to
>> __skb_flow_dissect(), since key_control is allocated on the stack
>> and may not be initialized.
>> 
>> Since key_basic and key_control are present for all flow keys, let's
>> make sure they are initialized.
>> 
>> Fixes: 62230715fd24 ("flow_dissector: do not dissect l4 ports for fragments")
>> Co-developed-by: Eric Dumazet <edumazet@...gle.com>
>> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>> Acked-by: Cong Wang <xiyou.wangcong@...il.com>
>> Signed-off-by: Jason Baron <jbaron@...mai.com>
> 
> Applied and queued up for -stable.

Actually this doesn't even compile:

In file included from drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:33:
./include/net/flow_dissector.h: In function ‘flow_dissector_init_keys’:
./include/net/flow_dissector.h:355:2: error: implicit declaration of function ‘memset’ [-Werror=implicit-function-declaration]
  memset(key_control, 0, sizeof(*key_control));
  ^~~~~~
./include/net/flow_dissector.h:355:2: warning: incompatible implicit declaration of built-in function ‘memset’
./include/net/flow_dissector.h:355:2: note: include ‘<string.h>’ or provide a declaration of ‘memset’
./include/net/flow_dissector.h:9:1:
+#include <string.h>
 
./include/net/flow_dissector.h:355:2:
  memset(key_control, 0, sizeof(*key_control));
  ^~~~~~