Re: [PATCH v3] ipvs: Fix checksumming on GSO of SCTP packets
Hi, On Thu, Apr 25, 2024 at 06:28:40PM +0200, Ismael Luceno wrote: > It was observed in the wild that pairs of consecutive packets would leave > the IPVS with the same wrong checksum, and the issue only went away when > disabling GSO. > > IPVS needs to avoid computing the SCTP checksum when using GSO. This patch went already upstream. It was no clear to me that a v3 was needed. You will have to post a follow up.
Re: [PATCH v2] ipvs: Fix checksumming on GSO of SCTP packets
On Sun, Apr 21, 2024 at 04:22:32PM +0200, Ismael Luceno wrote:
> It was observed in the wild that pairs of consecutive packets would leave
> the IPVS with the same wrong checksum, and the issue only went away when
> disabling GSO.
>
> IPVS needs to avoid computing the SCTP checksum when using GSO.
I am placing this into the nf.git tree for submission upstream in the
next pull request, unless stated otherwise.
Thanks.
> Fixes: 90017accff61 ("sctp: Add GSO support", 2016-06-02)
> Co-developed-by: Firo Yang
> Signed-off-by: Ismael Luceno
> Tested-by: Andreas Taschner
> CC: Michal Kubeček
> CC: Simon Horman
> CC: Julian Anastasov
> CC: lvs-de...@vger.kernel.org
> CC: netfilter-de...@vger.kernel.org
> CC: net...@vger.kernel.org
> CC: coret...@netfilter.org
> ---
>
> Notes:
> Changes since v1:
> * Added skb_is_gso before skb_is_gso_sctp.
> * Added "Fixes" tag.
>
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index a0921adc31a9..1e689c714127 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -126,7 +126,8 @@ sctp_snat_handler(struct sk_buff *skb, struct
> ip_vs_protocol *pp,
> if (sctph->source != cp->vport || payload_csum ||
> skb->ip_summed == CHECKSUM_PARTIAL) {
> sctph->source = cp->vport;
> - sctp_nat_csum(skb, sctph, sctphoff);
> + if (!skb_is_gso(skb) || !skb_is_gso_sctp(skb))
> + sctp_nat_csum(skb, sctph, sctphoff);
> } else {
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> }
> @@ -174,7 +175,8 @@ sctp_dnat_handler(struct sk_buff *skb, struct
> ip_vs_protocol *pp,
> (skb->ip_summed == CHECKSUM_PARTIAL &&
>!(skb_dst(skb)->dev->features & NETIF_F_SCTP_CRC))) {
> sctph->dest = cp->dport;
> - sctp_nat_csum(skb, sctph, sctphoff);
> + if (!skb_is_gso(skb) || !skb_is_gso_sctp(skb))
> + sctp_nat_csum(skb, sctph, sctphoff);
> } else if (skb->ip_summed != CHECKSUM_PARTIAL) {
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> }
> --
> 2.43.0
>
>
Re: [PATCH net] net: ipvs: avoid stat macros calls from preemptible context
On Mon, Jan 15, 2024 at 05:39:22PM +0300, Fedor Pchelkin wrote: > Inside decrement_ttl() upon discovering that the packet ttl has exceeded, > __IP_INC_STATS and __IP6_INC_STATS macros can be called from preemptible > context having the following backtrace: > > check_preemption_disabled: 48 callbacks suppressed > BUG: using __this_cpu_add() in preemptible [] code: curl/1177 > caller is decrement_ttl+0x217/0x830 Applied to nf.git, thanks
Re: drivers/net/ethernet/mediatek/mtk_ppe_offload.c - suspicious code?
On Sun, Apr 18, 2021 at 09:02:12PM -0400, Valdis Klētnieks wrote:
> While doing some code auditing for -Woverride_init, I spotted some
> questionable code
>
> commit 502e84e2382d92654a2ecbc52cdbdb5a11cdcec7
> Author: Felix Fietkau
> Date: Wed Mar 24 02:30:54 2021 +0100
>
> net: ethernet: mtk_eth_soc: add flow offloading support
>
> In drivers/net/ethernet/mediatek/mtk_ppe_offload.c:
>
> +static const struct rhashtable_params mtk_flow_ht_params = {
> + .head_offset = offsetof(struct mtk_flow_entry, node),
> + .head_offset = offsetof(struct mtk_flow_entry, cookie),
> + .key_len = sizeof(unsigned long),
> + .automatic_shrinking = true,
> +};
>
> What's intended for head_offset here?
It's a bug, there's a fix here:
https://www.spinics.net/lists/netdev/msg736368.html
Re: linux-next: build failure after merge of the net-next tree
On Mon, Apr 12, 2021 at 03:04:16PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the net-next tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> In file included from include/asm-generic/bug.h:20,
> from arch/x86/include/asm/bug.h:93,
> from include/linux/bug.h:5,
> from include/linux/mmdebug.h:5,
> from include/linux/gfp.h:5,
> from include/linux/umh.h:4,
> from include/linux/kmod.h:9,
> from net/bridge/netfilter/ebtables.c:14:
> net/bridge/netfilter/ebtables.c: In function '__ebt_find_table':
> net/bridge/netfilter/ebtables.c:1248:33: error: 'struct netns_xt' has no
> member named 'tables'
> 1248 | list_for_each_entry(t, >xt.tables[NFPROTO_BRIDGE], list) {
> | ^
> include/linux/kernel.h:708:26: note: in definition of macro 'container_of'
> 708 | void *__mptr = (void *)(ptr); \
> | ^~~
> include/linux/list.h:522:2: note: in expansion of macro 'list_entry'
> 522 | list_entry((ptr)->next, type, member)
> | ^~
> include/linux/list.h:628:13: note: in expansion of macro 'list_first_entry'
> 628 | for (pos = list_first_entry(head, typeof(*pos), member); \
> | ^~~~
> net/bridge/netfilter/ebtables.c:1248:2: note: in expansion of macro
> 'list_for_each_entry'
> 1248 | list_for_each_entry(t, >xt.tables[NFPROTO_BRIDGE], list) {
> | ^~~
> In file included from :
> net/bridge/netfilter/ebtables.c:1248:33: error: 'struct netns_xt' has no
> member named 'tables'
> 1248 | list_for_each_entry(t, >xt.tables[NFPROTO_BRIDGE], list) {
> | ^
> include/linux/compiler_types.h:300:9: note: in definition of macro
> '__compiletime_assert'
> 300 | if (!(condition)) \
> | ^
> include/linux/compiler_types.h:320:2: note: in expansion of macro
> '_compiletime_assert'
> 320 | _compiletime_assert(condition, msg, __compiletime_assert_,
> __COUNTER__)
> | ^~~
> include/linux/build_bug.h:39:37: note: in expansion of macro
> 'compiletime_assert'
>39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ^~
> include/linux/kernel.h:709:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 709 | BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> | ^~~~
> include/linux/kernel.h:709:20: note: in expansion of macro '__same_type'
> 709 | BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> |^~~
> include/linux/list.h:511:2: note: in expansion of macro 'container_of'
> 511 | container_of(ptr, type, member)
> | ^~~~
> include/linux/list.h:522:2: note: in expansion of macro 'list_entry'
> 522 | list_entry((ptr)->next, type, member)
> | ^~
> include/linux/list.h:628:13: note: in expansion of macro 'list_first_entry'
> 628 | for (pos = list_first_entry(head, typeof(*pos), member); \
> | ^~~~
> net/bridge/netfilter/ebtables.c:1248:2: note: in expansion of macro
> 'list_for_each_entry'
> 1248 | list_for_each_entry(t, >xt.tables[NFPROTO_BRIDGE], list) {
> | ^~~
> net/bridge/netfilter/ebtables.c:1248:33: error: 'struct netns_xt' has no
> member named 'tables'
> 1248 | list_for_each_entry(t, >xt.tables[NFPROTO_BRIDGE], list) {
> | ^
> include/linux/compiler_types.h:300:9: note: in definition of macro
> '__compiletime_assert'
> 300 | if (!(condition)) \
> | ^
> include/linux/compiler_types.h:320:2: note: in expansion of macro
> '_compiletime_assert'
> 320 | _compiletime_assert(condition, msg, __compiletime_assert_,
> __COUNTER__)
> | ^~~
> include/linux/build_bug.h:39:37: note: in expansion of macro
> 'compiletime_assert'
>39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ^~
> include/linux/kernel.h:709:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 709 | BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> | ^~~~
> include/linux/kernel.h:710:6: note: in expansion of macro '__same_type'
> 710 | !__same_type(*(ptr), void), \
> | ^~~
> include/linux/list.h:511:2: note: in expansion of macro 'container_of'
> 511 | container_of(ptr, type, member)
> | ^~~~
> include/linux/list.h:522:2: note: in expansion of macro 'list_entry'
> 522 | list_entry((ptr)->next, type, member)
> | ^~
> include/linux/list.h:628:13: note: in expansion of macro 'list_first_entry'
> 628 | for (pos = list_first_entry(head,
Re: [PATCH v5] audit: log nftables configuration change events once per table
On Wed, Mar 31, 2021 at 04:53:10PM -0400, Richard Guy Briggs wrote: > On 2021-03-31 22:22, Pablo Neira Ayuso wrote: > > On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote: > > > Reduce logging of nftables events to a level similar to iptables. > > > Restore the table field to list the table, adding the generation. > > > > > > Indicate the op as the most significant operation in the event. > > > > There's a UAF, Florian reported. I'm attaching an incremental fix. > > > > nf_tables_commit_audit_collect() refers to the trans object which > > might have been already released. > > Got it. Thanks Pablo. I didn't see it when running nft-test.py Where > was it reported? CONFIG_KASAN. > Here I tried to stay out of the way by putting that > call at the end of the loop but that was obviously a mistake in > hindsight. :-) No problem, I'll squash this incremental fix into your audit patch.
Re: [PATCH v5] audit: log nftables configuration change events once per table
On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote:
> @@ -8006,12 +7966,65 @@ static void nft_commit_notify(struct net *net, u32
> portid)
> WARN_ON_ONCE(!list_empty(>nft.notify_list));
> }
>
> +static int nf_tables_commit_audit_alloc(struct list_head *adl,
> + struct nft_table *table)
> +{
> + struct nft_audit_data *adp;
> +
> + list_for_each_entry(adp, adl, list) {
> + if (adp->table == table)
> + return 0;
> + }
> + adp = kzalloc(sizeof(*adp), GFP_KERNEL);
> + if (!adp)
> + return -ENOMEM;
> + adp->table = table;
> + INIT_LIST_HEAD(>list);
This INIT_LIST_HEAD is not required for an object that is going to be
inserted into the 'adl' list.
> + list_add(>list, adl);
If no objections, I'll amend this patch. I'll include the UAF fix and
remove this unnecessary INIT_LIST_HEAD.
Re: [PATCH v5] audit: log nftables configuration change events once per table
On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote:
> Reduce logging of nftables events to a level similar to iptables.
> Restore the table field to list the table, adding the generation.
>
> Indicate the op as the most significant operation in the event.
There's a UAF, Florian reported. I'm attaching an incremental fix.
nf_tables_commit_audit_collect() refers to the trans object which
might have been already released.
commit e4d272948d25b66d86fc241cefd95281bfb1079e
Author: Pablo Neira Ayuso
Date: Wed Mar 31 22:19:51 2021 +0200
netfilter: nf_tables: use-after-free
Signed-off-by: Pablo Neira Ayuso
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5dd4bb7cabf5..01674c0d9103 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8063,6 +8063,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
net->nft.gencursor = nft_gencursor_next(net);
list_for_each_entry_safe(trans, next, >nft.commit_list, list) {
+ nf_tables_commit_audit_collect(, trans->ctx.table,
+ trans->msg_type);
switch (trans->msg_type) {
case NFT_MSG_NEWTABLE:
if (nft_trans_table_update(trans)) {
@@ -8211,8 +8213,6 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
}
break;
}
- nf_tables_commit_audit_collect(, trans->ctx.table,
- trans->msg_type);
}
nft_commit_notify(net, NETLINK_CB(skb).portid);
Re: [PATCH][next] netfilter: nf_log_bridge: Fix missing assignment of ret on a call to nf_log_register
On Wed, Mar 31, 2021 at 03:26:06PM +0100, Colin King wrote: > From: Colin Ian King > > Currently the call to nf_log_register is returning an error code that > is not being assigned to ret and yet ret is being checked. Fix this by > adding in the missing assignment. Applied, thanks.
Re: [PATCH] netfilter: ipset: Remove duplicate declaration
On Sun, Mar 28, 2021 at 09:30:49PM +0200, Jozsef Kadlecsik wrote: > On Sat, 27 Mar 2021, Wan Jiabing wrote: > > > struct ip_set is declared twice. One is declared at 79th line, > > so remove the duplicate. > > Yes, it's a duplicate. Pablo, could you apply it? Applied, thanks Jozsef.
Re: [PATCH v5] audit: log nftables configuration change events once per table
On Sun, Mar 28, 2021 at 08:50:45PM -0400, Paul Moore wrote: [...] > Netfilter folks, were you planning to pull this via your tree/netdev > or would you like me to merge this via the audit tree? If the latter, > I would appreciate it if I could get an ACK from one of you; if the > former, my ACK is below. > > Acked-by: Paul Moore I'll merge this one into nf-next, this might simplify possible conflict resolution later on. Thanks for acking.
Re: [PATCH v2] audit: log nftables configuration change events once per table
On Mon, Mar 22, 2021 at 04:49:04PM -0400, Richard Guy Briggs wrote:
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index c1eb5cdb3033..42ba44890523 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
[...]
> @@ -8006,12 +7938,47 @@ static void nft_commit_notify(struct net *net, u32
> portid)
> WARN_ON_ONCE(!list_empty(>nft.notify_list));
> }
>
> +void nf_tables_commit_audit_collect(struct list_head *adl,
> + struct nft_trans *trans) {
nitpick: curly brace starts in the line.
> + struct nft_audit_data *adp;
> +
> + list_for_each_entry(adp, adl, list) {
> + if (adp->table == trans->ctx.table)
> + goto found;
> + }
> + adp = kzalloc(sizeof(*adp), GFP_KERNEL);
if (!adp)
...
> + adp->table = trans->ctx.table;
> + INIT_LIST_HEAD(>list);
> + list_add(>list, adl);
> +found:
> + adp->entries++;
> + if (!adp->op || adp->op > trans->msg_type)
> + adp->op = trans->msg_type;
> +}
> +
> +#define AUNFTABLENAMELEN (NFT_TABLE_MAXNAMELEN + 22)
> +
> +void nf_tables_commit_audit_log(struct list_head *adl, u32 generation) {
^
same thing here
> + struct nft_audit_data *adp, *adn;
> + char aubuf[AUNFTABLENAMELEN];
> +
> + list_for_each_entry_safe(adp, adn, adl, list) {
> + snprintf(aubuf, AUNFTABLENAMELEN, "%s:%u", adp->table->name,
> + generation);
> + audit_log_nfcfg(aubuf, adp->table->family, adp->entries,
> + nft2audit_op[adp->op], GFP_KERNEL);
> + list_del(>list);
> + kfree(adp);
> + }
> +}
> +
> static int nf_tables_commit(struct net *net, struct sk_buff *skb)
> {
> struct nft_trans *trans, *next;
> struct nft_trans_elem *te;
> struct nft_chain *chain;
> struct nft_table *table;
> + LIST_HEAD(adl);
> int err;
>
> if (list_empty(>nft.commit_list)) {
> @@ -8206,12 +8173,15 @@ static int nf_tables_commit(struct net *net, struct
> sk_buff *skb)
> }
> break;
> }
> + nf_tables_commit_audit_collect(, trans);
> }
>
> nft_commit_notify(net, NETLINK_CB(skb).portid);
> nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
> nf_tables_commit_release(net);
>
> + nf_tables_commit_audit_log(, net->nft.base_seq);
> +
This looks more self-contained and nicer, thanks.
Re: [PATCH] audit: log nftables configuration change events once per table
On Thu, Mar 18, 2021 at 11:39:52AM -0400, Richard Guy Briggs wrote:
> Reduce logging of nftables events to a level similar to iptables.
> Restore the table field to list the table, adding the generation.
>
> Indicate the op as the most significant operation in the event.
>
> A couple of sample events:
>
> type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) :
> proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64
> syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
> a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root
> euid=root suid=root fsuid=root egid=roo
> t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
> exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
> family=ipv6 entries=1 op=nft_register_table pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
> family=ipv4 entries=1 op=nft_register_table pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
> family=inet entries=1 op=nft_register_table pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>
> type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) :
> proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64
> syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
> a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root
> euid=root suid=root fsuid=root egid=r
> oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
> exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
> family=ipv6 entries=30 op=nft_register_chain pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
> family=ipv4 entries=30 op=nft_register_chain pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
> family=inet entries=165 op=nft_register_chain pid=367
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>
> The issue was originally documented in
> https://github.com/linux-audit/audit-kernel/issues/124
>
> Signed-off-by: Richard Guy Briggs
> ---
> include/linux/audit.h | 29
> net/netfilter/nf_tables_api.c | 132 +-
> 2 files changed, 78 insertions(+), 83 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 82b7c1116a85..bba6a0386742 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -118,6 +118,35 @@ enum audit_nfcfgop {
> AUDIT_NFT_OP_INVALID,
> };
>
> +static const u8 nft2audit_op[] = { // enum nf_tables_msg_types
> + /* NFT_MSG_NEWTABLE */ AUDIT_NFT_OP_TABLE_REGISTER,
> + /* NFT_MSG_GETTABLE */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELTABLE */ AUDIT_NFT_OP_TABLE_UNREGISTER,
> + /* NFT_MSG_NEWCHAIN */ AUDIT_NFT_OP_CHAIN_REGISTER,
> + /* NFT_MSG_GETCHAIN */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELCHAIN */ AUDIT_NFT_OP_CHAIN_UNREGISTER,
> + /* NFT_MSG_NEWRULE */ AUDIT_NFT_OP_RULE_REGISTER,
> + /* NFT_MSG_GETRULE */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELRULE */ AUDIT_NFT_OP_RULE_UNREGISTER,
> + /* NFT_MSG_NEWSET */ AUDIT_NFT_OP_SET_REGISTER,
> + /* NFT_MSG_GETSET */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELSET */ AUDIT_NFT_OP_SET_UNREGISTER,
> + /* NFT_MSG_NEWSETELEM */ AUDIT_NFT_OP_SETELEM_REGISTER,
> + /* NFT_MSG_GETSETELEM */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELSETELEM */ AUDIT_NFT_OP_SETELEM_UNREGISTER,
> + /* NFT_MSG_NEWGEN */ AUDIT_NFT_OP_GEN_REGISTER,
> + /* NFT_MSG_GETGEN */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_TRACE*/ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_NEWOBJ */ AUDIT_NFT_OP_OBJ_REGISTER,
> + /* NFT_MSG_GETOBJ */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELOBJ */ AUDIT_NFT_OP_OBJ_UNREGISTER,
> + /* NFT_MSG_GETOBJ_RESET */ AUDIT_NFT_OP_OBJ_RESET,
> + /* NFT_MSG_NEWFLOWTABLE */ AUDIT_NFT_OP_FLOWTABLE_REGISTER,
> + /* NFT_MSG_GETFLOWTABLE */ AUDIT_NFT_OP_INVALID,
> + /* NFT_MSG_DELFLOWTABLE */ AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> + /* NFT_MSG_MAX */ AUDIT_NFT_OP_INVALID,
> +};
> +
> extern int is_audit_feature_set(int which);
>
> extern int __init audit_register_class(int class, unsigned
Re: [PATCH net-next] netfilter: conntrack: Remove unused variable declaration
On Thu, Mar 11, 2021 at 01:55:59PM +0800, YueHaibing wrote:
> commit e97c3e278e95 ("tproxy: split off ipv6 defragmentation to a separate
> module") left behind this.
Applied, thanks.
Re: [PATCH RESEND][next] netfilter: Fix fall-through warnings for Clang
On Fri, Mar 05, 2021 at 02:42:09AM -0600, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple > warnings by explicitly adding multiple break statements instead of just > letting the code fall through to the next case. Applied to nf-next, thanks.
Re: [PATCH v2 0/3] Don't use RCU for x_tables synchronization
On Mon, Mar 08, 2021 at 02:24:10PM +1300, Mark Tomlinson wrote: > The patches to change to using RCU synchronization in x_tables cause > updating tables to be slowed down by an order of magnitude. This has > been tried before, see https://lore.kernel.org/patchwork/patch/151796/ > and ultimately was rejected. As mentioned in the patch description, a > different method can be used to ensure ordering of reads/writes. This > can simply be done by changing from smp_wmb() to smp_mb(). Applied.
Re: Panic after upgrading to 5.11.6 stable
On Sun, Mar 14, 2021 at 10:30:55AM +, David R wrote: > I attempted to upgrade my home server to 5.11 today. The system panics > soon after boot with the following :- > > In iptables by the looks of the stack. > > 5.10.23 works fine. > > Can provide config (and boot logs from 5.10.23) if required. Please have a look at: https://bugzilla.kernel.org/show_bug.cgi?id=211911
Re: [PATCH] uapi: nfnetlink_cthelper.h: fix userspace compilation error
On Mon, Feb 22, 2021 at 08:00:00AM +, Dmitry V. Levin wrote: > Apparently, and > could not be included into the same > compilation unit because of a cut-and-paste typo in the former header. Applied, thanks.
Re: [PATCH] netfilter: nf_tables: remove redundant assignment of variable err
On Thu, Feb 04, 2021 at 12:04:21AM +0100, Pablo Neira Ayuso wrote: > On Thu, Jan 28, 2021 at 05:59:23PM +, Colin King wrote: > > From: Colin Ian King > > > > The variable err is being assigned a value that is never read, > > the same error number is being returned at the error return > > path via label err1. Clean up the code by removing the assignment. > > Applied to nf, thanks. Sorry, I meant, nf-next
Re: [PATCH] netfilter: nf_tables: remove redundant assignment of variable err
On Thu, Jan 28, 2021 at 05:59:23PM +, Colin King wrote: > From: Colin Ian King > > The variable err is being assigned a value that is never read, > the same error number is being returned at the error return > path via label err1. Clean up the code by removing the assignment. Applied to nf, thanks.
Re: [PATCH] netfilter: Fix memleak in nf_nat_init
On Sat, Jan 09, 2021 at 08:01:21PM +0800, Dinghao Liu wrote: > When register_pernet_subsys() fails, nf_nat_bysource > should be freed just like when nf_ct_extend_register() > fails. Applied, thanks.
Re: [PATCH] selftests: netfilter: Pass family parameter "-f" to conntrack tool
On Tue, Jan 05, 2021 at 11:31:20PM +0800, Chen Yi wrote: > Fix nft_conntrack_helper.sh false fail report: > > 1) Conntrack tool need "-f ipv6" parameter to show out ipv6 traffic items. > > 2) Sleep 1 second after background nc send packet, to make sure check > is after this statement executed. > > False report: > FAIL: ns1-lkjUemYw did not show attached helper ip set via ruleset > PASS: ns1-lkjUemYw connection on port 2121 has ftp helper attached > ... > > After fix: > PASS: ns1-2hUniwU2 connection on port 2121 has ftp helper attached > PASS: ns2-2hUniwU2 connection on port 2121 has ftp helper attached > ... Applied.
Re: [PATCH net] selftests: netfilter: Pass the family parameter to conntrack tool
Please, Cc netfilter-de...@vger.kernel.org, and a more few comments
below.
On Mon, Jan 04, 2021 at 07:07:23PM +0800, Yi Chen wrote:
> From: yiche
>
> Fix nft_conntrack_helper.sh fake fail:
> conntrack tool need "-f ipv6" parameter to show out ipv6 traffic items.
> sleep 1 second after background nc send packet, to make sure check
> result after this statement is executed.
Missing Fixes: tag ?
> Signed-off-by: yiche
> ---
> .../selftests/netfilter/nft_conntrack_helper.sh | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/netfilter/nft_conntrack_helper.sh
> b/tools/testing/selftests/netfilter/nft_conntrack_helper.sh
> index edf0a48da6bf..ebdf2b23c8e3 100755
> --- a/tools/testing/selftests/netfilter/nft_conntrack_helper.sh
> +++ b/tools/testing/selftests/netfilter/nft_conntrack_helper.sh
> @@ -94,7 +94,13 @@ check_for_helper()
> local message=$2
> local port=$3
>
> - ip netns exec ${netns} conntrack -L -p tcp --dport $port 2> /dev/null
> |grep -q 'helper=ftp'
> + if [[ "$2" =~ "ipv6" ]];then
> + local family=ipv6
> + else
> + local family=ipv4
This branch coding style diverges from the existing code.
> + fi
Re: [PATCH][next] netfilter: nftables: fix incorrect increment of loop counter
On Mon, Dec 14, 2020 at 11:40:15PM +, Colin King wrote: > From: Colin Ian King > > The intention of the err_expr cleanup path is to iterate over the > allocated expr_array objects and free them, starting from i - 1 and > working down to the start of the array. Currently the loop counter > is being incremented instead of decremented and also the index i is > being used instead of k, repeatedly destroying the same expr_array > element. Fix this by decrementing k and using k as the index into > expr_array. Applied to nf.git, thanks.
Re: [PATCH][next] netfilter: nftables: fix incorrect increment of loop counter
On Tue, Dec 15, 2020 at 03:38:30PM +0100, Pablo Neira Ayuso wrote:
> Hi,
>
> On Mon, Dec 14, 2020 at 11:40:15PM +, Colin King wrote:
> > From: Colin Ian King
> >
> > The intention of the err_expr cleanup path is to iterate over the
> > allocated expr_array objects and free them, starting from i - 1 and
> > working down to the start of the array. Currently the loop counter
> > is being incremented instead of decremented and also the index i is
> > being used instead of k, repeatedly destroying the same expr_array
> > element. Fix this by decrementing k and using k as the index into
> > expr_array.
> >
> > Addresses-Coverity: ("Infinite loop")
> > Fixes: 8cfd9b0f8515 ("netfilter: nftables: generalize set expressions
> > support")
> > Signed-off-by: Colin Ian King
>
> Reviewed-by: Pablo Neira Ayuso
>
> @Jakub: Would you please take this one into net-next? Thanks!
You marked as "Awaiting Upstream", I'll take care of it.
Thanks.
Re: [PATCH][next] netfilter: nftables: fix incorrect increment of loop counter
Hi,
On Mon, Dec 14, 2020 at 11:40:15PM +, Colin King wrote:
> From: Colin Ian King
>
> The intention of the err_expr cleanup path is to iterate over the
> allocated expr_array objects and free them, starting from i - 1 and
> working down to the start of the array. Currently the loop counter
> is being incremented instead of decremented and also the index i is
> being used instead of k, repeatedly destroying the same expr_array
> element. Fix this by decrementing k and using k as the index into
> expr_array.
>
> Addresses-Coverity: ("Infinite loop")
> Fixes: 8cfd9b0f8515 ("netfilter: nftables: generalize set expressions
> support")
> Signed-off-by: Colin Ian King
Reviewed-by: Pablo Neira Ayuso
@Jakub: Would you please take this one into net-next? Thanks!
> ---
> net/netfilter/nf_tables_api.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 8d5aa0ac45f4..4186b1e52d58 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5254,8 +5254,8 @@ static int nft_set_elem_expr_clone(const struct nft_ctx
> *ctx,
> return 0;
>
> err_expr:
> - for (k = i - 1; k >= 0; k++)
> - nft_expr_destroy(ctx, expr_array[i]);
> + for (k = i - 1; k >= 0; k--)
> + nft_expr_destroy(ctx, expr_array[k]);
>
> return -ENOMEM;
> }
> --
> 2.29.2
>
Re: [PATCH] netfilter: Remove unnecessary conversion to bool
On Fri, Nov 06, 2020 at 04:20:13PM +0800, xiakaixu1...@gmail.com wrote: > From: Kaixu Xia > > Here we could use the '!=' expression to fix the following coccicheck > warning: > > ./net/netfilter/xt_nfacct.c:30:41-46: WARNING: conversion to bool not needed > here Applied.
Re: [PATCH net v3] ipvs: fix possible memory leak in ip_vs_control_net_init
On Tue, Nov 24, 2020 at 08:09:19PM +0200, Julian Anastasov wrote: > > Hello, > > On Tue, 24 Nov 2020, Wang Hai wrote: > > > kmemleak report a memory leak as follows: > > > > BUG: memory leak > > unreferenced object 0x8880759ea000 (size 256): > > backtrace: > > [] kmem_cache_zalloc include/linux/slab.h:656 [inline] > > [ ] __proc_create+0x23d/0x7d0 fs/proc/generic.c:421 > > [<9d718d02>] proc_create_reg+0x8e/0x140 fs/proc/generic.c:535 > > [<97bbfc4f>] proc_create_net_data+0x8c/0x1b0 fs/proc/proc_net.c:126 > > [<652480fc>] ip_vs_control_net_init+0x308/0x13a0 > > net/netfilter/ipvs/ip_vs_ctl.c:4169 > > [<4c927ebe>] __ip_vs_init+0x211/0x400 > > net/netfilter/ipvs/ip_vs_core.c:2429 > > [ ] ops_init+0xa8/0x3c0 net/core/net_namespace.c:151 > > [<153fd114>] setup_net+0x2de/0x7e0 net/core/net_namespace.c:341 > > [ ] copy_net_ns+0x27d/0x530 net/core/net_namespace.c:482 > > [ ] create_new_namespaces+0x382/0xa30 kernel/nsproxy.c:110 > > [<098a5757>] copy_namespaces+0x2e6/0x3b0 kernel/nsproxy.c:179 > > [<26ce39e9>] copy_process+0x220a/0x5f00 kernel/fork.c:2072 > > [ ] _do_fork+0xc7/0xda0 kernel/fork.c:2428 > > [<2974ee96>] __do_sys_clone3+0x18a/0x280 kernel/fork.c:2703 > > [<62ac0a4d>] do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 > > [<93f1ce2c>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > In the error path of ip_vs_control_net_init(), remove_proc_entry() needs > > to be called to remove the added proc entry, otherwise a memory leak > > will occur. > > > > Also, add some '#ifdef CONFIG_PROC_FS' because proc_create_net* return NULL > > when PROC is not used. > > > > Fixes: b17fc9963f83 ("IPVS: netns, ip_vs_stats and its procfs") > > Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.") > > Reported-by: Hulk Robot > > Signed-off-by: Wang Hai > > Looks good to me, thanks! > > Acked-by: Julian Anastasov Applied, thanks.
Re: [PATCH net-next,v5 0/9] netfilter: flowtable bridge and vlan enhancements
On Sun, Nov 22, 2020 at 02:51:18PM +, Alexander Lobakin wrote: > From: Pablo Neira Ayuso > Date: Sun, 22 Nov 2020 12:42:19 +0100 > > > On Sun, Nov 22, 2020 at 10:26:16AM +, Alexander Lobakin wrote: > >> From: Pablo Neira Ayuso > >> Date: Fri, 20 Nov 2020 13:49:12 +0100 > > [...] > >>> Something like this: > >>> > >>>fast path > >>> .. > >>>/ \ > >>>| IP forwarding | > >>>| / \ . > >>>| br0 eth0 > >>>. / \ > >>>-- veth1 veth2 > >>>. > >>>. > >>>. > >>> eth0 > >>>ab:cd:ef:ab:cd:ef > >>> VM > >> > >> I'm concerned about bypassing vlan and bridge's .ndo_start_xmit() in > >> case of this shortcut. We'll have incomplete netdevice Tx stats for > >> these two, as it gets updated inside this callbacks. > > > > TX device stats are being updated accordingly. > > > > # ip netns exec nsr1 ip -s link > > 1: lo: mtu 65536 qdisc noop state DOWN mode DEFAULT group > > default qlen 1000 > > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > RX: bytes packets errors dropped overrun mcast > > 0 00 0 0 0 > > TX: bytes packets errors dropped carrier collsns > > 0 00 0 0 0 > > 2: veth0@if2: mtu 1500 qdisc noqueue > > state UP mode DEFAULT group default qlen 1000 > > link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff link-netns ns1 > > RX: bytes packets errors dropped overrun mcast > > 213290848248 4869765 0 0 0 0 > > TX: bytes packets errors dropped carrier collsns > > 315346667 4777953 0 0 0 0 > > 3: veth1@if2: mtu 1500 qdisc noqueue > > state UP mode DEFAULT group default qlen 1000 > > link/ether 4a:81:2d:9a:02:88 brd ff:ff:ff:ff:ff:ff link-netns ns2 > > RX: bytes packets errors dropped overrun mcast > > 315337919 4777833 0 0 0 0 > > TX: bytes packets errors dropped carrier collsns > > 213290844826 4869708 0 0 0 0 > > 4: br0: mtu 1500 qdisc noqueue state UP > > mode DEFAULT group default qlen 1000 > > link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff > > RX: bytes packets errors dropped overrun mcast > > 4101 73 0 0 0 0 > > TX: bytes packets errors dropped carrier collsns > > 5256 74 0 0 0 0 > > Aren't these counters very low for br0, despite that br0 is an > intermediate point of traffic flow? Most packets follow the flowtable fast path, which is bypassing the br0 device. Bumping br0 stats would be misleading, it would make the user think that the packets follow the classic bridge layer path, while they do not. The flowtable have counters itself to allow the user to collect stats regarding the packets that follow the fastpath.
Re: [PATCH] ipvs: replace atomic_add_return()
On Tue, Nov 17, 2020 at 10:57:52PM +0200, Julian Anastasov wrote: > > Hello, > > On Mon, 16 Nov 2020, Yejune Deng wrote: > > > atomic_inc_return() looks better > > > > Signed-off-by: Yejune Deng > > Looks good to me for -next, thanks! > > Acked-by: Julian Anastasov Applied, thanks.
Re: [PATCH net-next,v5 0/9] netfilter: flowtable bridge and vlan enhancements
On Sun, Nov 22, 2020 at 10:26:16AM +, Alexander Lobakin wrote: > From: Pablo Neira Ayuso > Date: Fri, 20 Nov 2020 13:49:12 +0100 [...] > > Something like this: > > > >fast path > > .. > >/ \ > >| IP forwarding | > >| / \ . > >| br0 eth0 > >. / \ > >-- veth1 veth2 > >. > >. > >. > > eth0 > >ab:cd:ef:ab:cd:ef > > VM > > I'm concerned about bypassing vlan and bridge's .ndo_start_xmit() in > case of this shortcut. We'll have incomplete netdevice Tx stats for > these two, as it gets updated inside this callbacks. TX device stats are being updated accordingly. # ip netns exec nsr1 ip -s link 1: lo: mtu 65536 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 RX: bytes packets errors dropped overrun mcast 0 00 0 0 0 TX: bytes packets errors dropped carrier collsns 0 00 0 0 0 2: veth0@if2: mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000 link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff link-netns ns1 RX: bytes packets errors dropped overrun mcast 213290848248 4869765 0 0 0 0 TX: bytes packets errors dropped carrier collsns 315346667 4777953 0 0 0 0 3: veth1@if2: mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000 link/ether 4a:81:2d:9a:02:88 brd ff:ff:ff:ff:ff:ff link-netns ns2 RX: bytes packets errors dropped overrun mcast 315337919 4777833 0 0 0 0 TX: bytes packets errors dropped carrier collsns 213290844826 4869708 0 0 0 0 4: br0: mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000 link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff RX: bytes packets errors dropped overrun mcast 4101 73 0 0 0 0 TX: bytes packets errors dropped carrier collsns 5256 74 0 0 0 0 5: veth0.10@veth0: mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000 link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff RX: bytes packets errors dropped overrun mcast 4101 73 0 0 0 62 TX: bytes packets errors dropped carrier collsns 315342363 4777893 0 0 0 0
Re: [PATCH] MAINTAINERS: rectify file patterns for NETFILTER
Hi Lukas,
On Sun, Nov 15, 2020 at 07:58:33PM -0800, Joe Perches wrote:
> On Mon, 2020-11-09 at 10:19 +0100, Lukas Bulwahn wrote:
> > The two file patterns in the NETFILTER section:
> >
> > F: include/linux/netfilter*
> > F: include/uapi/linux/netfilter*
> >
> > intended to match the directories:
> >
> > ./include{/uapi}/linux/netfilter_{arp,bridge,ipv4,ipv6}
> >
> > A quick check with ./scripts/get_maintainer.pl --letters -f will show that
> > they are not matched, though, because this pattern only matches files, but
> > not directories.
> >
> > Rectify the patterns to match the intended directories.
> []
> diff --git a/MAINTAINERS b/MAINTAINERS
> []
> > @@ -12139,10 +12139,10 @@ W:http://www.nftables.org/
> > Q: http://patchwork.ozlabs.org/project/netfilter-devel/list/
> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
> > -F: include/linux/netfilter*
> > +F: include/linux/netfilter*/
> > F: include/linux/netfilter/
>
> This line could be deleted or perhaps moved up one line above
>
> F:include/linux/netfilter/
> F:include/linux/netfilter*/
>
> (as the second line already matches the first line's files too)
>
> > F: include/net/netfilter/
> > -F: include/uapi/linux/netfilter*
> > +F: include/uapi/linux/netfilter*/
> > F: include/uapi/linux/netfilter/
>
> same here.
>
> > F: net/*/netfilter.c
> > F: net/*/netfilter/
Please, send a v2 to address this feedback. Thank you.
Re: [PATCH] MAINTAINERS: rectify file patterns for NETFILTER
On Mon, Nov 09, 2020 at 10:19:42AM +0100, Lukas Bulwahn wrote:
> The two file patterns in the NETFILTER section:
>
> F: include/linux/netfilter*
> F: include/uapi/linux/netfilter*
>
> intended to match the directories:
>
> ./include{/uapi}/linux/netfilter_{arp,bridge,ipv4,ipv6}
>
> A quick check with ./scripts/get_maintainer.pl --letters -f will show that
> they are not matched, though, because this pattern only matches files, but
> not directories.
>
> Rectify the patterns to match the intended directories.
Applied, thanks.
Re: [PATCH v22 16/23] LSM: security_secid_to_secctx in netlink netfilter
Hi Casey,
On Wed, Nov 04, 2020 at 04:49:17PM -0800, Casey Schaufler wrote:
> Change netlink netfilter interfaces to use lsmcontext
> pointers, and remove scaffolding.
>
> Reviewed-by: Kees Cook
> Reviewed-by: John Johansen
> Acked-by: Stephen Smalley
> Signed-off-by: Casey Schaufler
> Cc: net...@vger.kernel.org
> Cc: netfilter-de...@vger.kernel.org
You can carry this tag in your follow up patches.
Acked-by: Pablo Neira Ayuso
Thanks.
> ---
> net/netfilter/nfnetlink_queue.c | 37 +
> 1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 84be5a49a157..0d8b83d84422 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -301,15 +301,13 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb,
> struct sock *sk)
> return -1;
> }
>
> -static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
> +static void nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsmcontext
> *context)
> {
> - u32 seclen = 0;
> #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
> struct lsmblob blob;
> - struct lsmcontext context = { };
>
> if (!skb || !sk_fullsock(skb->sk))
> - return 0;
> + return;
>
> read_lock_bh(>sk->sk_callback_lock);
>
> @@ -318,14 +316,12 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb,
> char **secdata)
>* blob. security_secid_to_secctx() will know which security
>* module to use to create the secctx. */
> lsmblob_init(, skb->secmark);
> - security_secid_to_secctx(, );
> - *secdata = context.context;
> + security_secid_to_secctx(, context);
> }
>
> read_unlock_bh(>sk->sk_callback_lock);
> - seclen = context.len;
> #endif
> - return seclen;
> + return;
> }
>
> static u32 nfqnl_get_bridge_size(struct nf_queue_entry *entry)
> @@ -398,12 +394,10 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> struct net_device *indev;
> struct net_device *outdev;
> struct nf_conn *ct = NULL;
> + struct lsmcontext context = { };
> enum ip_conntrack_info ctinfo;
> struct nfnl_ct_hook *nfnl_ct;
> bool csum_verify;
> - struct lsmcontext scaff; /* scaffolding */
> - char *secdata = NULL;
> - u32 seclen = 0;
>
> size = nlmsg_total_size(sizeof(struct nfgenmsg))
> + nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
> @@ -469,9 +463,9 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> }
>
> if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
> - seclen = nfqnl_get_sk_secctx(entskb, );
> - if (seclen)
> - size += nla_total_size(seclen);
> + nfqnl_get_sk_secctx(entskb, );
> + if (context.len)
> + size += nla_total_size(context.len);
> }
>
> skb = alloc_skb(size, GFP_ATOMIC);
> @@ -604,7 +598,8 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> nfqnl_put_sk_uidgid(skb, entskb->sk) < 0)
> goto nla_put_failure;
>
> - if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata))
> + if (context.len &&
> + nla_put(skb, NFQA_SECCTX, context.len, context.context))
> goto nla_put_failure;
>
> if (ct && nfnl_ct->build(skb, ct, ctinfo, NFQA_CT, NFQA_CT_INFO) < 0)
> @@ -632,10 +627,8 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> }
>
> nlh->nlmsg_len = skb->len;
> - if (seclen) {
> - lsmcontext_init(, secdata, seclen, 0);
> - security_release_secctx();
> - }
> + if (context.len)
> + security_release_secctx();
> return skb;
>
> nla_put_failure:
> @@ -643,10 +636,8 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> kfree_skb(skb);
> net_err_ratelimited("nf_queue: error creating packet message\n");
> nlmsg_failure:
> - if (seclen) {
> - lsmcontext_init(, secdata, seclen, 0);
> - security_release_secctx();
> - }
> + if (context.len)
> + security_release_secctx();
> return NULL;
> }
>
> --
> 2.24.1
>
Re: [PATCH linux-5.9 1/1] net: netfilter: fix KASAN: slab-out-of-bounds Read in nft_flow_rule_create
On Thu, Oct 29, 2020 at 12:02:41PM +0100, Greg KH wrote:
> On Tue, Oct 27, 2020 at 09:19:22AM +0100, Pablo Neira Ayuso wrote:
> > Hi Greg,
> >
> > On Tue, Oct 27, 2020 at 07:21:11AM +0100, Greg KH wrote:
> > > On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> > > > Adding stable.
> > >
> > > What did that do?
> >
> > Saeed is requesting that stable maintainers cherry-picks this patch:
> >
> > 31cc578ae2de ("netfilter: nftables_offload: KASAN slab-out-of-bounds
> > Read in nft_flow_rule_create")
> >
> > into stable 5.4 and 5.8.
>
> 5.9 is also a stable kernel :)
Oh, indeed, I forgot this one :)
> Will go queue it up everywhere...
Thanks.
Re: [PATCH linux-5.9 1/1] net: netfilter: fix KASAN: slab-out-of-bounds Read in nft_flow_rule_create
Hi Greg,
On Tue, Oct 27, 2020 at 07:21:11AM +0100, Greg KH wrote:
> On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> > Adding stable.
>
> What did that do?
Saeed is requesting that stable maintainers cherry-picks this patch:
31cc578ae2de ("netfilter: nftables_offload: KASAN slab-out-of-bounds
Read in nft_flow_rule_create")
into stable 5.4 and 5.8.
Thanks.
Re: [PATCH v5] ipvs: adjust the debug info in function set_tcp_state
On Wed, Sep 30, 2020 at 08:08:02AM +0300, Julian Anastasov wrote: > > Hello, > > On Mon, 28 Sep 2020, longguang.yue wrote: > > > Outputting client,virtual,dst addresses info when tcp state changes, > > which makes the connection debug more clear > > > > Signed-off-by: longguang.yue > > OK, v5 can be used instead of fixing v4. > > Acked-by: Julian Anastasov Applied, thanks.
Re: [PATCH nf v2] netfilter: conntrack: connection timeout after re-register
On Wed, Oct 07, 2020 at 12:32:52PM -0700, Francesco Ruggeri wrote: > If the first packet conntrack sees after a re-register is an outgoing > keepalive packet with no data (SEG.SEQ = SND.NXT-1), td_end is set to > SND.NXT-1. > When the peer correctly acknowledges SND.NXT, tcp_in_window fails > check III (Upper bound for valid (s)ack: sack <= receiver.td_end) and > returns false, which cascades into nf_conntrack_in setting > skb->_nfct = 0 and in later conntrack iptables rules not matching. > In cases where iptables are dropping packets that do not match > conntrack rules this can result in idle tcp connections to time out. Applied, thanks.
Re: [PATCH linux-5.9 1/1] net: netfilter: fix KASAN: slab-out-of-bounds Read in nft_flow_rule_create
On Mon, Oct 19, 2020 at 10:25:32AM -0700, saeed.mirzamohamm...@oracle.com wrote:
> From: Saeed Mirzamohammadi
>
> This patch fixes the issue due to:
>
> BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
> net/netfilter/nf_tables_offload.c:40
> Read of size 8 at addr 888103910b58 by task syz-executor227/16244
>
> The error happens when expr->ops is accessed early on before performing the
> boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
>
> This patch checks the boundary condition before expr->ops that fixes the
> slab-out-of-bounds Read issue.
Thanks. I made a slight variant of your patch.
I'm attaching it, it is also fixing the problem but it introduced
nft_expr_more() and use it everywhere.
Let me know if this looks fine to you.
>From 3f60e5f489ec44e8b0a7e9e622c93be4df335fb6 Mon Sep 17 00:00:00 2001
From: Saeed Mirzamohammadi
Date: Tue, 20 Oct 2020 13:41:36 +0200
Subject: [PATCH nf] netfilter: fix KASAN: slab-out-of-bounds Read in
nft_flow_rule_create
This patch fixes the issue due to:
BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
net/netfilter/nf_tables_offload.c:40
Read of size 8 at addr 888103910b58 by task syz-executor227/16244
The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.
Add nft_expr_more() and use it to fix this problem.
Signed-off-by: Saeed Mirzamohammadi
Signed-off-by: Pablo Neira Ayuso
---
include/net/netfilter/nf_tables.h | 6 ++
net/netfilter/nf_tables_api.c | 6 +++---
net/netfilter/nf_tables_offload.c | 4 ++--
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 3f7e56b1171e..55b4cadf290a 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -891,6 +891,12 @@ static inline struct nft_expr *nft_expr_last(const struct nft_rule *rule)
return (struct nft_expr *)>data[rule->dlen];
}
+static inline bool nft_expr_more(const struct nft_rule *rule,
+ const struct nft_expr *expr)
+{
+ return expr != nft_expr_last(rule) && expr->ops;
+}
+
static inline struct nft_userdata *nft_userdata(const struct nft_rule *rule)
{
return (void *)>data[rule->dlen];
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9957e0ed8658..65cb8e3c13d9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -302,7 +302,7 @@ static void nft_rule_expr_activate(const struct nft_ctx *ctx,
struct nft_expr *expr;
expr = nft_expr_first(rule);
- while (expr != nft_expr_last(rule) && expr->ops) {
+ while (nft_expr_more(rule, expr)) {
if (expr->ops->activate)
expr->ops->activate(ctx, expr);
@@ -317,7 +317,7 @@ static void nft_rule_expr_deactivate(const struct nft_ctx *ctx,
struct nft_expr *expr;
expr = nft_expr_first(rule);
- while (expr != nft_expr_last(rule) && expr->ops) {
+ while (nft_expr_more(rule, expr)) {
if (expr->ops->deactivate)
expr->ops->deactivate(ctx, expr, phase);
@@ -3080,7 +3080,7 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
* is called on error from nf_tables_newrule().
*/
expr = nft_expr_first(rule);
- while (expr != nft_expr_last(rule) && expr->ops) {
+ while (nft_expr_more(rule, expr)) {
next = nft_expr_next(expr);
nf_tables_expr_destroy(ctx, expr);
expr = next;
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 7c7e06624dc3..9f625724a20f 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -37,7 +37,7 @@ struct nft_flow_rule *nft_flow_rule_create(struct net *net,
struct nft_expr *expr;
expr = nft_expr_first(rule);
- while (expr->ops && expr != nft_expr_last(rule)) {
+ while (nft_expr_more(rule, expr)) {
if (expr->ops->offload_flags & NFT_OFFLOAD_F_ACTION)
num_actions++;
@@ -61,7 +61,7 @@ struct nft_flow_rule *nft_flow_rule_create(struct net *net,
ctx->net = net;
ctx->dep.type = NFT_OFFLOAD_DEP_UNSPEC;
- while (expr->ops && expr != nft_expr_last(rule)) {
+ while (nft_expr_more(rule, expr)) {
if (!expr->ops->offload) {
err = -EOPNOTSUPP;
goto err_out;
--
2.20.1
Re: [PATCH nf v2] netfilter: conntrack: connection timeout after re-register
On Wed, Oct 14, 2020 at 02:06:28AM +0200, Pablo Neira Ayuso wrote: > On Fri, Oct 09, 2020 at 10:05:48PM +0200, Florian Westphal wrote: > > Jozsef Kadlecsik wrote: > > > > The "delay unregister" remark was wrt. the "all rules were deleted" > > > > case, i.e. add a "grace period" rather than acting right away when > > > > conntrack use count did hit 0. > > > > > > Now I understand it, thanks really. The hooks are removed, so conntrack > > > cannot "see" the packets and the entries become stale. > > > > Yes. > > > > > What is the rationale behind "remove the conntrack hooks when there are > > > no > > > rule left referring to conntrack"? Performance optimization? But then the > > > content of the whole conntrack table could be deleted too... ;-) > > > > Yes, this isn't the case at the moment -- only hooks are removed, > > entries will eventually time out. > > > > > > Conntrack entries are not removed, only the base hooks get > > > > unregistered. > > > > This is a problem for tcp window tracking. > > > > > > > > When re-register occurs, kernel is supposed to switch the existing > > > > entries to "loose" mode so window tracking won't flag packets as > > > > invalid, but apparently this isn't enough to handle keepalive case. > > > > > > "loose" (nf_ct_tcp_loose) mode doesn't disable window tracking, it > > > enables/disables picking up already established connections. > > > > > > nf_ct_tcp_be_liberal would disable TCP window checking (but not tracking) > > > for non RST packets. > > > > You are right, mixup on my part. > > > > > But both seems to be modified only via the proc entries. > > > > Yes, we iterate table on re-register and modify the existing entries. > > For iptables-nft, it might be possible to avoid this deregister + > register ct hooks in the same transaction: Maybe add something like > nf_ct_netns_get_all() to bump refcounters by one _iff_ they are > 0 > before starting the transaction processing, then call > nf_ct_netns_put_all() which decrements refcounters and unregister > hooks if they reach 0. Hm, scratch that, put_all() would create an imbalance with this conditional increment. > The only problem with this approach is that this pulls in the > conntrack module, to solve that, struct nf_ct_hook in > net/netfilter/core.c could be used to store the reference to > ->netns_get_all and ->net_put_all. > > Legacy would still be flawed though.
Re: [PATCH nf v2] netfilter: conntrack: connection timeout after re-register
On Fri, Oct 09, 2020 at 10:05:48PM +0200, Florian Westphal wrote: > Jozsef Kadlecsik wrote: > > > The "delay unregister" remark was wrt. the "all rules were deleted" > > > case, i.e. add a "grace period" rather than acting right away when > > > conntrack use count did hit 0. > > > > Now I understand it, thanks really. The hooks are removed, so conntrack > > cannot "see" the packets and the entries become stale. > > Yes. > > > What is the rationale behind "remove the conntrack hooks when there are no > > rule left referring to conntrack"? Performance optimization? But then the > > content of the whole conntrack table could be deleted too... ;-) > > Yes, this isn't the case at the moment -- only hooks are removed, > entries will eventually time out. > > > > Conntrack entries are not removed, only the base hooks get unregistered. > > > This is a problem for tcp window tracking. > > > > > > When re-register occurs, kernel is supposed to switch the existing > > > entries to "loose" mode so window tracking won't flag packets as > > > invalid, but apparently this isn't enough to handle keepalive case. > > > > "loose" (nf_ct_tcp_loose) mode doesn't disable window tracking, it > > enables/disables picking up already established connections. > > > > nf_ct_tcp_be_liberal would disable TCP window checking (but not tracking) > > for non RST packets. > > You are right, mixup on my part. > > > But both seems to be modified only via the proc entries. > > Yes, we iterate table on re-register and modify the existing entries. For iptables-nft, it might be possible to avoid this deregister + register ct hooks in the same transaction: Maybe add something like nf_ct_netns_get_all() to bump refcounters by one _iff_ they are > 0 before starting the transaction processing, then call nf_ct_netns_put_all() which decrements refcounters and unregister hooks if they reach 0. The only problem with this approach is that this pulls in the conntrack module, to solve that, struct nf_ct_hook in net/netfilter/core.c could be used to store the reference to ->netns_get_all and ->net_put_all. Legacy would still be flawed though.
Re: [PATCH net-next] netfilter: nf_tables_offload: Remove unused macro FLOW_SETUP_BLOCK
On Fri, Sep 18, 2020 at 09:17:29PM +0800, YueHaibing wrote:
> commit 9a32669fecfb ("netfilter: nf_tables_offload: support indr block call")
> left behind this.
Applied.
Re: [PATCH net-next] ipvs: Remove unused macros
On Mon, Sep 21, 2020 at 09:24:40AM +0200, Simon Horman wrote:
> On Fri, Sep 18, 2020 at 09:16:56PM +0800, YueHaibing wrote:
> > They are not used since commit e4ff67513096 ("ipvs: add
> > sync_maxlen parameter for the sync daemon")
> >
> > Signed-off-by: YueHaibing
>
> Thanks, this look good to me.
>
> Acked-by: Simon Horman
>
> Pablo, please consider this for nf-next.
Applied, thanks.
Re: [PATCH net-next] netfilter: ebt_stp: Remove unused macro BPDU_TYPE_TCN
On Fri, Sep 04, 2020 at 08:56:53PM +0800, Wang Hai wrote: > BPDU_TYPE_TCN is never used after it was introduced. > So better to remove it. Applied, thanks.
Re: [PATCHv5 net-next] ipvs: remove dependency on ip6_tables
On Mon, Aug 31, 2020 at 08:12:05PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Sat, 29 Aug 2020, Yaroslav Bolyukin wrote:
>
> > This dependency was added because ipv6_find_hdr was in iptables specific
> > code but is no longer required
> >
> > Fixes: f8f626754ebe ("ipv6: Move ipv6_find_hdr() out of Netfilter code.")
> > Fixes: 63dca2c0b0e7 ("ipvs: Fix faulty IPv6 extension header handling in
> > IPVS").
> > Signed-off-by: Yaroslav Bolyukin
>
> Looks good to me, thanks! May be maintainers will
> remove the extra dot after the Fixes line.
Applied, thanks. I have also removed the extra dot.
Re: [PATCH] netfilter: nf_conntrack_sip: fix parsing error
On Fri, Aug 28, 2020 at 02:14:48PM -0400, Tong Zhang wrote: > Hi Pablo, > I'm not an expert in this networking stuff. > But from my point of view there's no point in checking if this > condition is always true. Understood. > There's also no need of returning anything from the > ct_sip_parse_numerical_param() > if they are all being ignored like this. Then probably update this code to ignore the return value?
Re: [PATCH] netfilter: nf_conntrack_sip: fix parsing error
On Sat, Aug 15, 2020 at 12:50:30PM -0400, Tong Zhang wrote: > ct_sip_parse_numerical_param can only return 0 or 1, but the caller is > checking parsing error using < 0 Is this are real issue in your setup or probably some static analysis tool is reporting? You are right that ct_sip_parse_numerical_param() never returns < 0, however, looking at: https://tools.ietf.org/html/rfc3261 see Page 161 expires is optional, my understanding is that your patch is making this option mandatory.
Re: [PATCH net-next] netfilter: xt_HMARK: Use ip_is_fragment() helper
On Thu, Aug 27, 2020 at 10:08:13PM +0800, YueHaibing wrote: > Use ip_is_fragment() to simpify code. Applied.
Re: [Linux-kernel-mentees] [PATCH net-next v2] ipvs: Fix uninit-value in do_ip_vs_set_ctl()
On Tue, Aug 11, 2020 at 03:46:40AM -0400, Peilin Ye wrote: > do_ip_vs_set_ctl() is referencing uninitialized stack value when `len` is > zero. Fix it. Applied to nf-next, thanks.
Re: [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum
On Fri, Aug 28, 2020 at 06:45:51PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso wrote:
> > Hi Will,
> >
> > Given this is for -stable maintainers only, I'd suggest:
> >
> > 1) Specify what -stable kernel versions this patch applies to.
> >Explain that this problem is gone since what kernel version.
> >
> > 2) Maybe clarify that this is only for stable in the patch subject,
> >e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4
>
> Hmm, we silently accept a tuple that we can't really deal with, no?
Oh, I overlook, existing kernels are affected. You're right.
> > > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6)
> > > + return -EOPNOTSUPP;
>
> I vote to apply this to nf.git
I have rebased this patch on top of nf.git, attached what I'll apply
to nf.git.
>From 4d3426b91eba6eb28f38a2b06ee024aff861aa16 Mon Sep 17 00:00:00 2001
From: Will McVicker
Date: Mon, 24 Aug 2020 19:38:32 +
Subject: [PATCH] netfilter: ctnetlink: add a range check for l3/l4 protonum
The indexes to the nf_nat_l[34]protos arrays come from userspace. So
check the tuple's family, e.g. l3num, when creating the conntrack in
order to prevent an OOB memory access during setup. Here is an example
kernel panic on 4.14.180 when userspace passes in an index greater than
NFPROTO_NUMPROTO.
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:...
Process poc (pid: 5614, stack limit = 0xa3933121)
CPU: 4 PID: 5614 Comm: poc Tainted: G S W O4.14.180-g051355490483
Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM
task: 2a3dfffe task.stack: a3933121
pc : __cfi_check_fail+0x1c/0x24
lr : __cfi_check_fail+0x1c/0x24
...
Call trace:
__cfi_check_fail+0x1c/0x24
name_to_dev_t+0x0/0x468
nfnetlink_parse_nat_setup+0x234/0x258
ctnetlink_parse_nat_setup+0x4c/0x228
ctnetlink_new_conntrack+0x590/0xc40
nfnetlink_rcv_msg+0x31c/0x4d4
netlink_rcv_skb+0x100/0x184
nfnetlink_rcv+0xf4/0x180
netlink_unicast+0x360/0x770
netlink_sendmsg+0x5a0/0x6a4
___sys_sendmsg+0x314/0x46c
SyS_sendmsg+0xb4/0x108
el0_svc_naked+0x34/0x38
Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack")
Signed-off-by: Will McVicker
[pa...@netfilter.org: rebased original patch on top of nf.git]
Signed-off-by: Pablo Neira Ayuso
---
net/netfilter/nf_conntrack_netlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 832eabecfbdd..d65846aa8059 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1404,7 +1404,8 @@ ctnetlink_parse_tuple_filter(const struct nlattr * const cda[],
if (err < 0)
return err;
-
+ if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6)
+ return -EOPNOTSUPP;
tuple->src.l3num = l3num;
if (flags & CTA_FILTER_FLAG(CTA_IP_DST) ||
--
2.20.1
Re: [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum
Hi Will,
Given this is for -stable maintainers only, I'd suggest:
1) Specify what -stable kernel versions this patch applies to.
Explain that this problem is gone since what kernel version.
2) Maybe clarify that this is only for stable in the patch subject,
e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4
Otherwise, this -stable maintainers might not identify this patch as
something that is targetted to them.
Thanks.
On Mon, Aug 24, 2020 at 07:38:32PM +, Will McVicker wrote:
> The indexes to the nf_nat_l[34]protos arrays come from userspace. So
> check the tuple's family, e.g. l3num, when creating the conntrack in
> order to prevent an OOB memory access during setup. Here is an example
> kernel panic on 4.14.180 when userspace passes in an index greater than
> NFPROTO_NUMPROTO.
>
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> Modules linked in:...
> Process poc (pid: 5614, stack limit = 0xa3933121)
> CPU: 4 PID: 5614 Comm: poc Tainted: G S W O4.14.180-g051355490483
> Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM
> task: 2a3dfffe task.stack: a3933121
> pc : __cfi_check_fail+0x1c/0x24
> lr : __cfi_check_fail+0x1c/0x24
> ...
> Call trace:
> __cfi_check_fail+0x1c/0x24
> name_to_dev_t+0x0/0x468
> nfnetlink_parse_nat_setup+0x234/0x258
> ctnetlink_parse_nat_setup+0x4c/0x228
> ctnetlink_new_conntrack+0x590/0xc40
> nfnetlink_rcv_msg+0x31c/0x4d4
> netlink_rcv_skb+0x100/0x184
> nfnetlink_rcv+0xf4/0x180
> netlink_unicast+0x360/0x770
> netlink_sendmsg+0x5a0/0x6a4
> ___sys_sendmsg+0x314/0x46c
> SyS_sendmsg+0xb4/0x108
> el0_svc_naked+0x34/0x38
>
> Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack")
> Signed-off-by: Will McVicker
> ---
> 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 31fa94064a62..0b89609a6e9d 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[],
> if (!tb[CTA_TUPLE_IP])
> return -EINVAL;
>
> + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6)
> + return -EOPNOTSUPP;
> tuple->src.l3num = l3num;
>
> err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple);
> --
> 2.28.0.297.g1956fa8f8d-goog
>
Re: [Linux-kernel-mentees] [PATCH net-next v2] ipvs: Fix uninit-value in do_ip_vs_set_ctl()
On Tue, Aug 11, 2020 at 02:59:59PM +0200, Simon Horman wrote: > On Tue, Aug 11, 2020 at 01:29:04PM +0300, Julian Anastasov wrote: > > > > Hello, > > > > On Tue, 11 Aug 2020, Peilin Ye wrote: > > > > > do_ip_vs_set_ctl() is referencing uninitialized stack value when `len` is > > > zero. Fix it. > > > > > > Reported-by: syzbot+23b5f9e7caf61d9a3...@syzkaller.appspotmail.com > > > Link: > > > https://syzkaller.appspot.com/bug?id=46ebfb92a8a812621a001ef04d90dfa459520fe2 > > > Suggested-by: Julian Anastasov > > > Signed-off-by: Peilin Ye > > > > Looks good to me, thanks! > > > > Acked-by: Julian Anastasov > > Pablo, could you consider this for nf-next or should we repost when > net-next re-opens? No worries, it will sit in netfilter's patchwork until net-next reopens.
Re: [PATCH nf] netfilter: nft_compat: remove flush counter optimization
On Sun, Aug 09, 2020 at 08:28:01PM +0200, Florian Westphal wrote: > WARNING: CPU: 1 PID: 16059 at lib/refcount.c:31 > refcount_warn_saturate+0xdf/0xf > [..] > __nft_mt_tg_destroy+0x42/0x50 [nft_compat] > nft_target_destroy+0x63/0x80 [nft_compat] > nf_tables_expr_destroy+0x1b/0x30 [nf_tables] > nf_tables_rule_destroy+0x3a/0x70 [nf_tables] > nf_tables_exit_net+0x186/0x3d0 [nf_tables] > > Happens when a compat expr is destoyed from abort path. > There is no functional impact; after this work queue is flushed > unconditionally if its pending. > > This removes the waitcount optimization. Test of repeated > iptables-restore of a ~60k kubernetes ruleset doesn't indicate > a slowdown. In case the counter is needed after all for some workloads > we can revert this and increment the refcount for the > != NFT_PREPARE_TRANS case to avoid the increment/decrement imbalance. > > While at it, also flush for match case, this was an oversight > in the original patch. Applied, thanks.
Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum
Hi, This patch is much smaller and if you confirm this is address the issue, then this is awesome. On Mon, Aug 03, 2020 at 06:31:56PM +, William Mcvicker wrote: [...] > diff --git a/net/netfilter/nf_conntrack_netlink.c > b/net/netfilter/nf_conntrack_netlink.c > index 31fa94064a62..56d310f8b29a 100644 > --- a/net/netfilter/nf_conntrack_netlink.c > +++ b/net/netfilter/nf_conntrack_netlink.c > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], > if (!tb[CTA_TUPLE_IP]) > return -EINVAL; > > + if (l3num >= NFPROTO_NUMPROTO) > + return -EINVAL; l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6. Other than that, bail out with EOPNOTSUPP. Thank you.
Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[]
Hi William, On Fri, Jul 31, 2020 at 12:26:11AM +, William Mcvicker wrote: > Hi Pablo, > > Yes, I believe this oops is only triggered by userspace when the user > specifically passes in an invalid nf_nat_l3protos index. I'm happy to re-work > the patch to check for this in ctnetlink_create_conntrack(). Great. Note that this code does not exist in the tree anymore. I'm not sure if this problem still exists upstream, this patch does not apply to nf.git. This fix should only go for -stable maintainers. > > BTW, do you have a Fixes: tag for this? This will be useful for > > -stable maintainer to pick up this fix. > > Regarding the Fixes: tag, I don't have one offhand since this bug was reported > to me, but I can search through the code history to find the commit that > exposed this vulnerability. That would be great. Thank you.
Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[]
Hi Will, On Mon, Jul 27, 2020 at 05:57:20PM +, Will McVicker wrote: > The indexes to the nf_nat_l[34]protos arrays come from userspace. So we > need to make sure that before indexing the arrays, we verify the index > is within the array bounds in order to prevent an OOB memory access. > Here is an example kernel panic on 4.14.180 when userspace passes in an > index greater than NFPROTO_NUMPROTO. > > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > Modules linked in:... > Process poc (pid: 5614, stack limit = 0xa3933121) > CPU: 4 PID: 5614 Comm: poc Tainted: G S W O4.14.180-g051355490483 > Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM > task: 2a3dfffe task.stack: a3933121 > pc : __cfi_check_fail+0x1c/0x24 > lr : __cfi_check_fail+0x1c/0x24 > ... > Call trace: > __cfi_check_fail+0x1c/0x24 > name_to_dev_t+0x0/0x468 > nfnetlink_parse_nat_setup+0x234/0x258 If this oops is only triggerable from userspace, I think a sanity check in nfnetlink_parse_nat_setup should suffice to reject unsupported layer 3 and layer 4 protocols. I mean, in this patch I see more chunks in the packet path, such as nf_nat_l3proto_ipv4 that should never happen. I would just fix the userspace ctnetlink path. BTW, do you have a Fixes: tag for this? This will be useful for -stable maintainer to pick up this fix. Thanks.
Re: [PATCH] netfilter: ip6tables: Remove redundant null checks
Applied, thanks.
Re: [PATCH v2] netfilter: Replace HTTP links with HTTPS ones
On Sat, Jul 25, 2020 at 07:02:25PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. Applied.
Re: [PATCH for v5.9] netfilter: Replace HTTP links with HTTPS ones
On Sun, Jul 19, 2020 at 01:52:02PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. LGTM. Can you squash this patch into this? netfilter: xtables: Replace HTTP links with HTTPS ones Probably better if this can be done for the entire netfilter tree in one single patch. Thanks.
Re: [PATCH v2][next] netfilter: Use fallthrough pseudo-keyword
On Wed, Jul 08, 2020 at 03:09:39PM -0500, Gustavo A. R. Silva wrote: > Replace the existing /* fall through */ comments and its variants with > the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary > fall-through markings when it is the case. Applied, thanks.
Re: [PATCH][next] netfilter: nf_tables: Use fallthrough pseudo-keyword
On Tue, Jul 07, 2020 at 02:47:17PM -0500, Gustavo A. R. Silva wrote:
> Replace the existing /* fall through */ comments and its variants with
> the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
> fall-through markings when it is the case.
I suggest:
netfilter: Use fallthrough pseudo-keyword
instead, since this is also updating iptables and ipset codebase.
More comments below, thanks.
> [1]
> https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> net/bridge/netfilter/ebtables.c |2 +-
> net/netfilter/ipset/ip_set_core.c |2 +-
> net/netfilter/nf_conntrack_h323_asn1.c |6 +++---
> net/netfilter/nf_conntrack_proto.c |2 +-
> net/netfilter/nf_conntrack_proto_tcp.c |2 +-
> net/netfilter/nf_conntrack_standalone.c |2 +-
> net/netfilter/nf_nat_core.c | 10 +-
> net/netfilter/nf_synproxy_core.c|6 ++
> net/netfilter/nf_tables_api.c |8
> net/netfilter/nf_tables_core.c |2 +-
> net/netfilter/nfnetlink_cttimeout.c |2 +-
> net/netfilter/nft_cmp.c |4 ++--
> net/netfilter/nft_ct.c |4 ++--
> net/netfilter/nft_fib.c |2 +-
> net/netfilter/nft_payload.c |2 +-
> net/netfilter/utils.c |8
> net/netfilter/x_tables.c|2 +-
> 17 files changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index c83ffe912163..38e946fdd041 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -1935,7 +1935,7 @@ static int compat_mtw_from_user(const struct
> compat_ebt_entry_mwt *mwt,
> size_kern = match_size;
> module_put(match->me);
> break;
> - case EBT_COMPAT_WATCHER: /* fallthrough */
> + case EBT_COMPAT_WATCHER:
> case EBT_COMPAT_TARGET:
> wt = xt_request_find_target(NFPROTO_BRIDGE, name,
> mwt->u.revision);
> diff --git a/net/netfilter/ipset/ip_set_core.c
> b/net/netfilter/ipset/ip_set_core.c
> index 340cb955af25..7d1f6d2da3f1 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -1642,7 +1642,7 @@ ip_set_dump_do(struct sk_buff *skb, struct
> netlink_callback *cb)
> goto next_set;
> if (set->variant->uref)
> set->variant->uref(set, cb, true);
> - /* fall through */
> + fallthrough;
> default:
> ret = set->variant->list(set, skb, cb);
> if (!cb->args[IPSET_CB_ARG0])
> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c
> b/net/netfilter/nf_conntrack_h323_asn1.c
> index 573cb4481481..e697a824b001 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -257,15 +257,15 @@ static unsigned int get_uint(struct bitstr *bs, int b)
> case 4:
> v |= *bs->cur++;
> v <<= 8;
> - /* fall through */
> + fallthrough;
> case 3:
> v |= *bs->cur++;
> v <<= 8;
> - /* fall through */
> + fallthrough;
> case 2:
> v |= *bs->cur++;
> v <<= 8;
> - /* fall through */
> + fallthrough;
> case 1:
> v |= *bs->cur++;
> break;
> diff --git a/net/netfilter/nf_conntrack_proto.c
> b/net/netfilter/nf_conntrack_proto.c
> index a0560d175a7f..95f79980348c 100644
> --- a/net/netfilter/nf_conntrack_proto.c
> +++ b/net/netfilter/nf_conntrack_proto.c
> @@ -610,7 +610,7 @@ void nf_ct_netns_put(struct net *net, uint8_t nfproto)
> switch (nfproto) {
> case NFPROTO_BRIDGE:
> nf_ct_netns_do_put(net, NFPROTO_BRIDGE);
> - /* fall through */
> + fallthrough;
> case NFPROTO_INET:
> nf_ct_netns_do_put(net, NFPROTO_IPV4);
> nf_ct_netns_do_put(net, NFPROTO_IPV6);
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index 1926fd56df56..6892e497781c 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -900,7 +900,7 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
> return -NF_REPEAT;
> return NF_DROP;
> }
> - /* Fall through */
> + fallthrough;
> case TCP_CONNTRACK_IGNORE:
> /* Ignored packets:
>*
> diff --git a/net/netfilter/nf_conntrack_standalone.c
>
Re: [PATCH] [net/ipv6] Remove redundant null check in ah_mt6
Hi Gaurav, On Wed, Jun 24, 2020 at 10:36:25PM -0400, Gaurav Singh wrote: > ah cannot be NULL since its already checked above after > assignment and is being dereferenced before in pr(). > Remove the redundant null check. Could you collapse all your patches into one? They look like the same logic change (patch description is the same in the four patches in the series). Please, prepend netfilter: to your patch subject, I suggest the following subject for the collapsed patch. netfilter: ip6tables: Remove redundant null checks Thanks.
Re: [PATCH ghak124 v3] audit: log nftables configuration change events
On Wed, Jun 24, 2020 at 08:34:23AM -0400, Richard Guy Briggs wrote:
> On 2020-06-24 12:03, Pablo Neira Ayuso wrote:
> > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote:
[...]
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 3558e76e2733..b9e7440cc87d 100644
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -12,6 +12,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct
> > > nft_ctx *ctx, int event)
> > > {
> > > struct sk_buff *skb;
> > > int err;
> > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
> > > + ctx->table->name, ctx->table->handle);
> > > +
> > > + audit_log_nfcfg(buf,
> > > + ctx->family,
> > > + ctx->table->use,
> > > + event == NFT_MSG_NEWTABLE ?
> > > + AUDIT_NFT_OP_TABLE_REGISTER :
> > > + AUDIT_NFT_OP_TABLE_UNREGISTER);
> > > + kfree(buf);
> >
> > As a follow up: Would you wrap this code into a function?
> >
> > nft_table_audit()
> >
> > Same thing for other pieces of code below.
>
> If I'm guessing right, you are asking for a supplementary follow-up
> cleanup patch to this one (or are you nacking this patch)?
No nack, it's just that I'd prefer to see this wrapped in a function.
I think your patch is already in the audit tree.
> Also, I gather you would like to see the kasprintf and kfree hidden in
> nft_table_audit(), handing this function at least 8 parameters? This
> sounds pretty messy given the format of the table field.
I think you can pass ctx and the specific object, e.g. table, in most
cases? There is also event and the gfp_flags. That counts 4 here, but
maybe I'm overlooking something.
Thanks.
Re: [PATCH ghak124 v3] audit: log nftables configuration change events
On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote:
> iptables, ip6tables, arptables and ebtables table registration,
> replacement and unregistration configuration events are logged for the
> native (legacy) iptables setsockopt api, but not for the
> nftables netlink api which is used by the nft-variant of iptables in
> addition to nftables itself.
>
> Add calls to log the configuration actions in the nftables netlink api.
>
> This uses the same NETFILTER_CFG record format but overloads the table
> field.
>
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0
> family=unspecified entries=2 op=nft_register_gen pid=396
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> ...
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) :
> table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> ...
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) :
> table=firewalld:1;filter_FORWARD:85 family=inet entries=8
> op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0
> comm=firewalld
> ...
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) :
> table=firewalld:1;filter_FORWARD:85 family=inet entries=101
> op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0
> comm=firewalld
> ...
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) :
> table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem
> pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> ...
> type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) :
> table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396
> subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>
> For further information please see issue
> https://github.com/linux-audit/audit-kernel/issues/124
>
> Signed-off-by: Richard Guy Briggs
> ---
> Changelog:
> v3:
> - inline message type rather than table
>
> v2:
> - differentiate between xtables and nftables
> - add set, setelem, obj, flowtable, gen
> - use nentries field as appropriate per type
> - overload the "tables" field with table handle and chain/set/flowtable
>
> include/linux/audit.h | 18
> kernel/auditsc.c | 24 --
> net/netfilter/nf_tables_api.c | 103
> ++
> 3 files changed, 142 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 3fcd9ee49734..604ede630580 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -98,6 +99,23 @@ enum audit_nfcfgop {
> AUDIT_XT_OP_REGISTER,
> AUDIT_XT_OP_REPLACE,
> AUDIT_XT_OP_UNREGISTER,
> + AUDIT_NFT_OP_TABLE_REGISTER,
> + AUDIT_NFT_OP_TABLE_UNREGISTER,
> + AUDIT_NFT_OP_CHAIN_REGISTER,
> + AUDIT_NFT_OP_CHAIN_UNREGISTER,
> + AUDIT_NFT_OP_RULE_REGISTER,
> + AUDIT_NFT_OP_RULE_UNREGISTER,
> + AUDIT_NFT_OP_SET_REGISTER,
> + AUDIT_NFT_OP_SET_UNREGISTER,
> + AUDIT_NFT_OP_SETELEM_REGISTER,
> + AUDIT_NFT_OP_SETELEM_UNREGISTER,
> + AUDIT_NFT_OP_GEN_REGISTER,
> + AUDIT_NFT_OP_OBJ_REGISTER,
> + AUDIT_NFT_OP_OBJ_UNREGISTER,
> + AUDIT_NFT_OP_OBJ_RESET,
> + AUDIT_NFT_OP_FLOWTABLE_REGISTER,
> + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> + AUDIT_NFT_OP_INVALID,
> };
>
> extern int is_audit_feature_set(int which);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 468a23390457..3a9100e95fda 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -75,6 +75,7 @@
> #include
> #include
> #include
> +#include
>
> #include "audit.h"
>
> @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab {
> };
>
> static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
> - { AUDIT_XT_OP_REGISTER, "register" },
> - { AUDIT_XT_OP_REPLACE, "replace" },
> - { AUDIT_XT_OP_UNREGISTER, "unregister"},
> + { AUDIT_XT_OP_REGISTER, "xt_register" },
> + { AUDIT_XT_OP_REPLACE, "xt_replace" },
> + { AUDIT_XT_OP_UNREGISTER, "xt_unregister"},
> + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" },
> + { AUDIT_NFT_OP_TABLE_UNREGISTER,"nft_unregister_table" },
> + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" },
> + { AUDIT_NFT_OP_CHAIN_UNREGISTER,"nft_unregister_chain" },
> + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule"},
> + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" },
> + { AUDIT_NFT_OP_SET_REGISTER,"nft_register_set" },
> + { AUDIT_NFT_OP_SET_UNREGISTER,
Re: Good idea to rename files in include/uapi/ ?
On Mon, Jun 22, 2020 at 01:37:09PM +0200, Jan Engelhardt wrote: > > On Monday 2020-06-15 01:34, Alexander A. Klimov wrote: > >> > >> A header file rename is no problem. We even have dummy headers > > Hmm.. if I understand all of you correctly, David, Stefano, Pablo and Al say > > like no, not a good idea, but only you, Jan, say like should be no problem. > > > > Jan, do you have anything like commit messages in mainline or public emails > > from maintainers confirming your opinion? > > I had already given the commit with the (email) message: > > >> Just look at xt_MARK.h, all it does is include xt_mark.h. Cf. > >> 28b949885f80efb87d7cebdcf879c99db12c37bd . Why rename this in 2020 ?
Re: Good idea to rename files in include/uapi/ ?
On Sun, Jun 14, 2020 at 11:08:08PM +0200, Jan Engelhardt wrote: > > On Sunday 2020-06-14 22:19, David Howells wrote: > >Alexander A. Klimov wrote: > > > >> *Is it a good idea to rename files in include/uapi/ ?* > > > >Very likely not. If programs out there are going to be built on a > >case-sensitive filesystem (which happens all the time), they're going to > >break > >if you rename the headers. We're kind of stuck with them. > > Netfilter has precedent for removing old headers, e.g. > 7200135bc1e61f1437dc326ae2ef2f310c50b4eb's ipt_ULOG.h. That's only because NFLOG has been there for ~10 years, so it was safe to remove ULOG support.
Re: memory leak in ctnetlink_start
On Tue, Jun 09, 2020 at 02:58:12PM -0700, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=128a9df210 > kernel config: https://syzkaller.appspot.com/x/.config?x=9a1aa05456dfd557 > dashboard link: https://syzkaller.appspot.com/bug?extid=b005af2cfb0411e617de > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17304a9610 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=168a9df210 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+b005af2cfb0411e61...@syzkaller.appspotmail.com > > executing program > executing program > executing program > BUG: memory leak > unreferenced object 0x88811c797900 (size 128): > comm "syz-executor256", pid 6458, jiffies 4294947022 (age 13.050s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > backtrace: > [<4276332a>] kmalloc include/linux/slab.h:555 [inline] > [<4276332a>] kzalloc include/linux/slab.h:669 [inline] > [<4276332a>] ctnetlink_alloc_filter+0x3a/0x2a0 > net/netfilter/nf_conntrack_netlink.c:924 > [<0047e6fb>] ctnetlink_start+0x3a/0x80 > net/netfilter/nf_conntrack_netlink.c:998 > [] __netlink_dump_start+0x1a3/0x2e0 > net/netlink/af_netlink.c:2343 > [<16b073fa>] netlink_dump_start include/linux/netlink.h:246 > [inline] > [<16b073fa>] ctnetlink_get_conntrack+0x26d/0x2f0 > net/netfilter/nf_conntrack_netlink.c:1611 > [ ] nfnetlink_rcv_msg+0x32f/0x370 > net/netfilter/nfnetlink.c:229 > [<08feca87>] netlink_rcv_skb+0x5a/0x180 > net/netlink/af_netlink.c:2469 > [<88caad78>] nfnetlink_rcv+0x83/0x1b0 > net/netfilter/nfnetlink.c:563 > [<52160488>] netlink_unicast_kernel net/netlink/af_netlink.c:1303 > [inline] > [<52160488>] netlink_unicast+0x20a/0x2f0 > net/netlink/af_netlink.c:1329 > [ ] netlink_sendmsg+0x2b5/0x560 > net/netlink/af_netlink.c:1918 > [ ] sock_sendmsg_nosec net/socket.c:652 [inline] > [ ] sock_sendmsg+0x4c/0x60 net/socket.c:672 > [ ] sys_sendmsg+0x2c4/0x2f0 net/socket.c:2352 > [<5bba2f8d>] ___sys_sendmsg+0x8a/0xd0 net/socket.c:2406 > [<60ac6563>] __sys_sendmsg+0x77/0xe0 net/socket.c:2439 > [<2f7b34b0>] do_syscall_64+0x6e/0x220 arch/x86/entry/common.c:295 > [<941009bb>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Hm, filter is not released in the error path. diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index d7bd8b1f27d5..832eabecfbdd 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -939,7 +939,8 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[], u8 family) filter->mark.mask = 0x; } } else if (cda[CTA_MARK_MASK]) { - return ERR_PTR(-EINVAL); + err = -EINVAL; + goto err_filter; } #endif if (!cda[CTA_FILTER]) @@ -947,15 +948,17 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[], u8 family) err = ctnetlink_parse_zone(cda[CTA_ZONE], >zone); if (err < 0) - return ERR_PTR(err); + goto err_filter; err = ctnetlink_parse_filter(cda[CTA_FILTER], filter); if (err < 0) - return ERR_PTR(err); + goto err_filter; if (filter->orig_flags) { - if (!cda[CTA_TUPLE_ORIG]) - return ERR_PTR(-EINVAL); + if (!cda[CTA_TUPLE_ORIG]) { + err = -EINVAL; + goto err_filter; + } err = ctnetlink_parse_tuple_filter(cda, >orig, CTA_TUPLE_ORIG, @@ -963,23 +966,32 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[], u8 family) >zone, filter->orig_flags); if (err < 0) - return ERR_PTR(err); + goto err_filter; } if (filter->reply_flags) { - if (!cda[CTA_TUPLE_REPLY]) - return ERR_PTR(-EINVAL); + if (!cda[CTA_TUPLE_REPLY]) { + err = -EINVAL; + goto err_filter; + } err = ctnetlink_parse_tuple_filter(cda, >reply, CTA_TUPLE_REPLY, filter->family, >zone, filter->orig_flags); - if (err < 0) - return ERR_PTR(err); + if (err < 0) { + err = -EINVAL; + goto err_filter; + } } return filter; + +err_filter: + kfree(filter); + + return ERR_PTR(err); } static bool ctnetlink_needs_filter(u8 family, const struct nlattr * const *cda) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 073aa1051d43..5792e9dcd9bc 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -6550,12 +6550,22 @@ static int nf_tables_newflowtable(struct
Re: memory leak in nf_tables_parse_netdev_hooks (3)
On Tue, Jun 09, 2020 at 02:58:12PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1741f5f210
> kernel config: https://syzkaller.appspot.com/x/.config?x=9a1aa05456dfd557
> dashboard link: https://syzkaller.appspot.com/bug?extid=eb9d5924c51d6d59e094
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126f544610
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177f16fe10
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+eb9d5924c51d6d59e...@syzkaller.appspotmail.com
Attaching a quick patch, I will have a closer look later.
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 073aa1051d43..5792e9dcd9bc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6550,12 +6550,22 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
return err;
}
+static void nft_flowtable_hook_release(struct nft_flowtable_hook *flowtable_hook)
+{
+ struct nft_hook *this, *next;
+
+ list_for_each_entry_safe(this, next, _hook->list, list) {
+ list_del(>list);
+ kfree(this);
+ }
+}
+
static int nft_delflowtable_hook(struct nft_ctx *ctx,
struct nft_flowtable *flowtable)
{
const struct nlattr * const *nla = ctx->nla;
struct nft_flowtable_hook flowtable_hook;
- struct nft_hook *this, *next, *hook;
+ struct nft_hook *this, *hook;
struct nft_trans *trans;
int err;
@@ -6564,33 +6574,40 @@ static int nft_delflowtable_hook(struct nft_ctx *ctx,
if (err < 0)
return err;
- list_for_each_entry_safe(this, next, _hook.list, list) {
+ list_for_each_entry(this, _hook.list, list) {
hook = nft_hook_list_find(>hook_list, this);
if (!hook) {
err = -ENOENT;
goto err_flowtable_del_hook;
}
hook->inactive = true;
- list_del(>list);
- kfree(this);
}
trans = nft_trans_alloc(ctx, NFT_MSG_DELFLOWTABLE,
sizeof(struct nft_trans_flowtable));
- if (!trans)
- return -ENOMEM;
+ if (!trans) {
+ err = -ENOMEM;
+ goto err_flowtable_del_hook;
+ }
nft_trans_flowtable(trans) = flowtable;
nft_trans_flowtable_update(trans) = true;
INIT_LIST_HEAD(_trans_flowtable_hooks(trans));
+ nft_flowtable_hook_release(_hook);
list_add_tail(>list, >net->nft.commit_list);
return 0;
err_flowtable_del_hook:
- list_for_each_entry(hook, _hook.list, list)
+ list_for_each_entry(this, >hook_list, list) {
+ hook = nft_hook_list_find(>hook_list, this);
+ if (!hook)
+ break;
+
hook->inactive = false;
+ }
+ nft_flowtable_hook_release(_hook);
return err;
}
Re: [PATCH net] netfilter: conntrack: Pass value of ctinfo to __nf_conntrack_update
On Wed, May 27, 2020 at 01:10:39AM -0700, Nathan Chancellor wrote: > Clang warns: > > net/netfilter/nf_conntrack_core.c:2068:21: warning: variable 'ctinfo' is > uninitialized when used here [-Wuninitialized] > nf_ct_set(skb, ct, ctinfo); >^~ > net/netfilter/nf_conntrack_core.c:2024:2: note: variable 'ctinfo' is > declared here > enum ip_conntrack_info ctinfo; > ^ > 1 warning generated. > > nf_conntrack_update was split up into nf_conntrack_update and > __nf_conntrack_update, where the assignment of ctifno is in > nf_conntrack_update but it is used in __nf_conntrack_update. > > Pass the value of ctinfo from nf_conntrack_update to > __nf_conntrack_update so that uninitialized memory is not used > and everything works properly. Applied, thanks.
Re: [PATCH 06/15] netfilter: conntrack: avoid gcc-10 zero-length-bounds warning
On Thu, Apr 30, 2020 at 11:30:48PM +0200, Arnd Bergmann wrote:
> gcc-10 warns around a suspicious access to an empty struct member:
>
> net/netfilter/nf_conntrack_core.c: In function '__nf_conntrack_alloc':
> net/netfilter/nf_conntrack_core.c:1522:9: warning: array subscript 0 is
> outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned
> char[0]'} [-Wzero-length-bounds]
> 1522 | memset(>__nfct_init_offset[0], 0,
> | ^~
> In file included from net/netfilter/nf_conntrack_core.c:37:
> include/net/netfilter/nf_conntrack.h:90:5: note: while referencing
> '__nfct_init_offset'
>90 | u8 __nfct_init_offset[0];
> | ^~
>
> The code is correct but a bit unusual. Rework it slightly in a way that
> does not trigger the warning, using an empty struct instead of an empty
> array. There are probably more elegant ways to do this, but this is the
> smallest change.
Applied, thanks.
Re: [PATCH] netfilter: nf_osf: avoid passing pointer to local var
On Wed, Apr 29, 2020 at 09:00:41PM +0200, Arnd Bergmann wrote: > gcc-10 points out that a code path exists where a pointer to a stack > variable may be passed back to the caller: > > net/netfilter/nfnetlink_osf.c: In function 'nf_osf_hdr_ctx_init': > cc1: warning: function may return address of local variable > [-Wreturn-local-addr] > net/netfilter/nfnetlink_osf.c:171:16: note: declared here > 171 | struct tcphdr _tcph; > |^ > > I am not sure whether this can happen in practice, but moving the > variable declaration into the callers avoids the problem. Applied, thanks.
Re: [PATCH tip/core/rcu 8/9] net/netfilter: Replace rcu_swap_protected() with rcu_replace()
On Wed, Oct 02, 2019 at 06:43:09PM -0700, paul...@kernel.org wrote: > From: "Paul E. McKenney" > > This commit replaces the use of rcu_swap_protected() with the more > intuitively appealing rcu_replace() as a step towards removing > rcu_swap_protected(). > > Link: > https://lore.kernel.org/lkml/CAHk-=wiAsJLw1egFEE=z7-ggtm6wcvtyytxza1+bhqta4gg...@mail.gmail.com/ > Reported-by: Linus Torvalds > Signed-off-by: Paul E. McKenney > Cc: Pablo Neira Ayuso > Cc: Jozsef Kadlecsik > Cc: Florian Westphal > Cc: "David S. Miller" > Cc: > Cc: > Cc: Acked-by: Pablo Neira Ayuso > --- > net/netfilter/nf_tables_api.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index d481f9b..8499baf 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -1461,8 +1461,9 @@ static void nft_chain_stats_replace(struct nft_trans > *trans) > if (!nft_trans_chain_stats(trans)) > return; > > - rcu_swap_protected(chain->stats, nft_trans_chain_stats(trans), > -lockdep_commit_lock_is_held(trans->ctx.net)); > + nft_trans_chain_stats(trans) = > + rcu_replace(chain->stats, nft_trans_chain_stats(trans), > + lockdep_commit_lock_is_held(trans->ctx.net)); > > if (!nft_trans_chain_stats(trans)) > static_branch_inc(_counters_enabled); > -- > 2.9.5 >
Re: [PATCH v2] netfilter: use __u8 instead of uint8_t in uapi header
On Tue, Sep 24, 2019 at 07:40:06AM +0900, Masahiro Yamada wrote:
> When CONFIG_UAPI_HEADER_TEST=y, exported headers are compile-tested to
> make sure they can be included from user-space.
>
> Currently, linux/netfilter_bridge/ebtables.h is excluded from the test
> coverage. To make it join the compile-test, we need to fix the build
> errors attached below.
>
> For a case like this, we decided to use __u{8,16,32,64} variable types
> in this discussion:
>
> https://lkml.org/lkml/2019/6/5/18
>
> Build log:
>
> CC usr/include/linux/netfilter_bridge/ebtables.h.s
> In file included from :32:0:
> ./usr/include/linux/netfilter_bridge/ebtables.h:126:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
> ./usr/include/linux/netfilter_bridge/ebtables.h:139:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
> ./usr/include/linux/netfilter_bridge/ebtables.h:152:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
Applied.
Re: [PATCH] netfilter: use __u8 instead of uint8_t in uapi header
On Sun, Sep 22, 2019 at 08:49:11PM +0900, Masahiro Yamada wrote:
> Hi Pablo,
>
> On Sun, Sep 22, 2019 at 4:13 PM Pablo Neira Ayuso wrote:
> >
> > On Sun, Sep 22, 2019 at 09:11:11AM +0200, Pablo Neira Ayuso wrote:
> > > On Sat, Sep 21, 2019 at 10:46:48PM +0900, Masahiro Yamada wrote:
> > > > When CONFIG_UAPI_HEADER_TEST=y, exported headers are compile-tested to
> > > > make sure they can be included from user-space.
> > > >
> > > > Currently, linux/netfilter_bridge/ebtables.h is excluded from the test
> > > > coverage. To make it join the compile-test, we need to fix the build
> > > > errors attached below.
> > > >
> > > > For a case like this, we decided to use __u{8,16,32,64} variable types
> > > > in this discussion:
> > > >
> > > > https://lkml.org/lkml/2019/6/5/18
> > > >
> > > > Build log:
> > > >
> > > > CC usr/include/linux/netfilter_bridge/ebtables.h.s
> > > > In file included from :32:0:
> > > > ./usr/include/linux/netfilter_bridge/ebtables.h:126:4: error: unknown
> > > > type name ‘uint8_t’
> > > > uint8_t revision;
> > > > ^~~
> > > > ./usr/include/linux/netfilter_bridge/ebtables.h:139:4: error: unknown
> > > > type name ‘uint8_t’
> > > > uint8_t revision;
> > > > ^~~
> > > > ./usr/include/linux/netfilter_bridge/ebtables.h:152:4: error: unknown
> > > > type name ‘uint8_t’
> > > > uint8_t revision;
> > > > ^~~
> > >
> > > Applied.
> >
> > Patch does not apply cleanly to nf.git, I have to keep it back, sorry
>
> Perhaps, reducing the context (git am -C) might help.
Not working for me.
> Shall I rebase and resend it?
Please do. Thanks.
Re: [PATCH] netfilter: use __u8 instead of uint8_t in uapi header
On Sun, Sep 22, 2019 at 09:11:11AM +0200, Pablo Neira Ayuso wrote:
> On Sat, Sep 21, 2019 at 10:46:48PM +0900, Masahiro Yamada wrote:
> > When CONFIG_UAPI_HEADER_TEST=y, exported headers are compile-tested to
> > make sure they can be included from user-space.
> >
> > Currently, linux/netfilter_bridge/ebtables.h is excluded from the test
> > coverage. To make it join the compile-test, we need to fix the build
> > errors attached below.
> >
> > For a case like this, we decided to use __u{8,16,32,64} variable types
> > in this discussion:
> >
> > https://lkml.org/lkml/2019/6/5/18
> >
> > Build log:
> >
> > CC usr/include/linux/netfilter_bridge/ebtables.h.s
> > In file included from :32:0:
> > ./usr/include/linux/netfilter_bridge/ebtables.h:126:4: error: unknown type
> > name ‘uint8_t’
> > uint8_t revision;
> > ^~~
> > ./usr/include/linux/netfilter_bridge/ebtables.h:139:4: error: unknown type
> > name ‘uint8_t’
> > uint8_t revision;
> > ^~~
> > ./usr/include/linux/netfilter_bridge/ebtables.h:152:4: error: unknown type
> > name ‘uint8_t’
> > uint8_t revision;
> > ^~~
>
> Applied.
Patch does not apply cleanly to nf.git, I have to keep it back, sorry
$ git am /tmp/yamada.masahiro.txt -s
Applying: netfilter: use __u8 instead of uint8_t in uapi header
error: patch failed: usr/include/Makefile:37
error: usr/include/Makefile: patch does not apply
Patch failed at 0001 netfilter: use __u8 instead of uint8_t in uapi header
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Re: [PATCH] netfilter: use __u8 instead of uint8_t in uapi header
On Sat, Sep 21, 2019 at 10:46:48PM +0900, Masahiro Yamada wrote:
> When CONFIG_UAPI_HEADER_TEST=y, exported headers are compile-tested to
> make sure they can be included from user-space.
>
> Currently, linux/netfilter_bridge/ebtables.h is excluded from the test
> coverage. To make it join the compile-test, we need to fix the build
> errors attached below.
>
> For a case like this, we decided to use __u{8,16,32,64} variable types
> in this discussion:
>
> https://lkml.org/lkml/2019/6/5/18
>
> Build log:
>
> CC usr/include/linux/netfilter_bridge/ebtables.h.s
> In file included from :32:0:
> ./usr/include/linux/netfilter_bridge/ebtables.h:126:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
> ./usr/include/linux/netfilter_bridge/ebtables.h:139:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
> ./usr/include/linux/netfilter_bridge/ebtables.h:152:4: error: unknown type
> name ‘uint8_t’
> uint8_t revision;
> ^~~
Applied.
Re: [PATCH net-next] netfilter: nf_tables: avoid excessive stack usage
On Sat, Sep 07, 2019 at 08:41:22PM +0200, Arnd Bergmann wrote:
> On Sat, Sep 7, 2019 at 8:07 PM Pablo Neira Ayuso wrote:
> >
> > Hi Arnd,
> >
> > On Fri, Sep 06, 2019 at 05:12:30PM +0200, Arnd Bergmann wrote:
> > > The nft_offload_ctx structure is much too large to put on the
> > > stack:
> > >
> > > net/netfilter/nf_tables_offload.c:31:23: error: stack frame size of 1200
> > > bytes in function 'nft_flow_rule_create' [-Werror,-Wframe-larger-than=]
> > >
> > > Use dynamic allocation here, as we do elsewhere in the same
> > > function.
> > >
> > > Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
> > > Signed-off-by: Arnd Bergmann
> > > ---
> > > Since we only really care about two members of the structure, an
> > > alternative would be a larger rewrite, but that is probably too
> > > late for v5.4.
> >
> > Thanks for this patch.
> >
> > I'm attaching a patch to reduce this structure size a bit. Do you
> > think this alternative patch is ok until this alternative rewrite
> > happens?
>
> I haven't tried it yet, but it looks like that would save 8 of the
> 48 bytes in each for each of the 24 registers (12 bytes on m68k
> or i386, which only use 4 byte alignment for nft_data), so
> this wouldn't make too much difference.
I'll take your patch as is.
> > Anyway I agree we should to get this structure away from the
> > stack, even after this is still large, so your patch (or a variant of
> > it) will be useful sooner than later I think.
>
> What I was thinking for a possible smaller fix would be to not
> pass the ctx into the expr->ops->offload callback but
> only pass the 'dep' member. Since I've never seen this code
> before, I have no idea if that would be an improvement
> in the end.
We might need this more fields of this context structure, this code is
very new, still under development, let's revisit this later.
Thanks.
Re: [PATCH net-next] netfilter: nf_tables: avoid excessive stack usage
Hi Arnd,
On Fri, Sep 06, 2019 at 05:12:30PM +0200, Arnd Bergmann wrote:
> The nft_offload_ctx structure is much too large to put on the
> stack:
>
> net/netfilter/nf_tables_offload.c:31:23: error: stack frame size of 1200
> bytes in function 'nft_flow_rule_create' [-Werror,-Wframe-larger-than=]
>
> Use dynamic allocation here, as we do elsewhere in the same
> function.
>
> Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
> Signed-off-by: Arnd Bergmann
> ---
> Since we only really care about two members of the structure, an
> alternative would be a larger rewrite, but that is probably too
> late for v5.4.
Thanks for this patch.
I'm attaching a patch to reduce this structure size a bit. Do you
think this alternative patch is ok until this alternative rewrite
happens? Anyway I agree we should to get this structure away from the
stack, even after this is still large, so your patch (or a variant of
it) will be useful sooner than later I think.
diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
index db104665a9e4..cc44d29e9fd7 100644
--- a/include/net/netfilter/nf_tables_offload.h
+++ b/include/net/netfilter/nf_tables_offload.h
@@ -5,10 +5,10 @@
#include
struct nft_offload_reg {
- u32 key;
- u32 len;
- u32 base_offset;
- u32 offset;
+ u8 key;
+ u8 len;
+ u8 base_offset;
+ u8 offset;
struct nft_data data;
struct nft_data mask;
};
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Tue, Aug 27, 2019 at 02:34:14PM -0300, Leonardo Bras wrote: > On Tue, 2019-08-27 at 12:35 +0200, Pablo Neira Ayuso wrote: [...] > > NFT_BREAK instead to stop evaluating this rule, this results in a > > mismatch, so you let the user decide what to do with packets that do > > not match your policy. > > Ok, I will replace for v3. Thanks. > > The drop case at the bottom of the fib eval function never actually > > never happens. > > Which one do you mean? Line 31 of net/netfilter/nft_fib_inet.c.
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Wed, Aug 21, 2019 at 11:15:06AM -0300, Leonardo Bras wrote:
> If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> dealing with a IPv6 package, it causes a kernel panic in
> fib6_node_lookup_1(), crashing in bad_page_fault.
Q: How do you get to see IPv6 packets if IPv6 module is disable?
> The panic is caused by trying to deference a very low address (0x38
> in ppc64le), due to ipv6.fib6_main_tbl = NULL.
> BUG: Kernel NULL pointer dereference at 0x0038
>
> Fix this behavior by dropping IPv6 packages if !ipv6_mod_enabled().
I'd suggest: s/package/packet/
[...]
> diff --git a/net/ipv6/netfilter/nft_fib_ipv6.c
> b/net/ipv6/netfilter/nft_fib_ipv6.c
> index 7ece86afd079..75acc417e2ff 100644
> --- a/net/ipv6/netfilter/nft_fib_ipv6.c
> +++ b/net/ipv6/netfilter/nft_fib_ipv6.c
> @@ -125,6 +125,11 @@ void nft_fib6_eval_type(const struct nft_expr *expr,
> struct nft_regs *regs,
> u32 *dest = >data[priv->dreg];
> struct ipv6hdr *iph, _iph;
>
> + if (!ipv6_mod_enabled()) {
> + regs->verdict.code = NF_DROP;
NFT_BREAK instead to stop evaluating this rule, this results in a
mismatch, so you let the user decide what to do with packets that do
not match your policy.
The drop case at the bottom of the fib eval function never actually
never happens.
Re: [PATCH 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Tue, Aug 20, 2019 at 01:15:58PM -0300, Leonardo Bras wrote: > On Tue, 2019-08-20 at 07:36 +0200, Florian Westphal wrote: > > Wouldn't fib_netdev.c have the same problem? > Probably, but I haven't hit this issue yet. > > > If so, might be better to place this test in both > > nft_fib6_eval_type and nft_fib6_eval. > > I think that is possible, and not very hard to do. > > But in my humble viewpoint, it looks like it's nft_fib_inet_eval() and > nft_fib_netdev_eval() have the responsibility to choose a valid > protocol or drop the package. > I am not sure if it would be a good move to transfer this > responsibility to nft_fib6_eval_type() and nft_fib6_eval(), so I would > rather add the same test to nft_fib_netdev_eval(). > > Does it make sense? Please, update common code to netdev and ip6 extensions as Florian suggests. Thanks.
Re: [PATCH] netfilter: add include guard to nf_conntrack_h323_types.h
On Mon, Aug 19, 2019 at 04:39:27PM +0900, Masahiro Yamada wrote: > Add a header include guard just in case. Applied.
Re: [PATCH] MAINTAINERS: Remove IP MASQUERADING record
On Wed, Aug 14, 2019 at 03:35:02PM +0300, Denis Efremov wrote:
> This entry is in MAINTAINERS for historical purpose.
> It doesn't match current sources since the commit
> adf82accc5f5 ("netfilter: x_tables: merge ip and
> ipv6 masquerade modules") moved the module.
> The net/netfilter/xt_MASQUERADE.c module is already under
> the netfilter section. Thus, there is no purpose to keep this
> separate entry in MAINTAINERS.
Applied, thanks.
Re: [PATCH] netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument
On Wed, Aug 14, 2019 at 09:58:09AM -0700, Nathan Chancellor wrote: > clang warns: > > net/netfilter/nft_bitwise.c:138:50: error: size argument in 'memcmp' > call is a comparison [-Werror,-Wmemsize-comparison] > if (memcmp(>xor, , sizeof(priv->xor) || > ~~^~ Applied, thanks.
Re: [PATCH v2] net/netfilter: remove unnecessary spaces
Applied, thanks.
Re: [PATCH] net/netfilter/nf_nat_proto.c - make tables static
On Thu, Aug 08, 2019 at 01:43:22AM -0400, Valdis Klētnieks wrote: > Sparse warns about two tables not being declared. > > CHECK net/netfilter/nf_nat_proto.c > net/netfilter/nf_nat_proto.c:725:26: warning: symbol 'nf_nat_ipv4_ops' was > not declared. Should it be static? > net/netfilter/nf_nat_proto.c:964:26: warning: symbol 'nf_nat_ipv6_ops' was > not declared. Should it be static? > > And in fact they can indeed be static. Applied, thanks.
Re: [PATCH] net/netfilter - add missing prototypes.
On Thu, Aug 08, 2019 at 01:28:08AM -0400, Valdis Klētnieks wrote: > Sparse rightly complains about undeclared symbols. > > CHECK net/netfilter/nft_set_hash.c > net/netfilter/nft_set_hash.c:647:21: warning: symbol 'nft_set_rhash_type' was > not declared. Should it be static? > net/netfilter/nft_set_hash.c:670:21: warning: symbol 'nft_set_hash_type' was > not declared. Should it be static? > net/netfilter/nft_set_hash.c:690:21: warning: symbol 'nft_set_hash_fast_type' > was not declared. Should it be static? > CHECK net/netfilter/nft_set_bitmap.c > net/netfilter/nft_set_bitmap.c:296:21: warning: symbol 'nft_set_bitmap_type' > was not declared. Should it be static? > CHECK net/netfilter/nft_set_rbtree.c > net/netfilter/nft_set_rbtree.c:470:21: warning: symbol 'nft_set_rbtree_type' > was not declared. Should it be static? > > Include nf_tables_core.h rather than nf_tables.h to pick up the additional > definitions. Applied, thanks.
Re: [PATCH] MAINTAINERS: ip masquerading: Update path to the driver
On Tue, Aug 13, 2019 at 09:09:41AM +0300, Denis Efremov wrote: > Update MAINTAINERS record to reflect the filename change > from ipt_MASQUERADE.c to xt_MASQUERADE.c This entry is there for historical purpose. I'd suggest you send a patch to remove it so this just falls under the netfilter section. Thanks.
Re: [PATCH net-next] netfilter: conntrack: use shared sysctl constants
On Tue, Jul 23, 2019 at 03:23:03AM +0200, Matteo Croce wrote:
> Use shared sysctl variables for zero and one constants, as in commit
> eec4844fae7c ("proc/sysctl: add shared variables for range check")
Applied, thanks.
Re: [PATCH] netfilter: add include guard to xt_connlabel.h
On Mon, Jul 29, 2019 at 12:51:38AM +0900, Masahiro Yamada wrote: > Add a header include guard just in case. Applied to nf.git, thanks. BTW, is the _UAPI_ prefix really needed? I can see netfilter headers under include/uapi/ sometimes are prefixed by UAPI and sometimes not. Thanks.
Re: linux-next: Signed-off-by missing for commit in the netfilter tree
On Thu, Jul 25, 2019 at 07:18:03AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Commit
>
> 5f5ff5ca2e18 ("netfilter: nf_tables: Make nft_meta expression more robust")
>
> is missing a Signed-off-by from its author.
Fixed, thanks.
Re: [PATCH v2] net/netfilter: remove unnecessary spaces
Looks good, but you will have to wait until net-next reopens:
http://vger.kernel.org/~davem/net-next.html
Will keep this in my patchwork until that happens.
Thanks.
On Tue, Jul 16, 2019 at 10:13:01AM +0800, yangxingwu wrote:
> this patch removes extra spaces
>
> Signed-off-by: yangxingwu
> ---
> net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
> net/netfilter/ipset/ip_set_list_set.c | 2 +-
> net/netfilter/ipvs/ip_vs_core.c| 2 +-
> net/netfilter/ipvs/ip_vs_mh.c | 4 ++--
> net/netfilter/ipvs/ip_vs_proto_tcp.c | 2 +-
> net/netfilter/nf_conntrack_ftp.c | 2 +-
> net/netfilter/nf_conntrack_proto_tcp.c | 2 +-
> net/netfilter/nfnetlink_log.c | 4 ++--
> net/netfilter/nfnetlink_queue.c| 4 ++--
> net/netfilter/xt_IDLETIMER.c | 2 +-
> 10 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h
> b/net/netfilter/ipset/ip_set_hash_gen.h
> index 10f6196..eb907d2 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -954,7 +954,7 @@ struct htype {
> mtype_data_netmask(d, NCIDR_GET(h->nets[j].cidr[0]));
> #endif
> key = HKEY(d, h->initval, t->htable_bits);
> - n = rcu_dereference_bh(hbucket(t, key));
> + n = rcu_dereference_bh(hbucket(t, key));
> if (!n)
> continue;
> for (i = 0; i < n->pos; i++) {
> diff --git a/net/netfilter/ipset/ip_set_list_set.c
> b/net/netfilter/ipset/ip_set_list_set.c
> index 8ada318..5c2be76 100644
> --- a/net/netfilter/ipset/ip_set_list_set.c
> +++ b/net/netfilter/ipset/ip_set_list_set.c
> @@ -289,7 +289,7 @@ struct list_set {
> if (n &&
> !(SET_WITH_TIMEOUT(set) &&
> ip_set_timeout_expired(ext_timeout(n, set
> - n = NULL;
> + n = NULL;
>
> e = kzalloc(set->dsize, GFP_ATOMIC);
> if (!e)
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 7138556..6b3ae76 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -615,7 +615,7 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff
> *skb,
> unsigned int flags = (svc->flags & IP_VS_SVC_F_ONEPACKET &&
> iph->protocol == IPPROTO_UDP) ?
> IP_VS_CONN_F_ONE_PACKET : 0;
> - union nf_inet_addr daddr = { .all = { 0, 0, 0, 0 } };
> + union nf_inet_addr daddr = { .all = { 0, 0, 0, 0 } };
>
> /* create a new connection entry */
> IP_VS_DBG(6, "%s(): create a cache_bypass entry\n", __func__);
> diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
> index 94d9d34..da0280c 100644
> --- a/net/netfilter/ipvs/ip_vs_mh.c
> +++ b/net/netfilter/ipvs/ip_vs_mh.c
> @@ -174,8 +174,8 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
> return 0;
> }
>
> - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> - sizeof(unsigned long), GFP_KERNEL);
> + table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> + sizeof(unsigned long), GFP_KERNEL);
> if (!table)
> return -ENOMEM;
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> index 915ac82..c7b46a9 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> @@ -710,7 +710,7 @@ static int __ip_vs_tcp_init(struct netns_ipvs *ipvs,
> struct ip_vs_proto_data *pd
> sizeof(tcp_timeouts));
> if (!pd->timeout_table)
> return -ENOMEM;
> - pd->tcp_state_table = tcp_states;
> + pd->tcp_state_table = tcp_states;
> return 0;
> }
>
> diff --git a/net/netfilter/nf_conntrack_ftp.c
> b/net/netfilter/nf_conntrack_ftp.c
> index 8c6c11b..26c1ff8 100644
> --- a/net/netfilter/nf_conntrack_ftp.c
> +++ b/net/netfilter/nf_conntrack_ftp.c
> @@ -162,7 +162,7 @@ static int try_rfc959(const char *data, size_t dlen,
> if (length == 0)
> return 0;
>
> - cmd->u3.ip = htonl((array[0] << 24) | (array[1] << 16) |
> + cmd->u3.ip = htonl((array[0] << 24) | (array[1] << 16) |
> (array[2] << 8) | array[3]);
> cmd->u.tcp.port = htons((array[4] << 8) | array[5]);
> return length;
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index 1e2cc83..48f3a67 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -1225,7 +1225,7 @@ static int tcp_to_nlattr(struct sk_buff *skb, struct
> nlattr *nla,
> [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = { .type = NLA_U8 },
> [CTA_PROTOINFO_TCP_WSCALE_REPLY]= { .type = NLA_U8 },
>
Re: [PATCH v5] net: netfilter: Fix rpfilter dropping vrf packets by mistake
On Tue, Jul 02, 2019 at 03:59:36AM +, Miaohe Lin wrote: > When firewalld is enabled with ipv4/ipv6 rpfilter, vrf > ipv4/ipv6 packets will be dropped. Vrf device will pass > through netfilter hook twice. One with enslaved device > and another one with l3 master device. So in device may > dismatch witch out device because out device is always > enslaved device.So failed with the check of the rpfilter > and drop the packets by mistake. Applied to nf.git, thanks.
Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)
On Tue, Jul 16, 2019 at 02:56:09AM +0900, Masahiro Yamada wrote: > On Tue, Jul 16, 2019 at 2:33 AM Pablo Neira Ayuso wrote: > > > > On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote: > > > CC'ing netfilter. > > > > > > On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap > > > wrote: > > > > > > > > On 7/14/19 9:48 PM, Stephen Rothwell wrote: > > > > > Hi all, > > > > > > > > > > Please do not add v5.4 material to your linux-next included branches > > > > > until after v5.3-rc1 has been released. > > > > > > > > > > Changes since 20190712: > > > > > > > > > > > > > Hi, > > > > > > > > I am seeing these build errors from HEADERS_TEST (or > > > > KERNEL_HEADERS_TEST) > > > > for include/net/netfilter/nf_tables_offload.h.s: > > > > > > > > CC include/net/netfilter/nf_tables_offload.h.s > > [...] > > > > Should this header file not be tested? > > This means you must endlessly exclude > headers that include nf_tables.h > > > > Yes, it should indeed be added. > > Adding 'header-test-' is the last resort. OK, so policy now is that all internal headers should compile standalone, right? I can have a look and make it compile standalone.
Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)
On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote: > CC'ing netfilter. > > On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap wrote: > > > > On 7/14/19 9:48 PM, Stephen Rothwell wrote: > > > Hi all, > > > > > > Please do not add v5.4 material to your linux-next included branches > > > until after v5.3-rc1 has been released. > > > > > > Changes since 20190712: > > > > > > > Hi, > > > > I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST) > > for include/net/netfilter/nf_tables_offload.h.s: > > > > CC include/net/netfilter/nf_tables_offload.h.s [...] > > Should this header file not be tested? Yes, it should indeed be added.
Re: [PATCH] ipvs: remove unnecessary space
On Wed, Jul 10, 2019 at 10:06:09AM +0200, Simon Horman wrote: > On Wed, Jul 10, 2019 at 03:45:52PM +0800, yangxingwu wrote: > > --- > > net/netfilter/ipvs/ip_vs_mh.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c > > index 94d9d34..98e358e 100644 > > --- a/net/netfilter/ipvs/ip_vs_mh.c > > +++ b/net/netfilter/ipvs/ip_vs_mh.c > > @@ -174,8 +174,8 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s, > > return 0; > > } > > > > - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE), > > -sizeof(unsigned long), GFP_KERNEL); > > + table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE), > > + sizeof(unsigned long), GFP_KERNEL); May I ask one thing? :-) Please, remove all unnecessary spaces in one go, search for: git grep "= " in the netfilter tree, and send a v2 for this one. Thanks.
Re: [PATCH] ipvs: remove unnecessary space
On Mon, Jul 15, 2019 at 09:57:03AM +0200, Pablo Neira Ayuso wrote: > On Fri, Jul 12, 2019 at 09:07:21PM +0800, yangxingwu wrote: > > this patch removes the extra space and use bitmap_zalloc instead > > > > Signed-off-by: yangxingwu > > --- > > net/netfilter/ipvs/ip_vs_mh.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c > > index 94d9d34..3229867 100644 > > --- a/net/netfilter/ipvs/ip_vs_mh.c > > +++ b/net/netfilter/ipvs/ip_vs_mh.c > > @@ -174,8 +174,7 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s, > > return 0; > > } > > > > - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE), > > -sizeof(unsigned long), GFP_KERNEL); > > + table = bitmap_zalloc(IP_VS_MH_TAB_SIZE, GFP_KERNEL); > > By doing: > > git grep "= " ... > > on the netfilter folders, I see more of these, it would be good if you > fix them at once or, probably, you want to use coccinelle for this. If patch subject is "remove unnecessary space" then just remove unnecessary spaces in the patch, otherwise I suggest you rename this to "ipvs: use bitmap_zalloc()" or such, since the space removal here is irrelevant.
Re: [PATCH] ipvs: remove unnecessary space
On Fri, Jul 12, 2019 at 09:07:21PM +0800, yangxingwu wrote: > this patch removes the extra space and use bitmap_zalloc instead > > Signed-off-by: yangxingwu > --- > net/netfilter/ipvs/ip_vs_mh.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c > index 94d9d34..3229867 100644 > --- a/net/netfilter/ipvs/ip_vs_mh.c > +++ b/net/netfilter/ipvs/ip_vs_mh.c > @@ -174,8 +174,7 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s, > return 0; > } > > - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE), > - sizeof(unsigned long), GFP_KERNEL); > + table = bitmap_zalloc(IP_VS_MH_TAB_SIZE, GFP_KERNEL); By doing: git grep "= " ... on the netfilter folders, I see more of these, it would be good if you fix them at once or, probably, you want to use coccinelle for this. Thanks.
Re: linux-next: Tree for Jul 3 (netfilter/ipvs/)
On Wed, Jul 03, 2019 at 09:29:26PM +0300, Julian Anastasov wrote: > > Hello, > > On Wed, 3 Jul 2019, Randy Dunlap wrote: > > > On 7/3/19 4:49 AM, Stephen Rothwell wrote: > > > Hi all, > > > > > > Changes since 20190702: > > > > > > > on i386: > > Oh, well. net/gre.h was included by CONFIG_NF_CONNTRACK, so > it is failing when CONFIG_NF_CONNTRACK is not used. > > Pablo, should I post v2 or just a fix? I let you choose.
Re: [PATCH] netfilter: nf_log: Replace a seq_printf() call by seq_puts() in seq_show()
On Tue, Jul 02, 2019 at 08:11:53PM +0200, Markus Elfring wrote: > From: Markus Elfring > Date: Tue, 2 Jul 2019 20:06:30 +0200 > > A string which did not contain a data format specification should be put > into a sequence. Thus use the corresponding function “seq_puts”. > > This issue was detected by using the Coccinelle software. Applied, thanks.
