* [PATCH 0/3] Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Leon Romanovsky, Saeed Mahameed, David S . Miller, Doug Ledford,
Jason Gunthorpe, linux-rdma, netdev, linux-kernel, Chuhong Yuan
Reference counters are preferred to use refcount_t instead of
atomic_t.
This is because the implementation of refcount_t can prevent
overflows and detect possible use-after-free.
First convert the refcount field to refcount_t in mlx5/driver.h.
Then convert the uses to refcount_() APIs.
Chuhong Yuan (3):
mlx5: Use refcount_t for refcount
net/mlx5: Use refcount_() APIs
IB/mlx5: Use refcount_() APIs
drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
include/linux/mlx5/driver.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 1/3] mlx5: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Leon Romanovsky, Saeed Mahameed, linux-rdma, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
include/linux/mlx5/driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 0e6da1840c7d..ec8fb382d426 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -398,7 +398,7 @@ enum mlx5_res_type {
struct mlx5_core_rsc_common {
enum mlx5_res_type res;
- atomic_t refcount;
+ refcount_t refcount;
struct completion free;
};
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] net/mlx5: Use refcount_() APIs
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma, linux-kernel, Chuhong Yuan
This patch depends on PATCH 1/3.
After converting refcount to refcount_t, use
refcount_() APIs to operate it.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
index b8ba74de9555..7b44d1e49604 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
@@ -53,7 +53,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn)
common = radix_tree_lookup(&table->tree, rsn);
if (common)
- atomic_inc(&common->refcount);
+ refcount_inc(&common->refcount);
spin_unlock_irqrestore(&table->lock, flags);
@@ -62,7 +62,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn)
void mlx5_core_put_rsc(struct mlx5_core_rsc_common *common)
{
- if (atomic_dec_and_test(&common->refcount))
+ if (refcount_dec_and_test(&common->refcount))
complete(&common->free);
}
@@ -209,7 +209,7 @@ static int create_resource_common(struct mlx5_core_dev *dev,
if (err)
return err;
- atomic_set(&qp->common.refcount, 1);
+ refcount_set(&qp->common.refcount, 1);
init_completion(&qp->common.free);
qp->pid = current->pid;
--
2.20.1
^ permalink raw reply related
* Re: memory leak in tipc_group_create_member
From: Ying Xue @ 2019-08-02 12:18 UTC (permalink / raw)
To: Hillf Danton, syzbot
Cc: davem, jon.maloy, linux-kernel, netdev, syzkaller-bugs,
tipc-discussion
In-Reply-To: <000000000000879057058f193fb5@google.com>
On 8/2/19 3:44 PM, Hillf Danton wrote:
>
> On Thu, 01 Aug 2019 19:38:06 -0700
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: a9815a4f Merge branch 'x86-urgent-for-linus' of git://git...
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=12a6dbf0600000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=37c48fb52e3789e6
>> dashboard link: https://syzkaller.appspot.com/bug?extid=f95d90c454864b3b5bc9
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13be3ecc600000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11c992b4600000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+f95d90c454864b3b5bc9@syzkaller.appspotmail.com
>>
>> executing program
>> BUG: memory leak
>> unreferenced object 0xffff888122bca200 (size 128):
>> comm "syz-executor232", pid 7065, jiffies 4294943817 (age 8.880s)
>> 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 18 a2 bc 22 81 88 ff ff ..........."....
>> backtrace:
>> [<000000005bada299>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
>> [<000000005bada299>] slab_post_alloc_hook mm/slab.h:522 [inline]
>> [<000000005bada299>] slab_alloc mm/slab.c:3319 [inline]
>> [<000000005bada299>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
>> [<00000000e7bcdc9f>] kmalloc include/linux/slab.h:552 [inline]
>> [<00000000e7bcdc9f>] kzalloc include/linux/slab.h:748 [inline]
>> [<00000000e7bcdc9f>] tipc_group_create_member+0x3c/0x190 net/tipc/group.c:306
>> [<0000000005f56f40>] tipc_group_add_member+0x34/0x40 net/tipc/group.c:327
>> [<0000000044406683>] tipc_nametbl_build_group+0x9b/0xf0 net/tipc/name_table.c:600
>> [<000000009f71e803>] tipc_sk_join net/tipc/socket.c:2901 [inline]
>> [<000000009f71e803>] tipc_setsockopt+0x170/0x490 net/tipc/socket.c:3006
>> [<000000007f61cbc2>] __sys_setsockopt+0x10f/0x220 net/socket.c:2084
>> [<00000000cc630372>] __do_sys_setsockopt net/socket.c:2100 [inline]
>> [<00000000cc630372>] __se_sys_setsockopt net/socket.c:2097 [inline]
>> [<00000000cc630372>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2097
>> [<00000000ec30be33>] do_syscall_64+0x76/0x1a0 arch/x86/entry/common.c:296
>> [<00000000271be3e6>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
Acked-by: Ying Xue <ying.xue@windriver.com>
>
> --- a/net/tipc/group.c
> +++ b/net/tipc/group.c
> @@ -273,7 +273,7 @@ static struct tipc_member *tipc_group_fi
> return NULL;
> }
>
> -static void tipc_group_add_to_tree(struct tipc_group *grp,
> +static struct tipc_member *tipc_group_add_to_tree(struct tipc_group *grp,
> struct tipc_member *m)
> {
> u64 nkey, key = (u64)m->node << 32 | m->port;
> @@ -282,7 +282,6 @@ static void tipc_group_add_to_tree(struc
>
> n = &grp->members.rb_node;
> while (*n) {
> - tmp = container_of(*n, struct tipc_member, tree_node);
> parent = *n;
> tmp = container_of(parent, struct tipc_member, tree_node);
> nkey = (u64)tmp->node << 32 | tmp->port;
> @@ -291,17 +290,18 @@ static void tipc_group_add_to_tree(struc
> else if (key > nkey)
> n = &(*n)->rb_right;
> else
> - return;
> + return tmp;
> }
> rb_link_node(&m->tree_node, parent, n);
> rb_insert_color(&m->tree_node, &grp->members);
> + return m;
> }
>
> static struct tipc_member *tipc_group_create_member(struct tipc_group *grp,
> u32 node, u32 port,
> u32 instance, int state)
> {
> - struct tipc_member *m;
> + struct tipc_member *m, *n;
>
> m = kzalloc(sizeof(*m), GFP_ATOMIC);
> if (!m)
> @@ -315,10 +315,14 @@ static struct tipc_member *tipc_group_cr
> m->instance = instance;
> m->bc_acked = grp->bc_snd_nxt - 1;
> grp->member_cnt++;
> - tipc_group_add_to_tree(grp, m);
> - tipc_nlist_add(&grp->dests, m->node);
> - m->state = state;
> - return m;
> + n = tipc_group_add_to_tree(grp, m);
> + if (n == m) {
> + tipc_nlist_add(&grp->dests, m->node);
> + m->state = state;
> + } else {
> + kfree(m);
> + }
> + return n;
> }
>
> void tipc_group_add_member(struct tipc_group *grp, u32 node,
> --
>
>
^ permalink raw reply
* Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites
From: Jan Kara @ 2019-08-02 12:41 UTC (permalink / raw)
To: Michal Hocko
Cc: john.hubbard, Andrew Morton, Christoph Hellwig, Dan Williams,
Dave Chinner, Dave Hansen, Ira Weiny, Jan Kara, Jason Gunthorpe,
Jérôme Glisse, LKML, amd-gfx, ceph-devel, devel, devel,
dri-devel, intel-gfx, kvm, linux-arm-kernel, linux-block,
linux-crypto, linux-fbdev, linux-fsdevel, linux-media, linux-mm,
linux-nfs, linux-rdma, linux-rpi-kernel, linux-xfs, netdev,
rds-devel, sparclinux, x86, xen-devel, John Hubbard
In-Reply-To: <20190802091244.GD6461@dhcp22.suse.cz>
On Fri 02-08-19 11:12:44, Michal Hocko wrote:
> On Thu 01-08-19 19:19:31, john.hubbard@gmail.com wrote:
> [...]
> > 2) Convert all of the call sites for get_user_pages*(), to
> > invoke put_user_page*(), instead of put_page(). This involves dozens of
> > call sites, and will take some time.
>
> How do we make sure this is the case and it will remain the case in the
> future? There must be some automagic to enforce/check that. It is simply
> not manageable to do it every now and then because then 3) will simply
> be never safe.
>
> Have you considered coccinele or some other scripted way to do the
> transition? I have no idea how to deal with future changes that would
> break the balance though.
Yeah, that's why I've been suggesting at LSF/MM that we may need to create
a gup wrapper - say vaddr_pin_pages() - and track which sites dropping
references got converted by using this wrapper instead of gup. The
counterpart would then be more logically named as unpin_page() or whatever
instead of put_user_page(). Sure this is not completely foolproof (you can
create new callsite using vaddr_pin_pages() and then just drop refs using
put_page()) but I suppose it would be a high enough barrier for missed
conversions... Thoughts?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker
From: Jason Gunthorpe @ 2019-08-02 12:46 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <42ead87b-1749-4c73-cbe4-29dbeb945041@redhat.com>
On Fri, Aug 02, 2019 at 05:40:07PM +0800, Jason Wang wrote:
> > This must be a proper barrier, like a spinlock, mutex, or
> > synchronize_rcu.
>
>
> I start with synchronize_rcu() but both you and Michael raise some
> concern.
I've also idly wondered if calling synchronize_rcu() under the various
mm locks is a deadlock situation.
> Then I try spinlock and mutex:
>
> 1) spinlock: add lots of overhead on datapath, this leads 0 performance
> improvement.
I think the topic here is correctness not performance improvement
> 2) SRCU: full memory barrier requires on srcu_read_lock(), which still leads
> little performance improvement
> 3) mutex: a possible issue is need to wait for the page to be swapped in (is
> this unacceptable ?), another issue is that we need hold vq lock during
> range overlap check.
I have a feeling that mmu notififers cannot safely become dependent on
progress of swap without causing deadlock. You probably should avoid
this.
> > And, again, you can't re-invent a spinlock with open coding and get
> > something better.
>
> So the question is if waiting for swap is considered to be unsuitable for
> MMU notifiers. If not, it would simplify codes. If not, we still need to
> figure out a possible solution.
>
> Btw, I come up another idea, that is to disable preemption when vhost thread
> need to access the memory. Then register preempt notifier and if vhost
> thread is preempted, we're sure no one will access the memory and can do the
> cleanup.
I think you should use the spinlock so at least the code is obviously
functionally correct and worry about designing some properly justified
performance change after.
Jason
^ permalink raw reply
* [PATCH] mkiss: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:57 UTC (permalink / raw)
Cc: David S . Miller, netdev, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/hamradio/mkiss.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index 442018ccd65e..b0afd7d13553 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -70,7 +70,7 @@ struct mkiss {
#define CRC_MODE_FLEX_TEST 3
#define CRC_MODE_SMACK_TEST 4
- atomic_t refcnt;
+ refcount_t refcnt;
struct completion dead;
};
@@ -668,7 +668,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
read_lock(&disc_data_lock);
ax = tty->disc_data;
if (ax)
- atomic_inc(&ax->refcnt);
+ refcount_inc(&ax->refcnt);
read_unlock(&disc_data_lock);
return ax;
@@ -676,7 +676,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
static void mkiss_put(struct mkiss *ax)
{
- if (atomic_dec_and_test(&ax->refcnt))
+ if (refcount_dec_and_test(&ax->refcnt))
complete(&ax->dead);
}
@@ -704,7 +704,7 @@ static int mkiss_open(struct tty_struct *tty)
ax->dev = dev;
spin_lock_init(&ax->buflock);
- atomic_set(&ax->refcnt, 1);
+ refcount_set(&ax->refcnt, 1);
init_completion(&ax->dead);
ax->tty = tty;
@@ -784,7 +784,7 @@ static void mkiss_close(struct tty_struct *tty)
* We have now ensured that nobody can start using ap from now on, but
* we have to wait for all existing users to finish.
*/
- if (!atomic_dec_and_test(&ax->refcnt))
+ if (!refcount_dec_and_test(&ax->refcnt))
wait_for_completion(&ax->dead);
/*
* Halt the transmit queue so that a new transmit cannot scribble
--
2.20.1
^ permalink raw reply related
* [PATCH] niu: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:57 UTC (permalink / raw)
Cc: David S . Miller, netdev, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Also convert refcount from 0-based to 1-based.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/sun/niu.c | 6 +++---
drivers/net/ethernet/sun/niu.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 0bc5863bffeb..5bf096e51db7 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -9464,7 +9464,7 @@ static struct niu_parent *niu_new_parent(struct niu *np,
memcpy(&p->id, id, sizeof(*id));
p->plat_type = ptype;
INIT_LIST_HEAD(&p->list);
- atomic_set(&p->refcnt, 0);
+ refcount_set(&p->refcnt, 1);
list_add(&p->list, &niu_parent_list);
spin_lock_init(&p->lock);
@@ -9524,7 +9524,7 @@ static struct niu_parent *niu_get_parent(struct niu *np,
port_name);
if (!err) {
p->ports[port] = np;
- atomic_inc(&p->refcnt);
+ refcount_inc(&p->refcnt);
}
}
mutex_unlock(&niu_parent_lock);
@@ -9552,7 +9552,7 @@ static void niu_put_parent(struct niu *np)
p->ports[port] = NULL;
np->parent = NULL;
- if (atomic_dec_and_test(&p->refcnt)) {
+ if (refcount_dec_and_test(&p->refcnt)) {
list_del(&p->list);
platform_device_unregister(p->plat_dev);
}
diff --git a/drivers/net/ethernet/sun/niu.h b/drivers/net/ethernet/sun/niu.h
index 04c215f91fc0..755e6dd4c903 100644
--- a/drivers/net/ethernet/sun/niu.h
+++ b/drivers/net/ethernet/sun/niu.h
@@ -3071,7 +3071,7 @@ struct niu_parent {
struct niu *ports[NIU_MAX_PORTS];
- atomic_t refcnt;
+ refcount_t refcnt;
struct list_head list;
spinlock_t lock;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v5 5/6] flow_offload: support get flow_block immediately
From: wenxu @ 2019-08-02 13:09 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: jiri, pablo, fw, netfilter-devel, netdev, John Hurley
In-Reply-To: <bac5c6a5-8a1b-ee74-988b-6c2a71885761@ucloud.cn>
在 2019/8/2 18:45, wenxu 写道:
> On 8/2/2019 7:11 AM, Jakub Kicinski wrote:
>> On Thu, 1 Aug 2019 11:03:46 +0800, wenxu@ucloud.cn wrote:
>>> From: wenxu <wenxu@ucloud.cn>
>>>
>>> The new flow-indr-block can't get the tcf_block
>>> directly. It provide a callback list to find the flow_block immediately
>>> when the device register and contain a ingress block.
>>>
>>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> First of all thanks for splitting the series up into more patches,
>> it is easier to follow the logic now!
>>
>>> @@ -328,6 +348,7 @@ struct flow_indr_block_dev {
>>>
>>> INIT_LIST_HEAD(&indr_dev->cb_list);
>>> indr_dev->dev = dev;
>>> + flow_get_default_block(indr_dev);
>>> if (rhashtable_insert_fast(&indr_setup_block_ht, &indr_dev->ht_node,
>>> flow_indr_setup_block_ht_params)) {
>>> kfree(indr_dev);
>> I wonder if it's still practical to keep the block information in the
>> indr_dev structure at all. The way this used to work was:
>>
>>
>> [hash table of devices] --------------
>> | | netdev |
>> | | refcnt |
>> indir_dev[tun0]| ------ | cached block | ---- [ TC block ]
>> | | callbacks | .
>> | -------------- \__ [cb, cb_priv, cb_ident]
>> | [cb, cb_priv, cb_ident]
>> | --------------
>> | | netdev |
>> | | refcnt |
>> indir_dev[tun1]| ------ | cached block | ---- [ TC block ]
>> | | callbacks |.
>> ----------------- -------------- \__ [cb, cb_priv, cb_ident]
>> [cb, cb_priv, cb_ident]
>>
>>
>> In the example above we have two tunnels tun0 and tun1, each one has a
>> indr_dev structure allocated, and for each one of them two drivers
>> registered for callbacks (hence the callbacks list has two entries).
>>
>> We used to cache the TC block in the indr_dev structure, but now that
>> there are multiple subsytems using the indr_dev we either have to have
>> a list of cached blocks (with entries for each subsystem) or just always
>> iterate over the subsystems :(
>>
>> After all the same device may have both a TC block and a NFT block.
>>
>> I think always iterating would be easier:
>>
>> The indr_dev struct would no longer have the block pointer, instead
>> when new driver registers for the callback instead of:
>>
>> if (indr_dev->ing_cmd_cb)
>> indr_dev->ing_cmd_cb(indr_dev->dev, indr_dev->flow_block,
>> indr_block_cb->cb, indr_block_cb->cb_priv,
>> FLOW_BLOCK_BIND);
>>
>> We'd have something like the loop in flow_get_default_block():
>>
>> for each (subsystem)
>> subsystem->handle_new_indir_cb(indr_dev, cb);
>>
>> And then per-subsystem logic would actually call the cb. Or:
>>
>> for each (subsystem)
>> block = get_default_block(indir_dev)
>> indr_dev->ing_cmd_cb(...)
> nft dev chian is also based on register_netdevice_notifier, So for unregister case,
>
> the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister. is right?
>
> So maybe we can cache the block as a list of all the subsystem in indr_dev ?
when the device is unregister the nft netdev chain related to this device will also be delete through netdevice_notifier
. So for unregister case,the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister.
cache for the block is not work because the chain already be delete and free. Maybe it improve the prio of
rep_netdev_event can help this?
>
>
^ permalink raw reply
* [PATCH AUTOSEL 5.2 06/76] netfilter: nf_tables: fix module autoload for redir
From: Sasha Levin @ 2019-08-02 13:18 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Christian Hesse, Pablo Neira Ayuso, Sasha Levin, netfilter-devel,
coreteam, netdev
In-Reply-To: <20190802131951.11600-1-sashal@kernel.org>
From: Christian Hesse <mail@eworm.de>
[ Upstream commit f41828ee10b36644bb2b2bfa9dd1d02f55aa0516 ]
Fix expression for autoloading.
Fixes: 5142967ab524 ("netfilter: nf_tables: fix module autoload with inet family")
Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nft_redir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c
index 8487eeff5c0ec..43eeb1f609f13 100644
--- a/net/netfilter/nft_redir.c
+++ b/net/netfilter/nft_redir.c
@@ -291,4 +291,4 @@ module_exit(nft_redir_module_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Arturo Borrero Gonzalez <arturo@debian.org>");
-MODULE_ALIAS_NFT_EXPR("nat");
+MODULE_ALIAS_NFT_EXPR("redir");
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.2 30/76] allocate_flower_entry: should check for null deref
From: Sasha Levin @ 2019-08-02 13:19 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Navid Emamdoost, David S . Miller, Sasha Levin, netdev
In-Reply-To: <20190802131951.11600-1-sashal@kernel.org>
From: Navid Emamdoost <navid.emamdoost@gmail.com>
[ Upstream commit bb1320834b8a80c6ac2697ab418d066981ea08ba ]
allocate_flower_entry does not check for allocation success, but tries
to deref the result. I only moved the spin_lock under null check, because
the caller is checking allocation's status at line 652.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index cfaf8f618d1f3..56742fa0c1af6 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -67,7 +67,8 @@ static struct ch_tc_pedit_fields pedits[] = {
static struct ch_tc_flower_entry *allocate_flower_entry(void)
{
struct ch_tc_flower_entry *new = kzalloc(sizeof(*new), GFP_KERNEL);
- spin_lock_init(&new->lock);
+ if (new)
+ spin_lock_init(&new->lock);
return new;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 03/30] netfilter: Fix rpfilter dropping vrf packets by mistake
From: Sasha Levin @ 2019-08-02 13:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Miaohe Lin, Pablo Neira Ayuso, Sasha Levin, netfilter-devel,
coreteam, netdev
In-Reply-To: <20190802132422.13963-1-sashal@kernel.org>
From: Miaohe Lin <linmiaohe@huawei.com>
[ Upstream commit b575b24b8eee37f10484e951b62ce2a31c579775 ]
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.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/ipv4/netfilter/ipt_rpfilter.c | 1 +
net/ipv6/netfilter/ip6t_rpfilter.c | 8 ++++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 37fb9552e8589..341d1bd637af2 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -96,6 +96,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
flow.flowi4_tos = RT_TOS(iph->tos);
flow.flowi4_scope = RT_SCOPE_UNIVERSE;
+ flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par), info->flags) ^ invert;
}
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 40eb16bd97860..d535768bea0fd 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
if (rpfilter_addr_linklocal(&iph->saddr)) {
lookup_flags |= RT6_LOOKUP_F_IFACE;
fl6.flowi6_oif = dev->ifindex;
- } else if ((flags & XT_RPFILTER_LOOSE) == 0)
+ /* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
+ } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev) ||
+ (flags & XT_RPFILTER_LOOSE) == 0)
fl6.flowi6_oif = dev->ifindex;
rt = (void *) ip6_route_lookup(net, &fl6, lookup_flags);
@@ -73,7 +75,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
goto out;
}
- if (rt->rt6i_idev->dev == dev || (flags & XT_RPFILTER_LOOSE))
+ if (rt->rt6i_idev->dev == dev ||
+ l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex ||
+ (flags & XT_RPFILTER_LOOSE))
ret = true;
out:
ip6_rt_put(rt);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 01/22] netfilter: nfnetlink: avoid deadlock due to synchronous request_module
From: Sasha Levin @ 2019-08-02 13:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Thomas Jarosch, Juliana Rodrigueiro,
Pablo Neira Ayuso, Sasha Levin, netfilter-devel, coreteam, netdev
From: Florian Westphal <fw@strlen.de>
[ Upstream commit 1b0890cd60829bd51455dc5ad689ed58c4408227 ]
Thomas and Juliana report a deadlock when running:
(rmmod nf_conntrack_netlink/xfrm_user)
conntrack -e NEW -E &
modprobe -v xfrm_user
They provided following analysis:
conntrack -e NEW -E
netlink_bind()
netlink_lock_table() -> increases "nl_table_users"
nfnetlink_bind()
# does not unlock the table as it's locked by netlink_bind()
__request_module()
call_usermodehelper_exec()
This triggers "modprobe nf_conntrack_netlink" from kernel, netlink_bind()
won't return until modprobe process is done.
"modprobe xfrm_user":
xfrm_user_init()
register_pernet_subsys()
-> grab pernet_ops_rwsem
..
netlink_table_grab()
calls schedule() as "nl_table_users" is non-zero
so modprobe is blocked because netlink_bind() increased
nl_table_users while also holding pernet_ops_rwsem.
"modprobe nf_conntrack_netlink" runs and inits nf_conntrack_netlink:
ctnetlink_init()
register_pernet_subsys()
-> blocks on "pernet_ops_rwsem" thanks to xfrm_user module
both modprobe processes wait on one another -- neither can make
progress.
Switch netlink_bind() to "nowait" modprobe -- this releases the netlink
table lock, which then allows both modprobe instances to complete.
Reported-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Reported-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nfnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 2278d9ab723bf..9837a61cb3e3b 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -490,7 +490,7 @@ static int nfnetlink_bind(struct net *net, int group)
ss = nfnetlink_get_subsys(type << 8);
rcu_read_unlock();
if (!ss)
- request_module("nfnetlink-subsys-%d", type);
+ request_module_nowait("nfnetlink-subsys-%d", type);
return 0;
}
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 03/22] mac80211: don't warn about CW params when not using them
From: Sasha Levin @ 2019-08-02 13:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Brian Norris, Johannes Berg, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20190802132547.14517-1-sashal@kernel.org>
From: Brian Norris <briannorris@chromium.org>
[ Upstream commit d2b3fe42bc629c2d4002f652b3abdfb2e72991c7 ]
ieee80211_set_wmm_default() normally sets up the initial CW min/max for
each queue, except that it skips doing this if the driver doesn't
support ->conf_tx. We still end up calling drv_conf_tx() in some cases
(e.g., ieee80211_reconfig()), which also still won't do anything
useful...except it complains here about the invalid CW parameters.
Let's just skip the WARN if we weren't going to do anything useful with
the parameters.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Link: https://lore.kernel.org/r/20190718015712.197499-1-briannorris@chromium.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/mac80211/driver-ops.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index bb886e7db47f1..f783d1377d9a8 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -169,11 +169,16 @@ int drv_conf_tx(struct ieee80211_local *local,
if (!check_sdata_in_driver(sdata))
return -EIO;
- if (WARN_ONCE(params->cw_min == 0 ||
- params->cw_min > params->cw_max,
- "%s: invalid CW_min/CW_max: %d/%d\n",
- sdata->name, params->cw_min, params->cw_max))
+ if (params->cw_min == 0 || params->cw_min > params->cw_max) {
+ /*
+ * If we can't configure hardware anyway, don't warn. We may
+ * never have initialized the CW parameters.
+ */
+ WARN_ONCE(local->ops->conf_tx,
+ "%s: invalid CW_min/CW_max: %d/%d\n",
+ sdata->name, params->cw_min, params->cw_max);
return -EINVAL;
+ }
trace_drv_conf_tx(local, sdata, ac, params);
if (local->ops->conf_tx)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 01/17] netfilter: nfnetlink: avoid deadlock due to synchronous request_module
From: Sasha Levin @ 2019-08-02 13:26 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Thomas Jarosch, Juliana Rodrigueiro,
Pablo Neira Ayuso, Sasha Levin, netfilter-devel, coreteam, netdev
From: Florian Westphal <fw@strlen.de>
[ Upstream commit 1b0890cd60829bd51455dc5ad689ed58c4408227 ]
Thomas and Juliana report a deadlock when running:
(rmmod nf_conntrack_netlink/xfrm_user)
conntrack -e NEW -E &
modprobe -v xfrm_user
They provided following analysis:
conntrack -e NEW -E
netlink_bind()
netlink_lock_table() -> increases "nl_table_users"
nfnetlink_bind()
# does not unlock the table as it's locked by netlink_bind()
__request_module()
call_usermodehelper_exec()
This triggers "modprobe nf_conntrack_netlink" from kernel, netlink_bind()
won't return until modprobe process is done.
"modprobe xfrm_user":
xfrm_user_init()
register_pernet_subsys()
-> grab pernet_ops_rwsem
..
netlink_table_grab()
calls schedule() as "nl_table_users" is non-zero
so modprobe is blocked because netlink_bind() increased
nl_table_users while also holding pernet_ops_rwsem.
"modprobe nf_conntrack_netlink" runs and inits nf_conntrack_netlink:
ctnetlink_init()
register_pernet_subsys()
-> blocks on "pernet_ops_rwsem" thanks to xfrm_user module
both modprobe processes wait on one another -- neither can make
progress.
Switch netlink_bind() to "nowait" modprobe -- this releases the netlink
table lock, which then allows both modprobe instances to complete.
Reported-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Reported-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nfnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 9adedba78eeac..044559c10e98e 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -495,7 +495,7 @@ static int nfnetlink_bind(struct net *net, int group)
ss = nfnetlink_get_subsys(type << 8);
rcu_read_unlock();
if (!ss)
- request_module("nfnetlink-subsys-%d", type);
+ request_module_nowait("nfnetlink-subsys-%d", type);
return 0;
}
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 03/17] mac80211: don't warn about CW params when not using them
From: Sasha Levin @ 2019-08-02 13:26 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Brian Norris, Johannes Berg, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20190802132635.14885-1-sashal@kernel.org>
From: Brian Norris <briannorris@chromium.org>
[ Upstream commit d2b3fe42bc629c2d4002f652b3abdfb2e72991c7 ]
ieee80211_set_wmm_default() normally sets up the initial CW min/max for
each queue, except that it skips doing this if the driver doesn't
support ->conf_tx. We still end up calling drv_conf_tx() in some cases
(e.g., ieee80211_reconfig()), which also still won't do anything
useful...except it complains here about the invalid CW parameters.
Let's just skip the WARN if we weren't going to do anything useful with
the parameters.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Link: https://lore.kernel.org/r/20190718015712.197499-1-briannorris@chromium.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/mac80211/driver-ops.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index c258f1041d330..df2e4e3112177 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -169,11 +169,16 @@ int drv_conf_tx(struct ieee80211_local *local,
if (!check_sdata_in_driver(sdata))
return -EIO;
- if (WARN_ONCE(params->cw_min == 0 ||
- params->cw_min > params->cw_max,
- "%s: invalid CW_min/CW_max: %d/%d\n",
- sdata->name, params->cw_min, params->cw_max))
+ if (params->cw_min == 0 || params->cw_min > params->cw_max) {
+ /*
+ * If we can't configure hardware anyway, don't warn. We may
+ * never have initialized the CW parameters.
+ */
+ WARN_ONCE(local->ops->conf_tx,
+ "%s: invalid CW_min/CW_max: %d/%d\n",
+ sdata->name, params->cw_min, params->cw_max);
return -EINVAL;
+ }
trace_drv_conf_tx(local, sdata, ac, params);
if (local->ops->conf_tx)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/3,v2] flow_offload hardware priority fixes
From: Pablo Neira Ayuso @ 2019-08-02 13:28 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, jakub.kicinski, jiri, marcelo.leitner, saeedm,
wenxu, gerlitz.or, paulb
Hi,
This patchset contains three updates for the flow_offload users:
1) Pass major tc priority to drivers so they do not have to
lshift it. This is a preparation patch for the fix coming in
patch 3/3.
2) Add a new structure to basechain objects to wrap the offload
data. This is another preparation patch for the fix coming in
patch 3/3.
3) Allocate priority field per rule from the commit path and
pass it on to drivers. Currently this is set to zero and
some drivers bail out with it, start by priority number 1
and allocate new priority from there on. The priority field
is limited to 8-bits at this stage for simplicity.
v2: address Jakub comments to not use the netfilter basechain
priority for this mapping.
Please, apply, thank you.
Pablo Neira Ayuso (3):
net: sched: use major priority number as hardware priority
netfilter: nf_tables_offload: add offload field to basechain
filter: nf_tables_offload: set priority field for rules
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
drivers/net/ethernet/mscc/ocelot_flower.c | 12 ++------
.../net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 +-
include/net/netfilter/nf_tables.h | 14 ++++++---
include/net/netfilter/nf_tables_offload.h | 6 ++++
include/net/pkt_cls.h | 2 +-
net/netfilter/nf_tables_api.c | 2 +-
net/netfilter/nf_tables_offload.c | 34 +++++++++++++++++-----
10 files changed, 51 insertions(+), 27 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/3,v2] net: sched: use major priority number as hardware priority
From: Pablo Neira Ayuso @ 2019-08-02 13:28 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, jakub.kicinski, jiri, marcelo.leitner, saeedm,
wenxu, gerlitz.or, paulb
In-Reply-To: <20190802132846.3067-1-pablo@netfilter.org>
tc transparently maps the software priority number to hardware. Update
it to pass the major priority which is what most drivers expect. Update
drivers too so they do not need to lshift the priority field of the
flow_cls_common_offload object. The stmmac driver is an exception, since
this code assumes the tc software priority is fine, therefore, lshift it
just to be conservative.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: no changes.
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
drivers/net/ethernet/mscc/ocelot_flower.c | 12 +++---------
drivers/net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 +-
include/net/pkt_cls.h | 2 +-
6 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index cc096f6011d9..744c0c640c10 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -3158,7 +3158,7 @@ mlx5e_flow_esw_attr_init(struct mlx5_esw_flow_attr *esw_attr,
esw_attr->parse_attr = parse_attr;
esw_attr->chain = f->common.chain_index;
- esw_attr->prio = TC_H_MAJ(f->common.prio) >> 16;
+ esw_attr->prio = f->common.prio;
esw_attr->in_rep = in_rep;
esw_attr->in_mdev = in_mdev;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index e8ac90564dbe..84a87d059333 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -471,7 +471,7 @@ int mlxsw_sp_acl_rulei_commit(struct mlxsw_sp_acl_rule_info *rulei)
void mlxsw_sp_acl_rulei_priority(struct mlxsw_sp_acl_rule_info *rulei,
unsigned int priority)
{
- rulei->priority = priority >> 16;
+ rulei->priority = priority;
}
void mlxsw_sp_acl_rulei_keymask_u32(struct mlxsw_sp_acl_rule_info *rulei,
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 59487d446a09..b894bc0c9c16 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -13,12 +13,6 @@ struct ocelot_port_block {
struct ocelot_port *port;
};
-static u16 get_prio(u32 prio)
-{
- /* prio starts from 0x1000 while the ids starts from 0 */
- return prio >> 16;
-}
-
static int ocelot_flower_parse_action(struct flow_cls_offload *f,
struct ocelot_ace_rule *rule)
{
@@ -168,7 +162,7 @@ static int ocelot_flower_parse(struct flow_cls_offload *f,
}
finished_key_parsing:
- ocelot_rule->prio = get_prio(f->common.prio);
+ ocelot_rule->prio = f->common.prio;
ocelot_rule->id = f->cookie;
return ocelot_flower_parse_action(f, ocelot_rule);
}
@@ -218,7 +212,7 @@ static int ocelot_flower_destroy(struct flow_cls_offload *f,
struct ocelot_ace_rule rule;
int ret;
- rule.prio = get_prio(f->common.prio);
+ rule.prio = f->common.prio;
rule.port = port_block->port;
rule.id = f->cookie;
@@ -236,7 +230,7 @@ static int ocelot_flower_stats_update(struct flow_cls_offload *f,
struct ocelot_ace_rule rule;
int ret;
- rule.prio = get_prio(f->common.prio);
+ rule.prio = f->common.prio;
rule.port = port_block->port;
rule.id = f->cookie;
ret = ocelot_ace_rule_stats_update(&rule);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
index 86e968cd5ffd..124a43dc136a 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
@@ -93,7 +93,7 @@ nfp_flower_install_rate_limiter(struct nfp_app *app, struct net_device *netdev,
return -EOPNOTSUPP;
}
- if (flow->common.prio != (1 << 16)) {
+ if (flow->common.prio != 1) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: qos rate limit offload requires highest priority");
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 58ea18af9813..5cd040215469 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -94,7 +94,7 @@ static int tc_fill_entry(struct stmmac_priv *priv,
struct stmmac_tc_entry *entry, *frag = NULL;
struct tc_u32_sel *sel = cls->knode.sel;
u32 off, data, mask, real_off, rem;
- u32 prio = cls->common.prio;
+ u32 prio = cls->common.prio << 16;
int ret;
/* Only 1 match per entry */
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e429809ca90d..98be18ef1ed3 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -646,7 +646,7 @@ tc_cls_common_offload_init(struct flow_cls_common_offload *cls_common,
{
cls_common->chain_index = tp->chain->index;
cls_common->protocol = tp->protocol;
- cls_common->prio = tp->prio;
+ cls_common->prio = tp->prio >> 16;
if (tc_skip_sw(flags) || flags & TCA_CLS_FLAGS_VERBOSE)
cls_common->extack = extack;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/3] netfilter: nf_tables_offload: add offload field to basechain
From: Pablo Neira Ayuso @ 2019-08-02 13:28 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, jakub.kicinski, jiri, marcelo.leitner, saeedm,
wenxu, gerlitz.or, paulb
In-Reply-To: <20190802132846.3067-1-pablo@netfilter.org>
Wrap offload objects in struct nft_base_chain around structure.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v1: initial patch version.
include/net/netfilter/nf_tables.h | 6 ++++--
include/net/netfilter/nf_tables_offload.h | 5 +++++
net/netfilter/nf_tables_api.c | 2 +-
net/netfilter/nf_tables_offload.c | 7 ++++---
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 9b624566b82d..87dbe62c0f27 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -952,7 +952,7 @@ struct nft_stats {
* @stats: per-cpu chain stats
* @chain: the chain
* @dev_name: device name that this base chain is attached to (if any)
- * @flow_block: flow block (for hardware offload)
+ * @offload: hardware offload data
*/
struct nft_base_chain {
struct nf_hook_ops ops;
@@ -962,7 +962,9 @@ struct nft_base_chain {
struct nft_stats __percpu *stats;
struct nft_chain chain;
char dev_name[IFNAMSIZ];
- struct flow_block flow_block;
+ struct {
+ struct flow_block flow_block;
+ } offload;
};
static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
index 3196663a10e3..fb3db391ade8 100644
--- a/include/net/netfilter/nf_tables_offload.h
+++ b/include/net/netfilter/nf_tables_offload.h
@@ -73,4 +73,9 @@ int nft_flow_rule_offload_commit(struct net *net);
(__reg)->key = __key; \
memset(&(__reg)->mask, 0xff, (__reg)->len);
+static inline void nft_basechain_offload_init(struct nft_base_chain *basechain)
+{
+ flow_block_init(&basechain->offload.flow_block);
+}
+
#endif
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 605a7cfe7ca7..a07d764c3555 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
chain->flags |= NFT_BASE_CHAIN | flags;
basechain->policy = NF_ACCEPT;
- flow_block_init(&basechain->flow_block);
+ nft_basechain_offload_init(basechain);
} else {
chain = kzalloc(sizeof(*chain), GFP_KERNEL);
if (chain == NULL)
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 64f5fd5f240e..84615381b06f 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -113,10 +113,11 @@ static void nft_flow_offload_common_init(struct flow_cls_common_offload *common,
static int nft_setup_cb_call(struct nft_base_chain *basechain,
enum tc_setup_type type, void *type_data)
{
+ struct flow_block *flow_block = &basechain->offload.flow_block;
struct flow_block_cb *block_cb;
int err;
- list_for_each_entry(block_cb, &basechain->flow_block.cb_list, list) {
+ list_for_each_entry(block_cb, &flow_block->cb_list, list) {
err = block_cb->cb(type, type_data, block_cb->cb_priv);
if (err < 0)
return err;
@@ -154,7 +155,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
static int nft_flow_offload_bind(struct flow_block_offload *bo,
struct nft_base_chain *basechain)
{
- list_splice(&bo->cb_list, &basechain->flow_block.cb_list);
+ list_splice(&bo->cb_list, &basechain->offload.flow_block.cb_list);
return 0;
}
@@ -198,7 +199,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
return -EOPNOTSUPP;
bo.command = cmd;
- bo.block = &basechain->flow_block;
+ bo.block = &basechain->offload.flow_block;
bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
bo.extack = &extack;
INIT_LIST_HEAD(&bo.cb_list);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/3] filter: nf_tables_offload: set priority field for rules
From: Pablo Neira Ayuso @ 2019-08-02 13:28 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, jakub.kicinski, jiri, marcelo.leitner, saeedm,
wenxu, gerlitz.or, paulb
In-Reply-To: <20190802132846.3067-1-pablo@netfilter.org>
This patch allocates the priority per rule starting from priority 1
since some drivers assume priority 0 never happens.
This patch is restricting the rule priority range to 8-bit integer since
the nft_rule object has 7-bit spare bits plus one that is scratched from
the handle. It should be possible to extend this later on by placing the
priority after the userdata area to turn this into 32-bits priority
field, to put this data away from the packet path cachelines.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v1: formerly "netfilter: nf_tables: map basechain priority to hardware priority"
address mapping to hardware based on comments from Jakub.
include/net/netfilter/nf_tables.h | 8 ++++++--
include/net/netfilter/nf_tables_offload.h | 1 +
net/netfilter/nf_tables_offload.c | 27 ++++++++++++++++++++++-----
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 87dbe62c0f27..a6308fcf5bf0 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -9,6 +9,7 @@
#include <linux/netfilter/nf_tables.h>
#include <linux/u64_stats_sync.h>
#include <linux/rhashtable.h>
+#include <linux/idr.h>
#include <net/netfilter/nf_flow_table.h>
#include <net/netlink.h>
#include <net/flow_offload.h>
@@ -824,14 +825,16 @@ int nft_expr_dump(struct sk_buff *skb, unsigned int attr,
* @genmask: generation mask
* @dlen: length of expression data
* @udata: user data is appended to the rule
+ * @prio: priority (for hardware offload)
* @data: expression data
*/
struct nft_rule {
struct list_head list;
- u64 handle:42,
+ u64 handle:41,
genmask:2,
dlen:12,
- udata:1;
+ udata:1,
+ prio:8;
unsigned char data[]
__attribute__((aligned(__alignof__(struct nft_expr))));
};
@@ -964,6 +967,7 @@ struct nft_base_chain {
char dev_name[IFNAMSIZ];
struct {
struct flow_block flow_block;
+ struct idr prio_idr;
} offload;
};
diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
index fb3db391ade8..70f226568fe7 100644
--- a/include/net/netfilter/nf_tables_offload.h
+++ b/include/net/netfilter/nf_tables_offload.h
@@ -76,6 +76,7 @@ int nft_flow_rule_offload_commit(struct net *net);
static inline void nft_basechain_offload_init(struct nft_base_chain *basechain)
{
flow_block_init(&basechain->offload.flow_block);
+ idr_init(&basechain->offload.prio_idr);
}
#endif
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 84615381b06f..21144938482a 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -103,10 +103,11 @@ void nft_offload_update_dependency(struct nft_offload_ctx *ctx,
}
static void nft_flow_offload_common_init(struct flow_cls_common_offload *common,
- __be16 proto,
- struct netlink_ext_ack *extack)
+ __be16 proto, u32 priority,
+ struct netlink_ext_ack *extack)
{
common->protocol = proto;
+ common->prio = priority;
common->extack = extack;
}
@@ -125,6 +126,8 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
return 0;
}
+#define NFT_OFFLOAD_PRIO_MAX U8_MAX
+
static int nft_flow_offload_rule(struct nft_trans *trans,
enum flow_cls_command command)
{
@@ -134,22 +137,36 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
struct nft_base_chain *basechain;
struct netlink_ext_ack extack;
__be16 proto = ETH_P_ALL;
+ u32 prio = 1;
+ int err;
if (!nft_is_base_chain(trans->ctx.chain))
return -EOPNOTSUPP;
basechain = nft_base_chain(trans->ctx.chain);
- if (flow)
+ if (flow) {
+ if (idr_alloc_u32(&basechain->offload.prio_idr, NULL, &prio,
+ NFT_OFFLOAD_PRIO_MAX, GFP_KERNEL) < 0)
+ return -E2BIG;
+
+ rule->prio = prio;
proto = flow->proto;
+ }
- nft_flow_offload_common_init(&cls_flow.common, proto, &extack);
+ nft_flow_offload_common_init(&cls_flow.common, proto, rule->prio,
+ &extack);
cls_flow.command = command;
cls_flow.cookie = (unsigned long) rule;
if (flow)
cls_flow.rule = flow->rule;
- return nft_setup_cb_call(basechain, TC_SETUP_CLSFLOWER, &cls_flow);
+ err = nft_setup_cb_call(basechain, TC_SETUP_CLSFLOWER, &cls_flow);
+ if ((err < 0 && command == FLOW_CLS_REPLACE) ||
+ (err == 0 && command == FLOW_CLS_DESTROY))
+ idr_remove(&basechain->offload.prio_idr, rule->prio);
+
+ return err;
}
static int nft_flow_offload_bind(struct flow_block_offload *bo,
--
2.11.0
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 07/30] mac80211: don't warn about CW params when not using them
From: Sasha Levin @ 2019-08-02 13:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Brian Norris, Johannes Berg, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20190802132422.13963-1-sashal@kernel.org>
From: Brian Norris <briannorris@chromium.org>
[ Upstream commit d2b3fe42bc629c2d4002f652b3abdfb2e72991c7 ]
ieee80211_set_wmm_default() normally sets up the initial CW min/max for
each queue, except that it skips doing this if the driver doesn't
support ->conf_tx. We still end up calling drv_conf_tx() in some cases
(e.g., ieee80211_reconfig()), which also still won't do anything
useful...except it complains here about the invalid CW parameters.
Let's just skip the WARN if we weren't going to do anything useful with
the parameters.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Link: https://lore.kernel.org/r/20190718015712.197499-1-briannorris@chromium.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/mac80211/driver-ops.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index bb886e7db47f1..f783d1377d9a8 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -169,11 +169,16 @@ int drv_conf_tx(struct ieee80211_local *local,
if (!check_sdata_in_driver(sdata))
return -EIO;
- if (WARN_ONCE(params->cw_min == 0 ||
- params->cw_min > params->cw_max,
- "%s: invalid CW_min/CW_max: %d/%d\n",
- sdata->name, params->cw_min, params->cw_max))
+ if (params->cw_min == 0 || params->cw_min > params->cw_max) {
+ /*
+ * If we can't configure hardware anyway, don't warn. We may
+ * never have initialized the CW parameters.
+ */
+ WARN_ONCE(local->ops->conf_tx,
+ "%s: invalid CW_min/CW_max: %d/%d\n",
+ sdata->name, params->cw_min, params->cw_max);
return -EINVAL;
+ }
trace_drv_conf_tx(local, sdata, ac, params);
if (local->ops->conf_tx)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 04/30] netfilter: nft_hash: fix symhash with modulus one
From: Sasha Levin @ 2019-08-02 13:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Laura Garcia Liebana, Pablo Neira Ayuso, Sasha Levin,
netfilter-devel, coreteam, netdev
In-Reply-To: <20190802132422.13963-1-sashal@kernel.org>
From: Laura Garcia Liebana <nevola@gmail.com>
[ Upstream commit 28b1d6ef53e3303b90ca8924bb78f31fa527cafb ]
The rule below doesn't work as the kernel raises -ERANGE.
nft add rule netdev nftlb lb01 ip daddr set \
symhash mod 1 map { 0 : 192.168.0.10 } fwd to "eth0"
This patch allows to use the symhash modulus with one
element, in the same way that the other types of hashes and
algorithms that uses the modulus parameter.
Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nft_hash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 24f2f7567ddb7..010a565b40001 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -131,7 +131,7 @@ static int nft_symhash_init(const struct nft_ctx *ctx,
priv->dreg = nft_parse_register(tb[NFTA_HASH_DREG]);
priv->modulus = ntohl(nla_get_be32(tb[NFTA_HASH_MODULUS]));
- if (priv->modulus <= 1)
+ if (priv->modulus < 1)
return -ERANGE;
if (priv->offset + priv->modulus - 1 < priv->offset)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 01/30] netfilter: nfnetlink: avoid deadlock due to synchronous request_module
From: Sasha Levin @ 2019-08-02 13:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Thomas Jarosch, Juliana Rodrigueiro,
Pablo Neira Ayuso, Sasha Levin, netfilter-devel, coreteam, netdev
From: Florian Westphal <fw@strlen.de>
[ Upstream commit 1b0890cd60829bd51455dc5ad689ed58c4408227 ]
Thomas and Juliana report a deadlock when running:
(rmmod nf_conntrack_netlink/xfrm_user)
conntrack -e NEW -E &
modprobe -v xfrm_user
They provided following analysis:
conntrack -e NEW -E
netlink_bind()
netlink_lock_table() -> increases "nl_table_users"
nfnetlink_bind()
# does not unlock the table as it's locked by netlink_bind()
__request_module()
call_usermodehelper_exec()
This triggers "modprobe nf_conntrack_netlink" from kernel, netlink_bind()
won't return until modprobe process is done.
"modprobe xfrm_user":
xfrm_user_init()
register_pernet_subsys()
-> grab pernet_ops_rwsem
..
netlink_table_grab()
calls schedule() as "nl_table_users" is non-zero
so modprobe is blocked because netlink_bind() increased
nl_table_users while also holding pernet_ops_rwsem.
"modprobe nf_conntrack_netlink" runs and inits nf_conntrack_netlink:
ctnetlink_init()
register_pernet_subsys()
-> blocks on "pernet_ops_rwsem" thanks to xfrm_user module
both modprobe processes wait on one another -- neither can make
progress.
Switch netlink_bind() to "nowait" modprobe -- this releases the netlink
table lock, which then allows both modprobe instances to complete.
Reported-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Reported-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nfnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 733d3e4a30d85..2cee032af46d2 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -530,7 +530,7 @@ static int nfnetlink_bind(struct net *net, int group)
ss = nfnetlink_get_subsys(type << 8);
rcu_read_unlock();
if (!ss)
- request_module("nfnetlink-subsys-%d", type);
+ request_module_nowait("nfnetlink-subsys-%d", type);
return 0;
}
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 15/42] allocate_flower_entry: should check for null deref
From: Sasha Levin @ 2019-08-02 13:22 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Navid Emamdoost, David S . Miller, Sasha Levin, netdev
In-Reply-To: <20190802132302.13537-1-sashal@kernel.org>
From: Navid Emamdoost <navid.emamdoost@gmail.com>
[ Upstream commit bb1320834b8a80c6ac2697ab418d066981ea08ba ]
allocate_flower_entry does not check for allocation success, but tries
to deref the result. I only moved the spin_lock under null check, because
the caller is checking allocation's status at line 652.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index f2aba5b160c2d..d45c435a599d6 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -67,7 +67,8 @@ static struct ch_tc_pedit_fields pedits[] = {
static struct ch_tc_flower_entry *allocate_flower_entry(void)
{
struct ch_tc_flower_entry *new = kzalloc(sizeof(*new), GFP_KERNEL);
- spin_lock_init(&new->lock);
+ if (new)
+ spin_lock_init(&new->lock);
return new;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 14/42] mac80211: don't warn about CW params when not using them
From: Sasha Levin @ 2019-08-02 13:22 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Brian Norris, Johannes Berg, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20190802132302.13537-1-sashal@kernel.org>
From: Brian Norris <briannorris@chromium.org>
[ Upstream commit d2b3fe42bc629c2d4002f652b3abdfb2e72991c7 ]
ieee80211_set_wmm_default() normally sets up the initial CW min/max for
each queue, except that it skips doing this if the driver doesn't
support ->conf_tx. We still end up calling drv_conf_tx() in some cases
(e.g., ieee80211_reconfig()), which also still won't do anything
useful...except it complains here about the invalid CW parameters.
Let's just skip the WARN if we weren't going to do anything useful with
the parameters.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Link: https://lore.kernel.org/r/20190718015712.197499-1-briannorris@chromium.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/mac80211/driver-ops.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index bb886e7db47f1..f783d1377d9a8 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -169,11 +169,16 @@ int drv_conf_tx(struct ieee80211_local *local,
if (!check_sdata_in_driver(sdata))
return -EIO;
- if (WARN_ONCE(params->cw_min == 0 ||
- params->cw_min > params->cw_max,
- "%s: invalid CW_min/CW_max: %d/%d\n",
- sdata->name, params->cw_min, params->cw_max))
+ if (params->cw_min == 0 || params->cw_min > params->cw_max) {
+ /*
+ * If we can't configure hardware anyway, don't warn. We may
+ * never have initialized the CW parameters.
+ */
+ WARN_ONCE(local->ops->conf_tx,
+ "%s: invalid CW_min/CW_max: %d/%d\n",
+ sdata->name, params->cw_min, params->cw_max);
return -EINVAL;
+ }
trace_drv_conf_tx(local, sdata, ac, params);
if (local->ops->conf_tx)
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox