Re: [PATCH 1/4] treewide: convert ISO_8859-1 text comments to utf-8

[Arnd Bergmann]

tools/perf/tests/.gitignore:
LLVM byte-codes, uncompressed
On Wed, Jul 25, 2018 at 2:55 AM, Andrew Morton
 wrote:
> On Tue, 24 Jul 2018 17:13:20 -0700 Joe Perches  wrote:
>
>> On Tue, 2018-07-24 at 14:00 -0700, Andrew Morton wrote:
>> > On Tue, 24 Jul 2018 13:13:25 +0200 Arnd Bergmann  wrote:
>> > > Almost all files in the kernel are either plain text or UTF-8
>> > > encoded. A couple however are ISO_8859-1, usually just a few
>> > > characters in a C comments, for historic reasons.
>> > > This converts them all to UTF-8 for consistency.
>> []
>> > Will we be getting a checkpatch rule to keep things this way?
>>
>> How would that be done?
>
> I'm using this, seems to work.
>
> if ! file $p | grep -q -P ", ASCII text|, UTF-8 Unicode text"
> then
> echo $p: weird charset
> fi

There are a couple of files that my version of 'find' incorrectly identified as
something completely different, like:

Documentation/devicetree/bindings/pinctrl/pinctrl-sx150x.txt:
SemOne archive data
Documentation/devicetree/bindings/rtc/epson,rtc7301.txt:
Microsoft Document Imaging Format
Documentation/filesystems/nfs/pnfs-block-server.txt:
PPMN archive data
arch/arm/boot/dts/bcm283x-rpi-usb-host.dtsi:
Sendmail frozen configuration  - version = "host";
Documentation/networking/segmentation-offloads.txt:
StuffIt Deluxe Segment (data) : gmentation Offloads in the
Linux Networking Stack
arch/sparc/include/asm/visasm.h:  SAS 7+
arch/xtensa/kernel/setup.c: ,
init=0x454c, stat=0x090a, dev=0x2009, bas=0x2020
drivers/cpufreq/powernow-k8.c:
TI-XX Graphing Calculator (FLASH)
tools/testing/selftests/net/forwarding/tc_shblocks.sh:
Minix filesystem, V2 (big endian)
tools/perf/tests/.gitignore:
LLVM byte-codes, uncompressed

All of the above seem to be valid ASCII or UTF-8 files, so the check
above will lead
to false-positives, but it may be good enough as they are the
exception, and may be
bugs in 'file'.

Not sure if we need to worry about 'file' not being installed.

   Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: fix CONFIG_NF_REJECT_IPV6=m link error

[Arnd Bergmann]

On Fri, Apr 13, 2018 at 3:15 PM, Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> On Mon, Apr 09, 2018 at 04:43:40PM +0200, Arnd Bergmann wrote:
>> On Mon, Apr 9, 2018 at 4:37 PM, Pablo Neira Ayuso <pa...@netfilter.org> 
>> wrote:
>> > Hi Arnd,
>> >
>> > On Mon, Apr 09, 2018 at 12:53:12PM +0200, Arnd Bergmann wrote:
>> >> We get a new link error with CONFIG_NFT_REJECT_INET=y and 
>> >> CONFIG_NF_REJECT_IPV6=m
>> >
>> > I think we can update NFT_REJECT_INET so it depends on NFT_REJECT_IPV4
>> > and NFT_REJECT_IPV6. This doesn't allow here CONFIG_NFT_REJECT_INET=y
>> > and CONFIG_NF_REJECT_IPV6=m.
>> >
>> > I mean, just like we do with NFT_FIB_INET.
>>
>> That can only work if NFT_REJECT_INET can be made a 'tristate' symbol
>> again, so that code gets built as a loadable module if
>> CONFIG_NF_REJECT_IPV6=m.
>>
>> > BTW, I think this problem has been is not related to the recent patch,
>> > but something older that kbuild robot has triggered more easily for
>> > some reason?
>>
>> 02c7b25e5f54 is the one that turned NF_TABLES_INET into a 'bool'
>> symbol. NFT_REJECT depends on NF_TABLES_INET, so it used to
>> restricted to a loadable module with IPV6=m, but can now be
>> built-in, which causes that link error.
>
> Still one more spin on this, I would like to see if we have a way to
> fix this by simplifing things a bit.
>
> Would this one I'm attaching would work?

One disadvantage is that it makes the vmlinux bigger since
NF_REJECT_IPV{4,6} can no longer be a module at all now.

I suspect you also stil get a link error with IPV6=m, this time because
the nf_reject_ipv6.o file fails to link against the ipv6 code, e.g.
ipv6_skip_exthdr() and icmpv6_send() appear to be unreachable here.
I haven't tried that though, so I might be missing something.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: fix CONFIG_NF_REJECT_IPV6=m link error

[Arnd Bergmann]

On Mon, Apr 9, 2018 at 4:37 PM, Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> Hi Arnd,
>
> On Mon, Apr 09, 2018 at 12:53:12PM +0200, Arnd Bergmann wrote:
>> We get a new link error with CONFIG_NFT_REJECT_INET=y and 
>> CONFIG_NF_REJECT_IPV6=m
>
> I think we can update NFT_REJECT_INET so it depends on NFT_REJECT_IPV4
> and NFT_REJECT_IPV6. This doesn't allow here CONFIG_NFT_REJECT_INET=y
> and CONFIG_NF_REJECT_IPV6=m.
>
> I mean, just like we do with NFT_FIB_INET.

That can only work if NFT_REJECT_INET can be made a 'tristate' symbol
again, so that code gets built as a loadable module if
CONFIG_NF_REJECT_IPV6=m.

> BTW, I think this problem has been is not related to the recent patch,
> but something older that kbuild robot has triggered more easily for
> some reason?

02c7b25e5f54 is the one that turned NF_TABLES_INET into a 'bool'
symbol. NFT_REJECT depends on NF_TABLES_INET, so it used to
restricted to a loadable module with IPV6=m, but can now be
built-in, which causes that link error.

  Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: fix CONFIG_NF_REJECT_IPV6=m link error

[Arnd Bergmann]

We get a new link error with CONFIG_NFT_REJECT_INET=y and 
CONFIG_NF_REJECT_IPV6=m
after larger parts of the nftables modules are linked together:

net/netfilter/nft_reject_inet.o: In function `nft_reject_inet_eval':
nft_reject_inet.c:(.text+0x17c): undefined reference to `nf_send_unreach6'
nft_reject_inet.c:(.text+0x190): undefined reference to `nf_send_reset6'

The problem is that with NF_TABLES_INET set, we implicitly try to use
the ipv6 version as well for NFT_REJECT, but when CONFIG_IPV6 is set to
a loadable module, it's impossible to reach that.

The best workaround I found is to express the above as a Kconfig
dependency, forcing NFT_REJECT itself to be 'm' in that particular
configuration.

Fixes: 02c7b25e5f54 ("netfilter: nf_tables: build-in filter chain type")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 704b3832dbad..44d8a55e9721 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -594,6 +594,7 @@ config NFT_QUOTA
 config NFT_REJECT
default m if NETFILTER_ADVANCED=n
tristate "Netfilter nf_tables reject support"
+   depends on !NF_TABLES_INET || (IPV6!=m || m)
help
  This option adds the "reject" expression that you can use to
  explicitly deny and notify via TCP reset/ICMP informational errors
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 40/47] netfilter: nf_tables: build-in filter chain type

[Arnd Bergmann]

On Fri, Mar 30, 2018 at 1:46 PM, Pablo Neira Ayuso  wrote:
> One module per supported filter chain family type takes too much memory
> for very little code - too much modularization - place all chain filter
> definitions in one single file.
>
> Signed-off-by: Pablo Neira Ayuso 

Hi Pablo,

I've bisected a link error to this patch:

net/netfilter/nft_reject_inet.o: In function `nft_reject_inet_eval':
nft_reject_inet.c:(.text+0xa7): undefined reference to `nf_send_unreach6'
nft_reject_inet.c:(.text+0x10c): undefined reference to `nf_send_unreach6'
nft_reject_inet.c:(.text+0x138): undefined reference to `nf_send_reset6'

Unfortunately I don't immediately see what went wrong, maybe you
can spot it.

   Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next 1/2] netfilter: nf_defrag: mark xt_table structures 'const' again

[Arnd Bergmann]

As a side-effect of adding the module option, we now get a section
mismatch warning:

WARNING: net/ipv4/netfilter/iptable_raw.o(.data+0x1c): Section mismatch in 
reference from the variable packet_raw to the function 
.init.text:iptable_raw_table_init()
The variable packet_raw references
the function __init iptable_raw_table_init()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

Apparently it's ok to link to a __net_init function from .rodata but not
from .data. We can address this by rearranging the logic so that the
structure is read-only again. Instead of writing to the .priority field
later, we have an extra copies of the structure with that flag. An added
advantage is that that we don't have writable function pointers with this
approach.

Fixes: 902d6a4c2a4f ("netfilter: nf_defrag: Skip defrag if NOTRACK is set")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
This might not be the best fix for the issue, please have a look if you
can come up with something nicer, or just apply this version.
---
 net/ipv4/netfilter/iptable_raw.c  | 24 +++-
 net/ipv6/netfilter/ip6table_raw.c | 24 +++-
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/netfilter/iptable_raw.c b/net/ipv4/netfilter/iptable_raw.c
index 29b64d3024e0..960625aabf04 100644
--- a/net/ipv4/netfilter/iptable_raw.c
+++ b/net/ipv4/netfilter/iptable_raw.c
@@ -17,7 +17,7 @@ static bool raw_before_defrag __read_mostly;
 MODULE_PARM_DESC(raw_before_defrag, "Enable raw table before defrag");
 module_param(raw_before_defrag, bool, );
 
-static struct xt_table packet_raw = {
+static const struct xt_table packet_raw = {
.name = "raw",
.valid_hooks =  RAW_VALID_HOOKS,
.me = THIS_MODULE,
@@ -26,6 +26,15 @@ static struct xt_table packet_raw = {
.table_init = iptable_raw_table_init,
 };
 
+static const struct xt_table packet_raw_before_defrag = {
+   .name = "raw",
+   .valid_hooks =  RAW_VALID_HOOKS,
+   .me = THIS_MODULE,
+   .af = NFPROTO_IPV4,
+   .priority = NF_IP_PRI_RAW_BEFORE_DEFRAG,
+   .table_init = iptable_raw_table_init,
+};
+
 /* The work comes in here from netfilter.c. */
 static unsigned int
 iptable_raw_hook(void *priv, struct sk_buff *skb,
@@ -39,15 +48,19 @@ static struct nf_hook_ops *rawtable_ops __read_mostly;
 static int __net_init iptable_raw_table_init(struct net *net)
 {
struct ipt_replace *repl;
+   const struct xt_table *table = _raw;
int ret;
 
+   if (raw_before_defrag)
+   table = _raw_before_defrag;
+
if (net->ipv4.iptable_raw)
return 0;
 
-   repl = ipt_alloc_initial_table(_raw);
+   repl = ipt_alloc_initial_table(table);
if (repl == NULL)
return -ENOMEM;
-   ret = ipt_register_table(net, _raw, repl, rawtable_ops,
+   ret = ipt_register_table(net, table, repl, rawtable_ops,
 >ipv4.iptable_raw);
kfree(repl);
return ret;
@@ -68,14 +81,15 @@ static struct pernet_operations iptable_raw_net_ops = {
 static int __init iptable_raw_init(void)
 {
int ret;
+   const struct xt_table *table = _raw;
 
if (raw_before_defrag) {
-   packet_raw.priority = NF_IP_PRI_RAW_BEFORE_DEFRAG;
+   table = _raw_before_defrag;
 
pr_info("Enabling raw table before defrag\n");
}
 
-   rawtable_ops = xt_hook_ops_alloc(_raw, iptable_raw_hook);
+   rawtable_ops = xt_hook_ops_alloc(table, iptable_raw_hook);
if (IS_ERR(rawtable_ops))
return PTR_ERR(rawtable_ops);
 
diff --git a/net/ipv6/netfilter/ip6table_raw.c 
b/net/ipv6/netfilter/ip6table_raw.c
index 3df7383f96d0..710fa0806c37 100644
--- a/net/ipv6/netfilter/ip6table_raw.c
+++ b/net/ipv6/netfilter/ip6table_raw.c
@@ -16,7 +16,7 @@ static bool raw_before_defrag __read_mostly;
 MODULE_PARM_DESC(raw_before_defrag, "Enable raw table before defrag");
 module_param(raw_before_defrag, bool, );
 
-static struct xt_table packet_raw = {
+static const struct xt_table packet_raw = {
.name = "raw",
.valid_hooks = RAW_VALID_HOOKS,
.me = THIS_MODULE,
@@ -25,6 +25,15 @@ static struct xt_table packet_raw = {
.table_init = ip6table_raw_table_init,
 };
 
+static const struct xt_table packet_raw_before_defrag = {
+   .name = "raw",
+   .valid_hooks = RAW_VALID_HOOKS,
+   .me = THIS_MODULE,
+   .af = NFPROTO_IPV6,
+   .priority = NF_IP6_PRI_RAW_BEFORE_DEFRAG,
+   .table_init = ip6table_raw_table_init,
+};
+
 /* The work comes in here from netfilter.c. */
 static unsigned int
 ip6table_raw_hook(void *priv, struct sk_buff *skb,
@@ -38,15 +47,19 @@ static struct nf_hook_ops

[PATCH net-next 2/2] netfilter: nf_defrag: move NF_CONNTRACK bits into #ifdef

[Arnd Bergmann]

We cannot access the skb->_nfct field when CONFIG_NF_CONNTRACK is
disabled:

net/ipv4/netfilter/nf_defrag_ipv4.c: In function 'ipv4_conntrack_defrag':
net/ipv4/netfilter/nf_defrag_ipv4.c:83:9: error: 'struct sk_buff' has no member 
named '_nfct'
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c: In function 'ipv6_defrag':
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68:9: error: 'struct sk_buff' has no 
member named '_nfct'

Both functions already have an #ifdef for this, so let's move the
check in there.

Fixes: 902d6a4c2a4f ("netfilter: nf_defrag: Skip defrag if NOTRACK is set")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Please double-check what the right behavior for !CONFIG_NF_CONNTRACK
should be, I was only guessing here.
---
 net/ipv4/netfilter/nf_defrag_ipv4.c   | 4 +++-
 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c 
b/net/ipv4/netfilter/nf_defrag_ipv4.c
index cbd987f6b1f8..a0d3ad60a411 100644
--- a/net/ipv4/netfilter/nf_defrag_ipv4.c
+++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
@@ -78,9 +78,11 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
if (skb_nfct(skb) && !nf_ct_is_template((struct nf_conn 
*)skb_nfct(skb)))
return NF_ACCEPT;
 #endif
+   if (skb->_nfct == IP_CT_UNTRACKED)
+   return NF_ACCEPT;
 #endif
/* Gather fragments. */
-   if (skb->_nfct != IP_CT_UNTRACKED && ip_is_fragment(ip_hdr(skb))) {
+   if (ip_is_fragment(ip_hdr(skb))) {
enum ip_defrag_users user =
nf_ct_defrag_user(state->hook, skb);
 
diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c 
b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
index 87b503a8f5ef..c87b48359e8f 100644
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
@@ -63,10 +63,10 @@ static unsigned int ipv6_defrag(void *priv,
/* Previously seen (loopback)?  */
if (skb_nfct(skb) && !nf_ct_is_template((struct nf_conn 
*)skb_nfct(skb)))
return NF_ACCEPT;
-#endif
 
if (skb->_nfct == IP_CT_UNTRACKED)
return NF_ACCEPT;
+#endif
 
err = nf_ct_frag6_gather(state->net, skb,
 nf_ct6_defrag_user(state->hook, skb));
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: nf_tables: flow_offload depends on flow_table

[Arnd Bergmann]

Without CONFIG_NF_FLOW_TABLE, the new nft_flow_offload module produces
a link error:

net/netfilter/nft_flow_offload.o: In function 
`nft_flow_offload_iterate_cleanup':
nft_flow_offload.c:(.text+0xb0): undefined reference to `nf_flow_table_iterate'
net/netfilter/nft_flow_offload.o: In function `flow_offload_iterate_cleanup':
nft_flow_offload.c:(.text+0x160): undefined reference to `flow_offload_dead'
net/netfilter/nft_flow_offload.o: In function `nft_flow_offload_eval':
nft_flow_offload.c:(.text+0xc4c): undefined reference to `flow_offload_alloc'
nft_flow_offload.c:(.text+0xc64): undefined reference to `flow_offload_add'
nft_flow_offload.c:(.text+0xc94): undefined reference to `flow_offload_free'

This adds a Kconfig dependency for it.

Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index ea447826e127..9019fa98003d 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -506,7 +506,7 @@ config NFT_CT
  connection tracking information such as the flow state.
 
 config NFT_FLOW_OFFLOAD
-   depends on NF_CONNTRACK
+   depends on NF_CONNTRACK && NF_FLOW_TABLE
tristate "Netfilter nf_tables hardware flow offload module"
help
  This option adds the "flow_offload" expression that you can use to
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next] netfilter: improve flow table Kconfig dependencies

[Arnd Bergmann]

The newly added NF_FLOW_TABLE options cause some build failures in
randconfig kernels:

- when CONFIG_NF_CONNTRACK is disabled, or is a loadable module but
  NF_FLOW_TABLE is built-in:

  In file included from net/netfilter/nf_flow_table.c:8:0:
  include/net/netfilter/nf_conntrack.h:59:22: error: field 'ct_general' has 
incomplete type
struct nf_conntrack ct_general;
  include/net/netfilter/nf_conntrack.h: In function 'nf_ct_get':
  include/net/netfilter/nf_conntrack.h:148:15: error: 'const struct sk_buff' 
has no member named '_nfct'
  include/net/netfilter/nf_conntrack.h: In function 'nf_ct_put':
  include/net/netfilter/nf_conntrack.h:157:2: error: implicit declaration of 
function 'nf_conntrack_put'; did you mean 'nf_ct_put'? 
[-Werror=implicit-function-declaration]

  net/netfilter/nf_flow_table.o: In function `nf_flow_offload_work_gc':
  (.text+0x1540): undefined reference to `nf_ct_delete'

- when CONFIG_NF_TABLES is disabled:

  In file included from net/ipv6/netfilter/nf_flow_table_ipv6.c:13:0:
  include/net/netfilter/nf_tables.h: In function 'nft_gencursor_next':
  include/net/netfilter/nf_tables.h:1189:14: error: 'const struct net' has no 
member named 'nft'; did you mean 'nf'?

 - when CONFIG_NF_FLOW_TABLE_INET is enabled, but NF_FLOW_TABLE_IPV4
  or NF_FLOW_TABLE_IPV6 are not, or are loadable modules

  net/netfilter/nf_flow_table_inet.o: In function `nf_flow_offload_inet_hook':
  nf_flow_table_inet.c:(.text+0x94): undefined reference to 
`nf_flow_offload_ipv6_hook'
  nf_flow_table_inet.c:(.text+0x40): undefined reference to 
`nf_flow_offload_ip_hook'

- when CONFIG_NF_FLOW_TABLES is disabled, but the other options are
  enabled:

  net/netfilter/nf_flow_table_inet.o: In function `nf_flow_offload_inet_hook':
  nf_flow_table_inet.c:(.text+0x6c): undefined reference to 
`nf_flow_offload_ipv6_hook'
  net/netfilter/nf_flow_table_inet.o: In function `nf_flow_inet_module_exit':
  nf_flow_table_inet.c:(.exit.text+0x8): undefined reference to 
`nft_unregister_flowtable_type'
  net/netfilter/nf_flow_table_inet.o: In function `nf_flow_inet_module_init':
  nf_flow_table_inet.c:(.init.text+0x8): undefined reference to 
`nft_register_flowtable_type'
  net/ipv4/netfilter/nf_flow_table_ipv4.o: In function 
`nf_flow_ipv4_module_exit':
  nf_flow_table_ipv4.c:(.exit.text+0x8): undefined reference to 
`nft_unregister_flowtable_type'
  net/ipv4/netfilter/nf_flow_table_ipv4.o: In function 
`nf_flow_ipv4_module_init':
  nf_flow_table_ipv4.c:(.init.text+0x8): undefined reference to 
`nft_register_flowtable_type'

This adds additional Kconfig dependencies to ensure that NF_CONNTRACK and 
NF_TABLES
are always visible from NF_FLOW_TABLE, and that the internal dependencies 
between
the four new modules are met.

Fixes: 7c23b629a808 ("netfilter: flow table support for the mixed IPv4/IPv6 
family")
Fixes: 0995210753a2 ("netfilter: flow table support for IPv6")
Fixes: 97add9f0d66d ("netfilter: flow table support for IPv4")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/ipv4/netfilter/Kconfig | 3 ++-
 net/ipv6/netfilter/Kconfig | 3 ++-
 net/netfilter/Kconfig  | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
index 7d5d444964aa..3ad46a90b0fc 100644
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -79,8 +79,9 @@ config NF_TABLES_ARP
 endif # NF_TABLES
 
 config NF_FLOW_TABLE_IPV4
-   select NF_FLOW_TABLE
tristate "Netfilter flow table IPv4 module"
+   depends on NF_CONNTRACK && NF_TABLES
+   select NF_FLOW_TABLE
help
  This option adds the flow table IPv4 support.
 
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 806e95375ec8..3fb2818317b8 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -72,8 +72,9 @@ endif # NF_TABLES_IPV6
 endif # NF_TABLES
 
 config NF_FLOW_TABLE_IPV6
-   select NF_FLOW_TABLE
tristate "Netfilter flow table IPv6 module"
+   depends on NF_CONNTRACK && NF_TABLES
+   select NF_FLOW_TABLE
help
  This option adds the flow table IPv6 support.
 
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 0ee0fcf3abbf..ea447826e127 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -665,8 +665,9 @@ endif # NF_TABLES_NETDEV
 endif # NF_TABLES
 
 config NF_FLOW_TABLE_INET
-   select NF_FLOW_TABLE
tristate "Netfilter flow table mixed IPv4/IPv6 module"
+   depends on NF_FLOW_TABLE_IPV4 && NF_FLOW_TABLE_IPV6
+   select NF_FLOW_TABLE
help
   This option adds the flow table mixed IPv4/IPv6 support.
 
@@ -674,6 +675,7 @@ config NF_FLOW_TABLE_INET
 
 config NF_FLOW_TABLE
tristate "Netfilter flow table module"
+   depends on NF_CONNTRACK && NF_TABLES
help
  This option adds the flow table co

[PATCH] netfilter: add nf_queue_entry forward declaration

[Arnd Bergmann]

The newly added callback pointers cause a warning for some configurations:

In file included from net/ipv6/af_inet6.c:45:0:
include/linux/netfilter_ipv6.h:38:51: error: 'struct nf_queue_entry' declared 
inside parameter list will not be visible outside of this definition or 
declaration [-Werror]

Adding a forward declaration for the type avoids the warnings.

Fixes: 9faa679ee7ec ("netfilter: move reroute indirection to struct 
nf_ipv6_ops")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 include/linux/netfilter_ipv4.h | 2 ++
 include/linux/netfilter_ipv6.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/linux/netfilter_ipv4.h b/include/linux/netfilter_ipv4.h
index 0259bcde6d2e..b31dabfdb453 100644
--- a/include/linux/netfilter_ipv4.h
+++ b/include/linux/netfilter_ipv4.h
@@ -18,6 +18,8 @@ struct ip_rt_info {
 
 int ip_route_me_harder(struct net *net, struct sk_buff *skb, unsigned 
addr_type);
 
+struct nf_queue_entry;
+
 #ifdef CONFIG_INET
 __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
   unsigned int dataoff, u_int8_t protocol);
diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
index e4cdcfdce0f9..288c597e75b3 100644
--- a/include/linux/netfilter_ipv6.h
+++ b/include/linux/netfilter_ipv6.h
@@ -18,6 +18,8 @@ struct ip6_rt_info {
u_int32_t mark;
 };
 
+struct nf_queue_entry;
+
 /*
  * Hook functions for ipv6 to allow xt_* modules to be built-in even
  * if IPv6 is a module.
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: fix clusterip_net_exit build regression

[Arnd Bergmann]

The added check produces a build error when CONFIG_PROC_FS is
disabled:

net/ipv4/netfilter/ipt_CLUSTERIP.c: In function 'clusterip_net_exit':
net/ipv4/netfilter/ipt_CLUSTERIP.c:822:28: error: 'cn' undeclared (first use in 
this function)

This moves the variable declaration out of the #ifdef to make it
available to the WARN_ON_ONCE().

Fixes: 613d0776d3fe ("netfilter: exit_net cleanup check added")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c 
b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index e35b8d074f06..69060e3abe85 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -813,8 +813,8 @@ static int clusterip_net_init(struct net *net)
 
 static void clusterip_net_exit(struct net *net)
 {
-#ifdef CONFIG_PROC_FS
struct clusterip_net *cn = net_generic(net, clusterip_net_id);
+#ifdef CONFIG_PROC_FS
proc_remove(cn->procdir);
cn->procdir = NULL;
 #endif
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [net-next] netfilter: add ifdef around ctnetlink_proto_size

[Arnd Bergmann]

This function is no longer marked 'inline', so we now get a warning
when it is unused:

net/netfilter/nf_conntrack_netlink.c:536:15: error: 'ctnetlink_proto_size' 
defined but not used [-Werror=unused-function]

We could mark it inline again, mark it __maybe_unused, or add an #ifdef
around the definition. I'm picking the third approach here since that
seems to be what the rest of the file has.

Fixes: 5caaed151a68 ("netfilter: conntrack: don't cache nlattr_tuple_size 
result in nla_size")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/nf_conntrack_netlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index 6e0adfefb9ed..59c08997bfdf 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -533,6 +533,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32 
seq, u32 type,
return -1;
 }
 
+#if defined(CONFIG_NETFILTER_NETLINK_GLUE_CT) || 
defined(CONFIG_NF_CONNTRACK_EVENTS)
 static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 {
const struct nf_conntrack_l3proto *l3proto;
@@ -552,6 +553,7 @@ static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 
return len + len4;
 }
+#endif
 
 static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)
 {
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: xt_hashlimit: avoid 64-bit division

[Arnd Bergmann]

On Thu, Sep 7, 2017 at 12:19 PM, Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> On Wed, Sep 06, 2017 at 10:48:22PM +0200, Arnd Bergmann wrote:
>> On Wed, Sep 6, 2017 at 10:22 PM, Vishwanath Pai <v...@akamai.com> wrote:
>> > On 09/06/2017 03:57 PM, Arnd Bergmann wrote:
>> >> 64-bit division is expensive on 32-bit architectures, and
>> >> requires a special function call to avoid a link error like:
>> >>
>> >> net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
>> >> xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'
>> >>
>> >> In the case of hashlimit_mt_common, we don't actually need a
>> >> 64-bit operation, we can simply rewrite the function slightly
>> >> to make that clear to the compiler.
>> >>
>> >> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
>> >> Signed-off-by: Arnd Bergmann <a...@arndb.de>
>> >> ---
>> >>  net/netfilter/xt_hashlimit.c | 5 -
>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
>> >> index 10d48234f5f4..50b53d86eef5 100644
>> >> --- a/net/netfilter/xt_hashlimit.c
>> >> +++ b/net/netfilter/xt_hashlimit.c
>> >> @@ -531,7 +531,10 @@ static u64 user2rate_bytes(u64 user)
>> >>  {
>> >>   u64 r;
>> >>
>> >> - r = user ? 0xULL / user : 0xULL;
>> >> + if (user > 0xULL)
>> >> + return 0;
>> >> +
>> >> + r = user ? 0xULL / (u32)user : 0xULL;
>> >>   r = (r - 1) << 4;
>> >>   return r;
>> >>  }
>> >>
>> >
>> > I have submitted another patch to fix this:
>> > https://patchwork.ozlabs.org/patch/809881/
>> >
>> > We have seen this problem before, I was careful not to introduce this
>> > again in the new patch but clearly I overlooked this particular line :(
>> >
>> > In the other cases we fixed it by replacing division with div64_u64().
>>
>> div64_u64() seems needlessly expensive here since the dividend
>> is known to be a 32-bit number. I guess the function is not called
>> frequently though, so it doesn't matter much.
>
> This is called from the packet path, only for the first packet for
> each new destination IP entry in the hashtable, still from the
> datapath. So if we can take something faster (for 32 bit arches) that
> is correct, I think it's sensible to take.
>
> Let me know in any case.

I think my version should be slightly better then, unless someone
finds something wrong with it.

   Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: xt_hashlimit: avoid 64-bit division

[Arnd Bergmann]

On Wed, Sep 6, 2017 at 10:22 PM, Vishwanath Pai <v...@akamai.com> wrote:
> On 09/06/2017 03:57 PM, Arnd Bergmann wrote:
>> 64-bit division is expensive on 32-bit architectures, and
>> requires a special function call to avoid a link error like:
>>
>> net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
>> xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'
>>
>> In the case of hashlimit_mt_common, we don't actually need a
>> 64-bit operation, we can simply rewrite the function slightly
>> to make that clear to the compiler.
>>
>> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
>> Signed-off-by: Arnd Bergmann <a...@arndb.de>
>> ---
>>  net/netfilter/xt_hashlimit.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
>> index 10d48234f5f4..50b53d86eef5 100644
>> --- a/net/netfilter/xt_hashlimit.c
>> +++ b/net/netfilter/xt_hashlimit.c
>> @@ -531,7 +531,10 @@ static u64 user2rate_bytes(u64 user)
>>  {
>>   u64 r;
>>
>> - r = user ? 0xULL / user : 0xULL;
>> + if (user > 0xULL)
>> + return 0;
>> +
>> + r = user ? 0xULL / (u32)user : 0xULL;
>>   r = (r - 1) << 4;
>>   return r;
>>  }
>>
>
> I have submitted another patch to fix this:
> https://patchwork.ozlabs.org/patch/809881/
>
> We have seen this problem before, I was careful not to introduce this
> again in the new patch but clearly I overlooked this particular line :(
>
> In the other cases we fixed it by replacing division with div64_u64().

div64_u64() seems needlessly expensive here since the dividend
is known to be a 32-bit number. I guess the function is not called
frequently though, so it doesn't matter much.

  Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: xt_hashlimit: avoid 64-bit division

[Arnd Bergmann]

64-bit division is expensive on 32-bit architectures, and
requires a special function call to avoid a link error like:

net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'

In the case of hashlimit_mt_common, we don't actually need a
64-bit operation, we can simply rewrite the function slightly
to make that clear to the compiler.

Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/xt_hashlimit.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10d48234f5f4..50b53d86eef5 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -531,7 +531,10 @@ static u64 user2rate_bytes(u64 user)
 {
u64 r;
 
-   r = user ? 0xULL / user : 0xULL;
+   if (user > 0xULL)
+   return 0;
+
+   r = user ? 0xULL / (u32)user : 0xULL;
r = (r - 1) << 4;
return r;
 }
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: fix stringop-overflow warning with UBSAN

[Arnd Bergmann]

Using gcc-7 with UBSAN enabled, we get this false-positive warning:

net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get':
net/netfilter/ipset/ip_set_core.c:1998:3: error: 'strncpy' writing 32 bytes 
into a region of size 2 overflows the destination [-Werror=stringop-overflow=]
   strncpy(req_get->set.name, set ? set->name : "",
   ^~~~
sizeof(req_get->set.name));
~~

This seems completely bogus, and I could not find a nice workaround.
To work around it in a less elegant way, I change the ?: operator
into an if()/else() construct.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/ipset/ip_set_core.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index e495b5e484b1..d7ebb021003b 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1995,8 +1995,12 @@ ip_set_sockfn_get(struct sock *sk, int optval, void 
__user *user, int *len)
}
nfnl_lock(NFNL_SUBSYS_IPSET);
set = ip_set(inst, req_get->set.index);
-   strncpy(req_get->set.name, set ? set->name : "",
-   IPSET_MAXNAMELEN);
+   if (set)
+   strncpy(req_get->set.name, set->name,
+   sizeof(req_get->set.name));
+   else
+   memset(req_get->set.name, '\0',
+  sizeof(req_get->set.name));
nfnl_unlock(NFNL_SUBSYS_IPSET);
goto copy;
}
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with lock/unlock pair

[Arnd Bergmann]

On Fri, Jun 30, 2017 at 2:01 AM, Paul E. McKenney
 wrote:
> There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> and it appears that all callers could do just as well with a lock/unlock
> pair.  This commit therefore replaces the spin_unlock_wait() call in
> do_task_dead() with spin_lock() followed immediately by spin_unlock().
> This should be safe from a performance perspective because the lock is
> this tasks ->pi_lock, and this is called only after the task exits.
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e91138fcde86..6dea3d9728c8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3461,7 +3461,8 @@ void __noreturn do_task_dead(void)
>  * is held by try_to_wake_up()
>  */
> smp_mb();
> -   raw_spin_unlock_wait(>pi_lock);
> +   raw_spin_lock(>pi_lock);
> +   raw_spin_unlock(>pi_lock);

Does the raw_spin_lock()/raw_spin_unlock() imply an smp_mb() or stronger?
Maybe it would be clearer to remove the extra barrier if so.

 Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: conntrack: Force inlining of build check to prevent build failure

[Arnd Bergmann]

On Wed, May 3, 2017 at 2:47 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Arnd,
>
> On Wed, May 3, 2017 at 2:32 PM, Arnd Bergmann <a...@arndb.de> wrote:
>> On Wed, May 3, 2017 at 2:18 PM, Geert Uytterhoeven <ge...@linux-m68k.org> 
>> wrote:
>>> If gcc (e.g. 4.1.2) decides not to inline total_extension_size(), the
>>> build will fail with:
>>>
>>> net/built-in.o: In function `nf_conntrack_init_start':
>>> (.text+0x9baf6): undefined reference to `__compiletime_assert_1893'
>>>
>>> or
>>>
>>> ERROR: "__compiletime_assert_1893" [net/netfilter/nf_conntrack.ko] 
>>> undefined!
>>>
>>> Fix this by forcing inlining of total_extension_size().
>>>
>>> Fixes: b3a5db109e0670d6 ("netfilter: conntrack: use u8 for extension sizes 
>>> again")
>>> Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
>>
>> I saw this as well when I tried building with "gcc-7 -Og", and came to the 
>> same
>> conclusion.
>
> Good^H^H^H^HBad to see it not only happens with ancient compilers ;-)

Right now we don't see it on newer compilers (I assume gcc-4.3 or up) as we
always build with either -O2 or -Os optimizations. I was playing with -Og the
other day to get faster builds, but that causes many build failures
and additional
warnings as the result of missing out on optimizations that we have come
to rely on.

It might be worth getting -Og to build if the compile time is
drastically faster, but
we probably have to completely do away with BUILD_BUG_ON() and similar
checks in that configuration, which in turn makes the build output less
valuable.

   Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: conntrack: Force inlining of build check to prevent build failure

[Arnd Bergmann]

On Wed, May 3, 2017 at 2:18 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> If gcc (e.g. 4.1.2) decides not to inline total_extension_size(), the
> build will fail with:
>
> net/built-in.o: In function `nf_conntrack_init_start':
> (.text+0x9baf6): undefined reference to `__compiletime_assert_1893'
>
> or
>
> ERROR: "__compiletime_assert_1893" [net/netfilter/nf_conntrack.ko] 
> undefined!
>
> Fix this by forcing inlining of total_extension_size().
>
> Fixes: b3a5db109e0670d6 ("netfilter: conntrack: use u8 for extension sizes 
> again")
> Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>

I saw this as well when I tried building with "gcc-7 -Og", and came to the same
conclusion.

Acked-by: Arnd Bergmann <a...@arndb.de>

With -Og, there were a couple of other instances of BUILD_BUG_ON() failing
to see a compile-time constant, presumably as it fails to inline any functions
that are not explicitly marked inline.

   Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [netfilter-next] netfilter: remove unused refcount variable

[Arnd Bergmann]

The refcount variable was accidentally introduced without any reference
to it. Removing it again avoids this warning:

net/netfilter/nfnetlink_acct.c: In function 'nfnl_acct_try_del':
net/netfilter/nfnetlink_acct.c:329:15: error: unused variable 'refcount' 
[-Werror=unused-variable]

Fixes: b54ab92b84b6 ("netfilter: refcounter conversions")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/nfnetlink_acct.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index f44cbd35357f..c86da174a5fc 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -326,7 +326,6 @@ static int nfnl_acct_get(struct net *net, struct sock *nfnl,
 static int nfnl_acct_try_del(struct nf_acct *cur)
 {
int ret = 0;
-   unsigned int refcount;
 
/* We want to avoid races with nfnl_acct_put. So only when the current
 * refcnt is 1, we decrease it to 0.
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: ipt_CLUSTERIP: fix build error without procfs

[Arnd Bergmann]

We can't access c->pde if CONFIG_PROC_FS is disabled:

net/ipv4/netfilter/ipt_CLUSTERIP.c: In function 'clusterip_config_find_get':
net/ipv4/netfilter/ipt_CLUSTERIP.c:147:9: error: 'struct clusterip_config' has 
no member named 'pde'

This moves the check inside of another #ifdef.

Fixes: 6c5d5cfbe3c5 ("netfilter: ipt_CLUSTERIP: check duplicate config when 
initializing")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c 
b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 1e38d8bf5631..52f26459efc3 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -144,7 +144,12 @@ clusterip_config_find_get(struct net *net, __be32 
clusterip, int entry)
rcu_read_lock_bh();
c = __clusterip_config_find(net, clusterip);
if (c) {
-   if (!c->pde || unlikely(!atomic_inc_not_zero(>refcount)))
+#ifdef CONFIG_PROC_FS
+   if (!c->pde)
+   c = NULL;
+   else
+#endif
+   if (unlikely(!atomic_inc_not_zero(>refcount)))
c = NULL;
else if (entry)
atomic_inc(>entries);
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/7] arm: put types.h in uapi

[Arnd Bergmann]

On Friday, January 6, 2017 10:43:53 AM CET Nicolas Dichtel wrote:
> 
> diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
> index a53cdb8f068c..c48fee3d7b3b 100644
> --- a/arch/arm/include/asm/types.h
> +++ b/arch/arm/include/asm/types.h
> @@ -1,40 +1,6 @@
>  #ifndef _ASM_TYPES_H
>  #define _ASM_TYPES_H
>  
> -#include 
...
> -#define __UINTPTR_TYPE__   unsigned long
> -#endif
> +#include 
>  
>  #endif /* _ASM_TYPES_H */
> 

Moving the file is correct as far as I can tell, but the extra
#include is not necessary here, as the kernel will automatically
search both arch/arm/include/ and arch/arm/include/uapi/.

The same applies to patches 2 and 4.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/7] nios2: put setup.h in uapi

[Arnd Bergmann]

On Friday, January 6, 2017 10:43:55 AM CET Nicolas Dichtel wrote:

> diff --git a/arch/nios2/include/uapi/asm/setup.h 
> b/arch/nios2/include/uapi/asm/setup.h
> new file mode 100644
> index ..8d8285997ba8
> --- /dev/null
> +++ b/arch/nios2/include/uapi/asm/setup.h
> @@ -0,0 +1,6 @@
> +#ifndef _UAPI_ASM_NIOS2_SETUP_H
> +#define _UAPI_ASM_NIOS2_SETUP_H
> +
> +#include 
> +
> +#endif /* _UAPI_ASM_NIOS2_SETUP_H */
> 

This one is only a redirect to an asm-generic header, so it can be
removed completely and replaced with a line in the 
arch/nios2/include/uapi/asm/ file:

generic-y += setup.h

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/7] uapi: export all headers under uapi directories

[Arnd Bergmann]

On Friday, January 6, 2017 10:43:52 AM CET Nicolas Dichtel wrote:
> Here is the v2 of this series. The first 5 patches are just cleanup: some
> exported headers were still under a non-uapi directory.

Since this is meant as a cleanup, I commented on this to point out a cleaner
way to do the same.

> The patch 6 was spotted by code review: there is no in-tree user of this
> functionality.
> The last patch remove the use of header-y. Now all files under an uapi
> directory are exported.

Very nice!

> asm is a bit special, most of architectures export asm//include/uapi/asm
> only, but there is two exceptions:
>  - cris which exports arch/cris/include/uapi/arch-v[10|32];

This is interesting, though not your problem. Maybe someone who understands
cris better can comment on this: How is the decision made about which of
the arch/user.h headers gets used? I couldn't find that in the sources,
but it appears to be based on kernel compile-time settings, which is
wrong for user space header files that should be independent of the kernel
config.

>  - tile which exports arch/tile/include/uapi/arch.
> Because I don't know if the output of 'make headers_install_all' can be 
> changed,
> I introduce subdir-y in Kbuild file. The headers_install_all target copies all
> asm//include/uapi/asm to usr/include/asm- but
> arch/cris/include/uapi/arch-v[10|32] and arch/tile/include/uapi/arch are not
> prefixed (they are put asis in usr/include/). If it's acceptable to modify the
> output of 'make headers_install_all' to export asm headers in
> usr/include/asm-/asm, then I could remove this new subdir-y and exports
> everything under arch//include/uapi/.

I don't know if anyone still uses "make headers_install_all", I suspect
distros these days all use "make headers_install", so it probably
doesn't matter much.

In case of cris, it should be easy enough to move all the contents of the
uapi/arch-*/*.h headers into the respective uapi/asm/*.h headers, they
only seem to be referenced from there.

For tile, I suspect that would not work as the arch/*.h headers are
apparently defined as interfaces for both user space and kernel.

> Note also that exported files for asm are a mix of files listed by:
>  - include/uapi/asm-generic/Kbuild.asm;
>  - arch/x86/include/uapi/asm/Kbuild;
>  - arch/x86/include/asm/Kbuild.
> This complicates a lot the processing (arch/x86/include/asm/Kbuild is also
> used by scripts/Makefile.asm-generic).
> 
> This series has been tested with a 'make headers_install' on x86 and a
> 'make headers_install_all'. I've checked the result of both commands.
> 
> This patch is built against linus tree. I don't know if it should be
> made against antoher tree.

The series should probably get merged through the kbuild tree, but testing
it on mainline is fine here.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] uapi: use wildcards to list files

[Arnd Bergmann]

On Tuesday, January 3, 2017 3:35:44 PM CET Nicolas Dichtel wrote:
> Regularly, when a new header is created in include/uapi/, the developer
> forgets to add it in the corresponding Kbuild file. This error is usually
> detected after the release is out.
> 
> In fact, all headers under include/uapi/ should be exported, so let's
> use wildcards.

I think the idea makes a lot of sense: if a header is in uapi, we should
really export it. However, using a wildcard expression seems a bit
backwards here, I think we should make this implicit and not have the
Kbuild file at all.

The "header-y" syntax was originally added back when the uapi headers
were mixed with the internal headers in the same directory. After
David Howells introduced the separate directory for uapi, it has
become a bit redundant.

Can you try to modify scripts/Makefile.headersinst instead so we
can simply remove the Kbuild files entirely?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] netfilter: nft_counter: rework atomic dump and reset

[Arnd Bergmann]

On Sunday, December 11, 2016 11:43:59 AM CET Pablo Neira Ayuso wrote:
> Dump and reset doesn't work unless cmpxchg64() is used both from packet
> and control plane paths. This approach is going to be slow though.
> Instead, use a percpu seqcount to fetch counters consistently, then
> subtract bytes and packets in case a reset was requested.
> 
> The cpu that running over the reset code is guaranteed to own this stats
> exclusively, we have to turn counters into signed 64bit though so stats
> update on reset don't get wrong on underflow.
> 
> This patch is based on original sketch from Eric Dumazet.
> 
> Fixes: 43da04a593d8 ("netfilter: nf_tables: atomic dump and reset for 
> stateful objects")
> Suggested-by: Eric Dumazet 
> Signed-off-by: Pablo Neira Ayuso 
> 

Looks great, I had a similar idea that I was going to suggest the
other day, but yours is better anyway.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: add cmpxchg64 helper for ARMv7-M

[Arnd Bergmann]

A change to the netfilter code in net-next introduced the first caller of
cmpxchg64 that can get built on ARMv7-M, leading to an error from the
assembler that points out the lack of 64-bit atomics on this architecture:

/tmp/ccMe7djj.s: Assembler messages:
/tmp/ccMe7djj.s:367: Error: selected processor does not support `ldrexd 
r0,r1,[lr]' in Thumb mode
/tmp/ccMe7djj.s:371: Error: selected processor does not support `strexd 
ip,r2,r3,[lr]' in Thumb mode
/tmp/ccMe7djj.s:389: Error: selected processor does not support `ldrexd 
r8,r9,[r7]' in Thumb mode
/tmp/ccMe7djj.s:393: Error: selected processor does not support `strexd 
lr,r0,r1,[r7]' in Thumb mode
scripts/Makefile.build:299: recipe for target 'net/netfilter/nft_counter.o' 
failed

This makes ARMv7-M use the same emulation from asm-generic/cmpxchg-local.h
that we use on architectures earlier than ARMv6K, to fix the build. The
32-bit atomics are available on ARMv7-M and we keep using them there.
This ARM specific change is probably something we should do regardless
of the netfilter code.

However, looking at the new nft_counter_reset() function in nft_counter.c,
this looks incorrect to me not just on ARMv7-M but also on other
architectures, with at least the following possible race:

CPU A   CPU B
u64_stats_fetch_begin_irq
u64_stats_update_begin
fetch(upper 32 bits)
fetch(old)
cmpxchg64(counter, old, 0);
fetch(lower 32 bits)
u64_stats_fetch_retry_irq == true
store(upper 32 bits)
fetch(old)
cmpxchg64(counter, old, 0);
store(lower 32 bits)
u64_stats_update_end
u64_stats_fetch_retry_irq == true
fetch(old)
cmpxchg64(counter, old, 0);
u64_stats_fetch_retry_irq == false

In this example, the data returned by __nft_counter_reset() is zero
as we overwrite the per-cpu counter value during the retries.

Fixes: 43da04a593d8 ("netfilter: nf_tables: atomic dump and reset for stateful 
objects")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 arch/arm/include/asm/cmpxchg.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 97882f9bad12..12215515ba02 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -240,6 +240,7 @@ static inline unsigned long __cmpxchg_local(volatile void 
*ptr,
sizeof(*(ptr)));\
 })
 
+#ifndef CONFIG_CPU_V7M
 static inline unsigned long long __cmpxchg64(unsigned long long *ptr,
 unsigned long long old,
 unsigned long long new)
@@ -273,6 +274,18 @@ static inline unsigned long long __cmpxchg64(unsigned long 
long *ptr,
 
 #define cmpxchg64_local(ptr, o, n) cmpxchg64_relaxed((ptr), (o), (n))
 
+#else
+
+/* ARMv7-M has 32-bit ldrex/strex but no ldrexd/strexd */
+
+#define cmpxchg64(ptr, o, n)   __cmpxchg64_local_generic((ptr), (o), 
(n))
+#define cmpxchg64_relaxed(ptr, o, n)   __cmpxchg64_local_generic((ptr), (o), 
(n))
+#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), 
(n))
+
+#include 
+
+#endif
+
 #endif /* __LINUX_ARM_ARCH__ >= 6 */
 
 #endif /* __ASM_ARM_CMPXCHG_H */
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] [nf-next] netfilter: fix NF_REPEAT handling

[Arnd Bergmann]

gcc correctly identified a theoretical uninitialized variable use:

net/netfilter/nf_conntrack_core.c: In function 'nf_conntrack_in':
net/netfilter/nf_conntrack_core.c:1125:14: error: 'l4proto' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]

This could only happen when we 'goto out' before looking up l4proto,
and then enter the retry, implying that l3proto->get_l4proto()
returned NF_REPEAT. This does not currently get returned in any
code path and probably won't ever happen, but is not good to
rely on.

Moving the repeat handling up a little should have the same
behavior as today but avoids the warning by making that case
impossible to enter.

Fixes: 08733a0cb7de ("netfilter: handle NF_REPEAT from nf_conntrack_in()")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
The patch causing this is currently only in nf-next, and not yet
in net-next.
---
 net/netfilter/nf_conntrack_core.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index de4b8a75f30b..610c9de0ce18 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1337,6 +1337,8 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned 
int hooknum,
NF_CT_STAT_INC_ATOMIC(net, invalid);
if (ret == -NF_DROP)
NF_CT_STAT_INC_ATOMIC(net, drop);
+   if (ret == -NF_REPEAT && tmpl)
+   goto repeat;
ret = -ret;
goto out;
}
@@ -1349,10 +1351,7 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned 
int hooknum,
 * closed/aborted connection. We have to go back and create a
 * fresh conntrack.
 */
-   if (ret == NF_REPEAT)
-   goto repeat;
-   else
-   nf_ct_put(tmpl);
+   nf_ct_put(tmpl);
}
 
return ret;
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] [net-next] udp: provide udp{4,6}_lib_lookup for nf_socket_ipv{4,6}

[Arnd Bergmann]

Since commit ca065d0cf80f ("udp: no longer use SLAB_DESTROY_BY_RCU")
the udp6_lib_lookup and udp4_lib_lookup functions are only
provided when it is actually possible to call them.

However, moving the callers now caused a link error:

net/built-in.o: In function `nf_sk_lookup_slow_v6':
(.text+0x131a39): undefined reference to `udp6_lib_lookup'
net/ipv4/netfilter/nf_socket_ipv4.o: In function `nf_sk_lookup_slow_v4':
nf_socket_ipv4.c:(.text.nf_sk_lookup_slow_v4+0x114): undefined reference to 
`udp4_lib_lookup'

This extends the #ifdef so we also provide the functions when
CONFIG_NF_SOCKET_IPV4 or CONFIG_NF_SOCKET_IPV6, respectively
are set.

Fixes: 8db4c5be88f6 ("netfilter: move socket lookup infrastructure to 
nf_socket_ipv{4,6}.c")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
The build failure came from the netfilter tree but is now
present in net-next, so if the solution is correct, this
patch can be applied there.
---
 net/ipv4/udp.c | 3 ++-
 net/ipv6/udp.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 097b70628631..c827e4ea509e 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -580,7 +580,8 @@ EXPORT_SYMBOL_GPL(udp4_lib_lookup_skb);
  * Does increment socket refcount.
  */
 #if IS_ENABLED(CONFIG_NETFILTER_XT_MATCH_SOCKET) || \
-IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TPROXY)
+IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TPROXY) || \
+IS_ENABLED(CONFIG_NF_SOCKET_IPV4)
 struct sock *udp4_lib_lookup(struct net *net, __be32 saddr, __be16 sport,
 __be32 daddr, __be16 dport, int dif)
 {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 5313818b7485..86a8cacd333b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -302,7 +302,8 @@ EXPORT_SYMBOL_GPL(udp6_lib_lookup_skb);
  * Does increment socket refcount.
  */
 #if IS_ENABLED(CONFIG_NETFILTER_XT_MATCH_SOCKET) || \
-IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TPROXY)
+IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TPROXY) || \
+IS_ENABLED(CONFIG_NF_SOCKET_IPV6)
 struct sock *udp6_lib_lookup(struct net *net, const struct in6_addr *saddr, 
__be16 sport,
 const struct in6_addr *daddr, __be16 dport, int 
dif)
 {
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings

[Arnd Bergmann]

On Friday, October 28, 2016 6:21:49 PM CEST Florian Westphal wrote:
> Good point.  In case oif is NULL we don't have to search the result
> list for a match anyway, so we could do this (not even build tested):
> 

It didn't apply cleanly, but I've integrated it with the change to
initialize oif to NULL and the added #ifdef I had in my first version
and got a clean build.

I sent out a v2 now, but didn't try hard to understand your changes.

Arnd

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [v2 netfilter-next] netfilter: nf_tables: fib warnings

[Arnd Bergmann]

The newly added nft fib code produces two warnings:

net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' 
[-Werror=unused-variable]
net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized 
in this function [-Werror=maybe-uninitialized]

The first one is obvious as the only user of that variable is
inside of an #ifdef

The second one is a bit trickier. It's clear that oif is in fact
uninitialized when it gets used when neither NFTA_FIB_F_IIF nor
NFTA_FIB_F_OIF are set, and just setting it to NULL won't work
as it may later get dereferenced.

However, there is no need to search the result list if it is
NULL, as Florian pointed out. This integrates his (untested)
change to do so. I have confirmed that the combined patch
solves both warnings, but as I don't fully understand Florian's
change, I can't tell if it's correct.

Suggested-by: Florian Westphal <f...@strlen.de>
Fixes: 84f5eedb983e ("netfilter: nf_tables: add fib expression")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: integrate changes that Florian suggested
---
 net/ipv4/netfilter/nft_fib_ipv4.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c 
b/net/ipv4/netfilter/nft_fib_ipv4.c
index 6787c563cfc9..db91fd42db67 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -77,7 +77,9 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
};
const struct net_device *oif;
struct net_device *found;
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
int i;
+#endif
 
/*
 * Do not set flowi4_oif, it restricts results (for example, asking
@@ -90,6 +92,8 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
oif = pkt->out;
else if (priv->flags & NFTA_FIB_F_IIF)
oif = pkt->in;
+   else
+   oif = NULL;
 
if (pkt->hook == NF_INET_PRE_ROUTING && fib4_is_local(pkt->skb)) {
nft_fib_store_result(dest, priv->result, pkt, LOOPBACK_IFINDEX);
@@ -130,6 +134,11 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
break;
}
 
+   if (!oif) {
+   found = FIB_RES_DEV(res);
+   goto ok;
+   }
+
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
for (i = 0; i < res.fi->fib_nhs; i++) {
struct fib_nh *nh = >fib_nh[i];
@@ -139,16 +148,12 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
goto ok;
}
}
-#endif
-   if (priv->flags & NFTA_FIB_F_OIF) {
-   found = FIB_RES_DEV(res);
-   if (found == oif)
-   goto ok;
-   return;
-   }
-
-   *dest = FIB_RES_DEV(res)->ifindex;
return;
+#else
+   found = FIB_RES_DEV(res);
+   if (found != oif)
+   return;
+#endif
 ok:
switch (priv->result) {
case NFT_FIB_RESULT_OIF:
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings

[Arnd Bergmann]

On Friday, October 28, 2016 5:50:31 PM CEST Florian Westphal wrote:
> Arnd Bergmann <a...@arndb.de> wrote:
> > The newly added nft fib code produces two warnings:
> > 
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> > net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' 
> > [-Werror=unused-variable]
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> > net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used 
> > uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > The first one is obvious as the only user of that variable is
> > inside of an #ifdef, but the second one is a bit trickier.
> > It is clear that 'oif' is uninitialized here if neither
> > NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
> > 
> > I have no idea how that should be handled, this patch just
> > returns without doing anything, which may or may not be
> > the right thing to do.
> 
> It should be initialized to NULL.

Ok, I had considered that, but wasn't sure if ->nh_dev could
ever be NULL, as that would then get dereferenced.

Arnd

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings

[Arnd Bergmann]

The newly added nft fib code produces two warnings:

net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' 
[-Werror=unused-variable]
net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized 
in this function [-Werror=maybe-uninitialized]

The first one is obvious as the only user of that variable is
inside of an #ifdef, but the second one is a bit trickier.
It is clear that 'oif' is uninitialized here if neither
NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.

I have no idea how that should be handled, this patch just
returns without doing anything, which may or may not be
the right thing to do.

Fixes: 84f5eedb983e ("netfilter: nf_tables: add fib expression")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/ipv4/netfilter/nft_fib_ipv4.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c 
b/net/ipv4/netfilter/nft_fib_ipv4.c
index 6787c563cfc9..b29f70593e8b 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -77,7 +77,9 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
};
const struct net_device *oif;
struct net_device *found;
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
int i;
+#endif
 
/*
 * Do not set flowi4_oif, it restricts results (for example, asking
@@ -90,6 +92,8 @@ void nft_fib4_eval(const struct nft_expr *expr, struct 
nft_regs *regs,
oif = pkt->out;
else if (priv->flags & NFTA_FIB_F_IIF)
oif = pkt->in;
+   else
+   return;
 
if (pkt->hook == NF_INET_PRE_ROUTING && fib4_is_local(pkt->skb)) {
nft_fib_store_result(dest, priv->result, pkt, LOOPBACK_IFINDEX);
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning

[Arnd Bergmann]

On Monday, October 24, 2016 10:47:54 PM CEST Julian Anastasov wrote:
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c 
> > b/net/netfilter/ipvs/ip_vs_sync.c
> > index 1b07578bedf3..9350530c16c1 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -283,6 +283,7 @@ struct ip_vs_sync_buff {
> >   */
> >  static void ntoh_seq(struct ip_vs_seq *no, struct ip_vs_seq *ho)
> >  {
> > + memset(ho, 0, sizeof(*ho));
> >   ho->init_seq   = get_unaligned_be32(>init_seq);
> >   ho->delta  = get_unaligned_be32(>delta);
> >   ho->previous_delta = get_unaligned_be32(>previous_delta);
> 
> So, now there is a double write here?

Correct. I would hope that a sane version of gcc would just not
perform the first write. What happens instead is that the version
that produces the warning here moves the initialization to the
top of the calling function.

> What about such constructs?:
> 
> *ho = (struct ip_vs_seq) {
> .init_seq   = get_unaligned_be32(>init_seq),
> ...
> };
> 
> Any difference in the compiled code or warnings?

Yes, it's one of many things I tried. What happens here is that
the warning remains as long as all fields are initialized together,
e.g. these two produces the same warning:

a)
ho->init_seq   = get_unaligned_be32(>init_seq);
ho->delta  = get_unaligned_be32(>delta);
ho->previous_delta = get_unaligned_be32(>previous_delta);

b)
   *ho = (struct ip_vs_seq) {
   .init_seq   = get_unaligned_be32(>init_seq);
   .delta  = get_unaligned_be32(>delta);
   .previous_delta = get_unaligned_be32(>previous_delta);
   };

but this one does not:

c)
   *ho = (struct ip_vs_seq) {
   .delta  = get_unaligned_be32(>delta);
   .previous_delta = get_unaligned_be32(>previous_delta);
   };
   ho->init_seq   = get_unaligned_be32(>init_seq);

I have absolutely no idea what is going on inside of gcc here.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning

[Arnd Bergmann]

Building the ip_vs_sync code with CONFIG_OPTIMIZE_INLINING on x86
confuses the compiler to the point where it produces a rather
dubious warning message:

net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.init_seq’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
  struct ip_vs_sync_conn_options opt;
 ^~~
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.delta’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.previous_delta’ may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void *)+12).init_seq’ 
may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void *)+12).delta’ may 
be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void 
*)+12).previous_delta’ may be used uninitialized in this function 
[-Werror=maybe-uninitialized]

The problem appears to be a combination of a number of factors, including
the __builtin_bswap32 compiler builtin being slightly odd, having a large
amount of code inlined into a single function, and the way that some
functions only get partially inlined here.

I've spent way too much time trying to work out a way to improve the
code, but the best I've come up with is to add an explicit memset
right before the ip_vs_seq structure is first initialized here. When
the compiler works correctly, this has absolutely no effect, but in the
case that produces the warning, the warning disappears.

In the process of analysing this warning, I also noticed that
we use memcpy to copy the larger ip_vs_sync_conn_options structure
over two members of the ip_vs_conn structure. This works because
the layout is identical, but seems error-prone, so I'm changing
this in the process to directly copy the two members. This change
seemed to have no effect on the object code or the warning, but
it deals with the same data, so I kept the two changes together.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/ipvs/ip_vs_sync.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 1b07578bedf3..9350530c16c1 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -283,6 +283,7 @@ struct ip_vs_sync_buff {
  */
 static void ntoh_seq(struct ip_vs_seq *no, struct ip_vs_seq *ho)
 {
+   memset(ho, 0, sizeof(*ho));
ho->init_seq   = get_unaligned_be32(>init_seq);
ho->delta  = get_unaligned_be32(>delta);
ho->previous_delta = get_unaligned_be32(>previous_delta);
@@ -917,8 +918,10 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, 
struct ip_vs_conn_param *pa
kfree(param->pe_data);
}
 
-   if (opt)
-   memcpy(>in_seq, opt, sizeof(*opt));
+   if (opt) {
+   cp->in_seq = opt->in_seq;
+   cp->out_seq = opt->out_seq;
+   }
atomic_set(>in_pkts, sysctl_sync_threshold(ipvs));
cp->state = state;
cp->old_state = cp->state;
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning

[Arnd Bergmann]

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 Makefile   | 10 ++
 arch/arc/Makefile  |  4 +++-
 scripts/Makefile.ubsan |  4 
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x...@kernel.org
Cc: linux-me...@vger.kernel.org
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Cc: linux-s...@vger.kernel.org
Cc: Ilya Dryomov <idryo...@gmail.com>
Cc: dri-de...@lists.freedesktop.org
Cc: linux-...@lists.infradead.org
Cc: Herbert Xu <herb...@gondor.apana.org.au>
Cc: linux-cry...@vger.kernel.org
Cc: "David S. Miller" <da...@davemloft.net>
Cc: net...@vger.kernel.org
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: ceph-de...@vger.kernel.org
Cc: linux-f2fs-de...@lists.sourceforge.net
Cc: linux-e...@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL  =
 AFLAGS_KERNEL  =
 LDFLAGS_vmlinux =
-CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  
-Wno-maybe-uninitialized
 CFLAGS_KCOV:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS  += $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS  += $(call cc-disa

[PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning

[Arnd Bergmann]

The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:

net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]

This removes the variable in question and instead moves the
condition into the switch itself, which is potentially more
efficient than adding a bogus 'default' clause as in my
first approach, and is nicer than using the 'uninitialized_var'
macro.

Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Link: http://patchwork.ozlabs.org/patch/677114/
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/nft_range.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

Cc: Pablo Neira Ayuso <pa...@netfilter.org>

diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358..2dd80f4 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -28,22 +28,20 @@ static void nft_range_eval(const struct nft_expr *expr,
 const struct nft_pktinfo *pkt)
 {
const struct nft_range_expr *priv = nft_expr_priv(expr);
-   bool mismatch;
int d1, d2;
 
d1 = memcmp(>data[priv->sreg], >data_from, priv->len);
d2 = memcmp(>data[priv->sreg], >data_to, priv->len);
switch (priv->op) {
case NFT_RANGE_EQ:
-   mismatch = (d1 < 0 || d2 > 0);
+   if (d1 < 0 || d2 > 0)
+   regs->verdict.code = NFT_BREAK;
break;
case NFT_RANGE_NEQ:
-   mismatch = (d1 >= 0 && d2 <= 0);
+   if (d1 >= 0 && d2 <= 0)
+   regs->verdict.code = NFT_BREAK;
break;
}
-
-   if (mismatch)
-   regs->verdict.code = NFT_BREAK;
 }
 
 static const struct nla_policy nft_range_policy[NFTA_RANGE_MAX + 1] = {
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/28] Reenable maybe-uninitialized warnings

[Arnd Bergmann]

This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.

After talking to Linus in person at Linaro Connect about this, I
spent some time on finding all the remaining warnings, and this
is the resulting patch series. More details are in the description
of the last patch that actually enables the warning.

Let me know if there are other warnings that I missed, and whether
you think these are still appropriate for v4.9 or not.
A couple of patches are non-obvious, and could use some more
detailed review.

Arnd

Arnd Bergmann (28):
  [v2] netfilter: nf_tables: avoid uninitialized variable warning
  [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  [v2] infiniband: shut up a maybe-uninitialized warning
  f2fs: replace a build-time warning with runtime WARN_ON
  ext2: avoid bogus -Wmaybe-uninitialized warning
  NFSv4.1: work around -Wmaybe-uninitialized warning
  ceph: avoid false positive maybe-uninitialized warning
  staging: lustre: restore initialization of return code
  staging: lustre: remove broken dead code in
cfs_cpt_table_create_pattern
  UBI: fix uninitialized access of vid_hdr pointer
  block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  [media] rc: print correct variable for z8f0811
  [media] dib0700: fix uninitialized data on 'repeat' event
  iio: accel: sca3000_core: avoid potentially uninitialized variable
  crypto: aesni: avoid -Wmaybe-uninitialized warning
  pcmcia: fix return value of soc_pcmcia_regulator_set
  spi: fsl-espi: avoid processing uninitalized data on error
  drm: avoid uninitialized timestamp use in wait_vblank
  brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  net: bcm63xx: avoid referencing uninitialized variable
  net/hyperv: avoid uninitialized variable
  x86: apm: avoid uninitialized data
  x86: mark target address as output in 'insb' asm
  x86: math-emu: possible uninitialized variable use
  s390: pci: don't print uninitialized data for debugging
  nios2: fix timer initcall return value
  rocker: fix maybe-uninitialized warning
  Kbuild: bring back -Wmaybe-uninitialized warning

 Makefile   |  10 +-
 arch/arc/Makefile  |   4 +-
 arch/nios2/kernel/time.c   |   1 +
 arch/s390/pci/pci_dma.c|   2 +-
 arch/x86/crypto/aesni-intel_glue.c | 121 +
 arch/x86/include/asm/io.h  |   4 +-
 arch/x86/kernel/apm_32.c   |   5 +-
 arch/x86/math-emu/Makefile |   4 +-
 arch/x86/math-emu/reg_compare.c|  16 +--
 drivers/block/rbd.c|   1 +
 drivers/gpu/drm/drm_irq.c  |   4 +-
 drivers/infiniband/core/cma.c  |  56 +-
 drivers/media/i2c/ir-kbd-i2c.c |   2 +-
 drivers/media/usb/dvb-usb/dib0700_core.c   |  10 +-
 drivers/mtd/nand/mtk_ecc.c |  19 ++--
 drivers/mtd/ubi/eba.c  |   2 +-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |   3 +-
 drivers/net/ethernet/rocker/rocker_ofdpa.c |   4 +-
 drivers/net/hyperv/netvsc_drv.c|   2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c |   2 +-
 drivers/pcmcia/soc_common.c|   2 +-
 drivers/spi/spi-fsl-espi.c |   2 +-
 drivers/staging/iio/accel/sca3000_core.c   |   2 +
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   |   7 --
 drivers/staging/lustre/lustre/lov/lov_pack.c   |   2 +
 fs/ceph/super.c|   3 +-
 fs/ext2/inode.c|   7 +-
 fs/f2fs/data.c |   7 ++
 fs/nfs/nfs4session.c   |  10 +-
 net/netfilter/nft_range.c  |  10 +-
 scripts/Makefile.ubsan |   4 +
 31 files changed, 187 insertions(+), 141 deletions(-)

-- 
Cc: x...@kernel.org
Cc: linux-me...@vger.kernel.org
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Cc: linux-s...@vger.kernel.org
Cc: Ilya Dryomov <idryo...@gmail.com>
Cc: dri-de...@lists.freedesktop.org
Cc: linux-...@lists.infradead.org
Cc: Herbert Xu <herb...@gondor.apana.org.au>
Cc: linux-cry...@vger.kernel.org
Cc: "David S. Miller" <da...@davemloft.net>
Cc: net...@vger.kernel.org
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: ceph-de...@vger.kernel.org
Cc: linux-f2fs-de...@lists.sourceforge.net
Cc: linux-e...@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] netfilter: xt_hashlimit: uses div_u64 for division

[Arnd Bergmann]

On Friday 30 September 2016, Eric Dumazet wrote:
> On Fri, 2016-09-30 at 18:05 +0200, Arnd Bergmann wrote:
> >  net/netfilter/xt_hashlimit.c | 17 ++---
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
> > index 44a095ecc7b7..3d5525df6eb3 100644
> > --- a/net/netfilter/xt_hashlimit.c
> > +++ b/net/netfilter/xt_hashlimit.c
> > @@ -464,20 +464,23 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
> >  static u64 user2credits(u64 user, int revision)
> >  {
> > if (revision == 1) {
> > +   u32 user32 = user; /* use 32-bit division */
> > +
> 
> This looks dangerous to me. Have you really tried all possible cases ?

Yes, I'm pretty certain about that: The 11d5f15723c9 patch that introduced this
kept the existing implementation for the revision==1 case, except for changing
the types.

> Caller (even if using revision == 1) does
> user2credits(cfg->avg * cfg->burst, revision);
> 
> Since this is not a fast path, I would prefer to keep the 64bit divide.
>
> Vishwanath version looks safer.

Ok, fair enough. I couldn't tell how much of a fast path this
was, and it's more a general issue that I see with other developers
blindly using div_u64() whenever getting this link error.

Since I already had the patch by the time I saw the other one
(which is also at v3 and got comments), I just sent it out along
with the other two patches I had for netfilter.

I also ended up introducing a typo in a last-minute change, so I'll let
Vishwanath and you work out the best implementation and withdraw my
version.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] netfilter: xt_hashlimit: uses div_u64 for division

[Arnd Bergmann]

The newly added support for high-resolution pps rates introduced multiple 64-bit
division operations in one function, which fails on all 32-bit architectures:

net/netfilter/xt_hashlimit.o: In function `user2credits':
xt_hashlimit.c:(.text.user2credits+0x3c): undefined reference to 
`__aeabi_uldivmod'
xt_hashlimit.c:(.text.user2credits+0x68): undefined reference to 
`__aeabi_uldivmod'
xt_hashlimit.c:(.text.user2credits+0x88): undefined reference to 
`__aeabi_uldivmod'

This replaces the division with an explicit call to div_u64 for version 2
to documents that this is a slow operation, and reverts back to 32-bit arguments
for the version 1 data to restore the original faster 32-bit division.

With both changes combined, we no longer get a link error.

Fixes: 11d5f15723c9 ("netfilter: xt_hashlimit: Create revision 2 to support 
higher pps rates")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Vishwanath Pai already sent a patch for this, and I did my version 
independently.
The difference is that his version also the more expensive division for the
version 1 variant that doesn't need it.

See also http://patchwork.ozlabs.org/patch/676713/
---
 net/netfilter/xt_hashlimit.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 44a095ecc7b7..3d5525df6eb3 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -464,20 +464,23 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
 static u64 user2credits(u64 user, int revision)
 {
if (revision == 1) {
+   u32 user32 = user; /* use 32-bit division */
+
/* If multiplying would overflow... */
-   if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
+   if (user32 > 0x / (HZ*CREDITS_PER_JIFFY_v1))
/* Divide first. */
-   return (user / XT_HASHLIMIT_SCALE) *\
+   return (user32 / XT_HASHLIMIT_SCALE) *
HZ * CREDITS_PER_JIFFY_v1;
 
-   return (user * HZ * CREDITS_PER_JIFFY_v1) \
-   / XT_HASHLIMIT_SCALE;
+   return (user32 * HZ * CREDITS_PER_JIFFY_v1) /
+   XT_HASHLIMIT_SCALE;
} else {
if (user > 0x / (HZ*CREDITS_PER_JIFFY))
-   return (user / XT_HASHLIMIT_SCALE_v2) *\
-   HZ * CREDITS_PER_JIFFY;
+   return div_u64_u64(user, XT_HASHLIMIT_SCALE_v2) *
+  HZ * CREDITS_PER_JIFFY;
 
-   return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE_v2;
+   return div_u64_u64(user * HZ * CREDITS_PER_JIFFY,
+  XT_HASHLIMIT_SCALE_v2);
}
 }
 
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] netfilter: hide reference to nf_hooks_ingress

[Arnd Bergmann]

A recent cleanup added an unconditional reference to the nf_hooks_ingress 
pointer,
but that fails when CONFIG_NETFILTER_INGRESS is disabled and that member is
not present in net_device:

net/netfilter/core.c: In function 'nf_set_hooks_head':
net/netfilter/core.c:96:30: error: 'struct net_device' has no member named 
'nf_hooks_ingress'

This avoids the build error by simply enclosing the assignment in an #ifdef,
which may or may not be the correct fix.

Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 576a9c0406a9..5ccff1d9f209 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -90,10 +90,12 @@ static void nf_set_hooks_head(struct net *net, const struct 
nf_hook_ops *reg,
 {
switch (reg->pf) {
case NFPROTO_NETDEV:
+#ifdef CONFIG_NETFILTER_INGRESS
/* We already checked in nf_register_net_hook() that this is
 * used from ingress.
 */
rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
+#endif
break;
default:
rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] netfilter: nf_tables: avoid uninitialized variable warning

[Arnd Bergmann]

The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:

net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]

This can be trivially avoided by added a 'default:' clause.

Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 net/netfilter/nft_range.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358482d1..72dff5bffca8 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -40,6 +40,8 @@ static void nft_range_eval(const struct nft_expr *expr,
case NFT_RANGE_NEQ:
mismatch = (d1 >= 0 && d2 <= 0);
break;
+   default:
+   mismatch = 0;
}
 
if (mismatch)
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: conntrack: remove uninitialized shadow variable

[Arnd Bergmann]

On Monday 09 May 2016 22:01:17 Pablo Neira Ayuso wrote:
> On Mon, May 09, 2016 at 09:47:23PM +0200, Arnd Bergmann wrote:
> > A recent commit introduced an unconditional use of an uninitialized
> > variable, as reported in this gcc warning:
> > 
> > net/netfilter/nf_conntrack_core.c: In function '__nf_conntrack_confirm':
> > net/netfilter/nf_conntrack_core.c:632:33: error: 'ctinfo' may be used 
> > uninitialized in this function [-Werror=maybe-uninitialized]
> >bytes = atomic64_read([CTINFO2DIR(ctinfo)].bytes);
> >  ^
> > net/netfilter/nf_conntrack_core.c:628:26: note: 'ctinfo' was declared here
> >enum ip_conntrack_info ctinfo;
> > 
> > The problem is that a local variable shadows the function parameter.
> > This removes the local variable, which looks like what Pablo originally
> > intended.
> 
> Acked-by: Pablo Neira Ayuso <pa...@netfilter.org>
> 
> Sorry for this, I wonder why gcc didn't catch up this here.
> 
> @David, you can integrate this into your net-next tree.
> 
> Thanks for fixing up this Arnd.

By default, an allmodconfig build will hide these warnings because of
excessive false positives from CONFIG_CC_OPTIMIZE_FOR_SIZE. I've
tried twice to get a patch merged that disables CONFIG_CC_OPTIMIZE_FOR_SIZE
in allmodconfig so we get better warnings, but that patch unfortunately
got ignored.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] netfilter: ctnetlink: add more #ifdef around unused code

[Arnd Bergmann]

A recent patch removed many 'inline' annotations for static
functions in this file, which has caused warnings for functions
that are not used in a given configuration, in particular when
CONFIG_NF_CONNTRACK_EVENTS is disabled:

nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used

I first tried to replace some of the existing #ifdefs with nicer
'if (IS_ENABLED())' checks, but ran into several other problems
with that, so this patch adds even more #ifdef conditionals to
avoid the remaining warnings. Another option would be to put
'__maybe_unused' annotations in place of the previous 'inline'
keyword.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 4054ff45454a ("netfilter: ctnetlink: remove unnecessary inlining")
---
 net/netfilter/nf_conntrack_netlink.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index caa4efe5930b..f893012986c7 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -336,6 +336,7 @@ nla_put_failure:
 #endif
 
 #ifdef CONFIG_NF_CONNTRACK_LABELS
+#ifdef CONFIG_NF_CONNTRACK_EVENTS
 static int ctnetlink_label_size(const struct nf_conn *ct)
 {
struct nf_conn_labels *labels = nf_ct_labels_find(ct);
@@ -344,6 +345,7 @@ static int ctnetlink_label_size(const struct nf_conn *ct)
return 0;
return nla_total_size(labels->words * sizeof(long));
 }
+#endif
 
 static int
 ctnetlink_dump_labels(struct sk_buff *skb, const struct nf_conn *ct)
@@ -526,6 +528,7 @@ nla_put_failure:
return -1;
 }
 
+#if defined(CONFIG_NF_CONNTRACK_EVENTS) || 
defined(CONFIG_NETFILTER_NETLINK_GLUE_CT)
 static size_t ctnetlink_proto_size(const struct nf_conn *ct)
 {
struct nf_conntrack_l3proto *l3proto;
@@ -543,16 +546,6 @@ static size_t ctnetlink_proto_size(const struct nf_conn 
*ct)
return len;
 }
 
-static size_t ctnetlink_acct_size(const struct nf_conn *ct)
-{
-   if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))
-   return 0;
-   return 2 * nla_total_size(0) /* CTA_COUNTERS_ORIG|REPL */
-  + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
-  + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
-  ;
-}
-
 static int ctnetlink_secctx_size(const struct nf_conn *ct)
 {
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
@@ -568,6 +561,18 @@ static int ctnetlink_secctx_size(const struct nf_conn *ct)
return 0;
 #endif
 }
+#endif
+
+#ifdef CONFIG_NF_CONNTRACK_EVENTS
+static size_t ctnetlink_acct_size(const struct nf_conn *ct)
+{
+   if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))
+   return 0;
+   return 2 * nla_total_size(0) /* CTA_COUNTERS_ORIG|REPL */
+  + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
+  + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
+  ;
+}
 
 static size_t ctnetlink_timestamp_size(const struct nf_conn *ct)
 {
@@ -580,7 +585,6 @@ static size_t ctnetlink_timestamp_size(const struct nf_conn 
*ct)
 #endif
 }
 
-#ifdef CONFIG_NF_CONNTRACK_EVENTS
 static size_t ctnetlink_nlmsg_size(const struct nf_conn *ct)
 {
return NLMSG_ALIGN(sizeof(struct nfgenmsg))
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] fix IS_ERR_VALUE usage

[Arnd Bergmann]

On Monday 15 February 2016 15:35:18 Andrzej Hajda wrote:
> 
> Andrzej Hajda (7):
>   netfilter: fix IS_ERR_VALUE usage
>   MIPS: module: fix incorrect IS_ERR_VALUE macro usages
>   drivers: char: mem: fix IS_ERROR_VALUE usage
>   atmel-isi: fix IS_ERR_VALUE usage
>   serial: clps711x: fix IS_ERR_VALUE usage
>   fbdev: exynos: fix IS_ERR_VALUE usage
>   usb: gadget: fsl_qe_udc: fix IS_ERR_VALUE usage
> 

Can you Cc me the next time on all of the patches? I only got
three of them this time.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: ipvs: avoid unused variable warnings

[Arnd Bergmann]

On Thursday 28 January 2016 08:39:53 Simon Horman wrote:
> On Wed, Jan 27, 2016 at 10:01:42PM +0200, Julian Anastasov wrote:
> > 
> >   Hello,
> > 
> > On Wed, 27 Jan 2016, Arnd Bergmann wrote:
> > 
> > > The proc_create() and remove_proc_entry() functions do not reference
> > > their arguments when CONFIG_PROC_FS is disabled, so we get a couple
> > > of warnings about unused variables in IPVS:
> > > 
> > > ipvs/ip_vs_app.c:608:14: warning: unused variable 'net' 
> > > [-Wunused-variable]
> > > ipvs/ip_vs_ctl.c:3950:14: warning: unused variable 'net' 
> > > [-Wunused-variable]
> > > ipvs/ip_vs_ctl.c:3994:14: warning: unused variable 'net' 
> > > [-Wunused-variable]
> > > 
> > > This removes the local variables and instead looks them up separately
> > > for each use, which obviously avoids the warning.
> > > 
> > > Signed-off-by: Arnd Bergmann <a...@arndb.de>
> > > Fixes: 4c50a8ce2b63 ("netfilter: ipvs: avoid unused variable warning")
> > 
> >   Looks like your previous patch for ip_vs_app_net_cleanup
> > was delayed in ipvs-next tree. I guess, Simon should drop it and
> > use this one instead when net-next opens:
> > 
> > Acked-by: Julian Anastasov <j...@ssi.bg>
> 
> Thanks, and sorry about not pushing the other patch to net-next.
> I have dropped it and queued up this one in its place.

Ah, I had not realized that the other patch was still in ipvs-next
and not merged in mainline. I did most of my testing on linux-next
(with the previous patch) and then validated the new one on
4.5-rc1, which led me to update it to contain the same hunk again.

Replacing the original patch works fine though, thanks for picking
it up!

Arnd
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html