* Re: [PATCH] netfilter/x_tables: do not fail xt_alloc_table_info too easilly
From: Vlastimil Babka @ 2018-08-08 8:16 UTC (permalink / raw)
To: Michal Hocko, Florian Westphal
Cc: Georgi Nikolov, Andrew Morton, David S. Miller, netfilter-devel,
coreteam, netdev, linux-kernel, linux-mm, Michal Hocko
In-Reply-To: <20180807195400.23687-1-mhocko@kernel.org>
On 08/07/2018 09:54 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> eacd86ca3b03 ("net/netfilter/x_tables.c: use kvmalloc()
> in xt_alloc_table_info()") has unintentionally fortified
> xt_alloc_table_info allocation when __GFP_RETRY has been dropped from
> the vmalloc fallback. Later on there was a syzbot report that this
> can lead to OOM killer invocations when tables are too large and
> 0537250fdc6c ("netfilter: x_tables: make allocation less aggressive")
> has been merged to restore the original behavior. Georgi Nikolov however
> noticed that he is not able to install his iptables anymore so this can
> be seen as a regression.
>
> The primary argument for 0537250fdc6c was that this allocation path
> shouldn't really trigger the OOM killer and kill innocent tasks. On the
> other hand the interface requires root and as such should allow what the
> admin asks for. Root inside a namespaces makes this more complicated
> because those might be not trusted in general. If they are not then such
> namespaces should be restricted anyway. Therefore drop the __GFP_NORETRY
> and replace it by __GFP_ACCOUNT to enfore memcg constrains on it.
>
> Fixes: 0537250fdc6c ("netfilter: x_tables: make allocation less aggressive")
> Reported-by: Georgi Nikolov <gnikolov@icdsoft.com>
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
IIRC According to Florian there are more places like this in the
netfilter code?
^ permalink raw reply
* KASAN: use-after-free Read in __rhashtable_lookup
From: syzbot @ 2018-08-08 7:37 UTC (permalink / raw)
To: davem, linux-kernel, linux-rdma, netdev, rds-devel,
santosh.shilimkar, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: a394b3af206c ppp: mppe: Remove VLA usage
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1335d6f0400000
kernel config: https://syzkaller.appspot.com/x/.config?x=cb640e89c1953f27
dashboard link: https://syzkaller.appspot.com/bug?extid=c6369755db236fb370e7
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c6369755db236fb370e7@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in memcmp+0xe8/0x150 lib/string.c:861
Read of size 1 at addr ffff8801d811ec30 by task syz-executor6/10904
CPU: 1 PID: 10904 Comm: syz-executor6 Not tainted 4.18.0-rc7+ #169
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
memcmp+0xe8/0x150 lib/string.c:861
memcmp include/linux/string.h:386 [inline]
rhashtable_compare include/linux/rhashtable.h:462 [inline]
__rhashtable_lookup.isra.8.constprop.19+0x70f/0xd50
include/linux/rhashtable.h:484
rhashtable_lookup include/linux/rhashtable.h:516 [inline]
rhashtable_lookup_fast include/linux/rhashtable.h:542 [inline]
rds_add_bound net/rds/bind.c:117 [inline]
rds_bind+0x7c9/0x1510 net/rds/bind.c:238
__sys_bind+0x331/0x440 net/socket.c:1500
__do_sys_bind net/socket.c:1511 [inline]
__se_sys_bind net/socket.c:1509 [inline]
__x64_sys_bind+0x73/0xb0 net/socket.c:1509
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x456b29
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f6ae7f39c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000031
RAX: ffffffffffffffda RBX: 00007f6ae7f3a6d4 RCX: 0000000000456b29
RDX: 0000000000000010 RSI: 0000000020000840 RDI: 0000000000000014
RBP: 0000000000930140 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004ca698 R14: 00000000004c26f1 R15: 0000000000000001
Allocated by task 10898:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
sk_prot_alloc+0x69/0x2e0 net/core/sock.c:1462
sk_alloc+0x106/0x17b0 net/core/sock.c:1522
rds_create+0x14f/0x770 net/rds/af_rds.c:666
__sock_create+0x53c/0x940 net/socket.c:1294
sock_create net/socket.c:1334 [inline]
__sys_socket+0x106/0x260 net/socket.c:1364
__do_sys_socket net/socket.c:1373 [inline]
__se_sys_socket net/socket.c:1371 [inline]
__x64_sys_socket+0x73/0xb0 net/socket.c:1371
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 10895:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
sk_prot_free net/core/sock.c:1503 [inline]
__sk_destruct+0x70a/0xa60 net/core/sock.c:1587
sk_destruct+0x78/0x90 net/core/sock.c:1595
__sk_free+0xcf/0x300 net/core/sock.c:1606
sk_free+0x42/0x50 net/core/sock.c:1617
sock_put include/net/sock.h:1691 [inline]
rds_release+0x3e9/0x570 net/rds/af_rds.c:91
__sock_release+0xd7/0x250 net/socket.c:598
sock_close+0x19/0x20 net/socket.c:1158
__fput+0x355/0x8b0 fs/file_table.c:209
____fput+0x15/0x20 fs/file_table.c:243
task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:192 [inline]
exit_to_usermode_loop+0x313/0x370 arch/x86/entry/common.c:166
prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8801d811e7c0
which belongs to the cache RDS of size 1608
The buggy address is located 1136 bytes inside of
1608-byte region [ffff8801d811e7c0, ffff8801d811ee08)
The buggy address belongs to the page:
page:ffffea0007604780 count:1 mapcount:0 mapping:ffff8801cd8a0300 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffff8801cd89f348 ffffea000716f8c8 ffff8801cd8a0300
raw: 0000000000000000 ffff8801d811e0c0 0000000100000002 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801d811eb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d811eb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801d811ec00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801d811ec80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d811ed00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* [PATCH 1/1] net-next: hinic: fix a problem in free_tx_poll()
From: Zhao Chen @ 2018-08-08 6:37 UTC (permalink / raw)
To: davem
Cc: linux-kernel, netdev, zhaochen6, tony.qu, yin.yinshi, luoshaokai,
fy.wang, luoxianjun, wangzhou1
This patch fixes the problem below. The problem can be reproduced by the
following steps:
1) Connecting all HiNIC interfaces
2) On server side
# sudo ifconfig eth0 192.168.100.1 up #Using MLX CX4 card
# iperf -s
3) On client side
# sudo ifconfig eth0 192.168.100.2 up #Using our HiNIC card
# iperf -c 192.168.101.1 -P 10 -t 100000
after hours of testing, we will see errors:
hinic 0000:05:00.0: No MGMT msg handler, mod = 0
hinic 0000:05:00.0: No MGMT msg handler, mod = 0
hinic 0000:05:00.0: No MGMT msg handler, mod = 0
hinic 0000:05:00.0: No MGMT msg handler, mod = 0
The errors are caused by the following problem.
1) The hinic_get_wqe() checks the "wq->delta" to allocate new WQEs:
if (atomic_sub_return(num_wqebbs, &wq->delta) <= 0) {
atomic_add(num_wqebbs, &wq->delta);
return ERR_PTR(-EBUSY);
}
If the WQE occupies multiple pages, the shadow WQE will be used. Then the
hinic_xmit_frame() fills the WQE.
2) While in parallel with 1), the free_tx_poll() checks the "wq->delta"
to free old WQEs:
if ((atomic_read(&wq->delta) + num_wqebbs) > wq->q_depth)
return ERR_PTR(-EBUSY);
There is a probability that the shadow WQE which hinic_xmit_frame() is
using will be damaged by copy_wqe_to_shadow():
if (curr_pg != end_pg) {
void *shadow_addr = &wq->shadow_wqe[curr_pg * wq->max_wqe_size];
copy_wqe_to_shadow(wq, shadow_addr, num_wqebbs, *cons_idx);
return shadow_addr;
}
This can cause WQE data error and you will see the above error messages.
This patch fixes the problem.
Signed-off-by: Zhao Chen <zhaochen6@huawei.com>
---
.../net/ethernet/huawei/hinic/hinic_hw_qp.c | 36 ++++++++++++++-----
.../net/ethernet/huawei/hinic/hinic_hw_qp.h | 6 +++-
drivers/net/ethernet/huawei/hinic/hinic_tx.c | 18 ++++++++--
3 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.c
index b9db6d649743..cb239627770f 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.c
@@ -635,17 +635,18 @@ void hinic_sq_write_wqe(struct hinic_sq *sq, u16 prod_idx,
}
/**
- * hinic_sq_read_wqe - read wqe ptr in the current ci and update the ci
+ * hinic_sq_read_wqebb - read wqe ptr in the current ci and update the ci, the
+ * wqe only have one wqebb
* @sq: send queue
* @skb: return skb that was saved
- * @wqe_size: the size of the wqe
+ * @wqe_size: the wqe size ptr
* @cons_idx: consumer index of the wqe
*
* Return wqe in ci position
**/
-struct hinic_sq_wqe *hinic_sq_read_wqe(struct hinic_sq *sq,
- struct sk_buff **skb,
- unsigned int *wqe_size, u16 *cons_idx)
+struct hinic_sq_wqe *hinic_sq_read_wqebb(struct hinic_sq *sq,
+ struct sk_buff **skb,
+ unsigned int *wqe_size, u16 *cons_idx)
{
struct hinic_hw_wqe *hw_wqe;
struct hinic_sq_wqe *sq_wqe;
@@ -658,6 +659,8 @@ struct hinic_sq_wqe *hinic_sq_read_wqe(struct hinic_sq *sq,
if (IS_ERR(hw_wqe))
return NULL;
+ *skb = sq->saved_skb[*cons_idx];
+
sq_wqe = &hw_wqe->sq_wqe;
ctrl = &sq_wqe->ctrl;
ctrl_info = be32_to_cpu(ctrl->ctrl_info);
@@ -665,11 +668,28 @@ struct hinic_sq_wqe *hinic_sq_read_wqe(struct hinic_sq *sq,
*wqe_size = sizeof(*ctrl) + sizeof(sq_wqe->task);
*wqe_size += SECT_SIZE_FROM_8BYTES(buf_sect_len);
+ *wqe_size = ALIGN(*wqe_size, sq->wq->wqebb_size);
- *skb = sq->saved_skb[*cons_idx];
+ return &hw_wqe->sq_wqe;
+}
+
+/**
+ * hinic_sq_read_wqe - read wqe ptr in the current ci and update the ci
+ * @sq: send queue
+ * @skb: return skb that was saved
+ * @wqe_size: the size of the wqe
+ * @cons_idx: consumer index of the wqe
+ *
+ * Return wqe in ci position
+ **/
+struct hinic_sq_wqe *hinic_sq_read_wqe(struct hinic_sq *sq,
+ struct sk_buff **skb,
+ unsigned int wqe_size, u16 *cons_idx)
+{
+ struct hinic_hw_wqe *hw_wqe;
- /* using the real wqe size to read wqe again */
- hw_wqe = hinic_read_wqe(sq->wq, *wqe_size, cons_idx);
+ hw_wqe = hinic_read_wqe(sq->wq, wqe_size, cons_idx);
+ *skb = sq->saved_skb[*cons_idx];
return &hw_wqe->sq_wqe;
}
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.h
index df729a1587e9..6c84f83ec283 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_qp.h
@@ -165,7 +165,11 @@ void hinic_sq_write_wqe(struct hinic_sq *sq, u16 prod_idx,
struct hinic_sq_wqe *hinic_sq_read_wqe(struct hinic_sq *sq,
struct sk_buff **skb,
- unsigned int *wqe_size, u16 *cons_idx);
+ unsigned int wqe_size, u16 *cons_idx);
+
+struct hinic_sq_wqe *hinic_sq_read_wqebb(struct hinic_sq *sq,
+ struct sk_buff **skb,
+ unsigned int *wqe_size, u16 *cons_idx);
void hinic_sq_put_wqe(struct hinic_sq *sq, unsigned int wqe_size);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 2353ec829c04..c5fca0356c9c 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -283,7 +283,11 @@ static void free_all_tx_skbs(struct hinic_txq *txq)
int nr_sges;
u16 ci;
- while ((sq_wqe = hinic_sq_read_wqe(sq, &skb, &wqe_size, &ci))) {
+ while ((sq_wqe = hinic_sq_read_wqebb(sq, &skb, &wqe_size, &ci))) {
+ sq_wqe = hinic_sq_read_wqe(sq, &skb, wqe_size, &ci);
+ if (!sq_wqe)
+ break;
+
nr_sges = skb_shinfo(skb)->nr_frags + 1;
hinic_sq_get_sges(sq_wqe, txq->free_sges, nr_sges);
@@ -319,11 +323,21 @@ static int free_tx_poll(struct napi_struct *napi, int budget)
do {
hw_ci = HW_CONS_IDX(sq) & wq->mask;
- sq_wqe = hinic_sq_read_wqe(sq, &skb, &wqe_size, &sw_ci);
+ /* Reading a WQEBB to get real WQE size and consumer index. */
+ sq_wqe = hinic_sq_read_wqebb(sq, &skb, &wqe_size, &sw_ci);
if ((!sq_wqe) ||
(((hw_ci - sw_ci) & wq->mask) * wq->wqebb_size < wqe_size))
break;
+ /* If this WQE have multiple WQEBBs, we will read again to get
+ * full size WQE.
+ */
+ if (wqe_size > wq->wqebb_size) {
+ sq_wqe = hinic_sq_read_wqe(sq, &skb, wqe_size, &sw_ci);
+ if (unlikely(!sq_wqe))
+ break;
+ }
+
tx_bytes += skb->len;
pkts++;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v6 7/9] net/mlx4: Do not call pcie_print_link_status()
From: Leon Romanovsky @ 2018-08-08 6:10 UTC (permalink / raw)
To: Alexandru Gagniuc
Cc: linux-pci, bhelgaas, jakub.kicinski, keith.busch, alex_gagniuc,
austin_bolen, shyam_iyer, Ariel Elior, everest-linux-l2,
David S. Miller, Michael Chan, Ganesh Goudar, Jeff Kirsher,
Tariq Toukan, Saeed Mahameed, Dirk van der Merwe, netdev,
linux-kernel, intel-wired-lan, linux-rdma, oss-drivers
In-Reply-To: <20180806232600.25694-7-mr.nuke.me@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]
On Mon, Aug 06, 2018 at 06:25:41PM -0500, Alexandru Gagniuc wrote:
> This is now done by the PCI core to warn of sub-optimal bandwidth.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 7 -------
> 1 file changed, 7 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* for editing
From: Roy Robinson @ 2018-08-08 2:16 UTC (permalink / raw)
To: netdev
Did you receive my email yesterday?
Just want to check if you have needs for photo editing for our studio?
We normally edit 300 images within 12-24 hours.
We take care of different kinds of e-commerce photos, jewelry images, and
portrait model images.
Including cutting out and clipping path and others, we also give retouching
for your images.
You may drop us a photo if you want to check our quality, we will provide
testing.
Thanks,
Roy
^ permalink raw reply
* Re: [PATCH v6 8/9] net/mlx5: Do not call pcie_print_link_status()
From: Leon Romanovsky @ 2018-08-08 6:08 UTC (permalink / raw)
To: Alexandru Gagniuc
Cc: linux-pci, bhelgaas, jakub.kicinski, keith.busch, alex_gagniuc,
austin_bolen, shyam_iyer, Ariel Elior, everest-linux-l2,
David S. Miller, Michael Chan, Ganesh Goudar, Jeff Kirsher,
Tariq Toukan, Saeed Mahameed, Dirk van der Merwe, netdev,
linux-kernel, intel-wired-lan, linux-rdma, oss-drivers
In-Reply-To: <20180806232600.25694-8-mr.nuke.me@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 357 bytes --]
On Mon, Aug 06, 2018 at 06:25:42PM -0500, Alexandru Gagniuc wrote:
> This is now done by the PCI core to warn of sub-optimal bandwidth.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ----
> 1 file changed, 4 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* wait for your reply
From: Roy Robinson @ 2018-08-07 10:57 UTC (permalink / raw)
To: netdev
Did you receive my email yesterday?
Just want to check if you have needs for photo editing for our studio?
We normally edit 300 images within 12-24 hours.
We take care of different kinds of e-commerce photos, jewelry images, and
portrait model images.
Including cutting out and clipping path and others, we also give retouching
for your images.
You may drop us a photo if you want to check our quality, we will provide
testing.
Thanks,
Roy
^ permalink raw reply
* wait for your reply
From: Roy Robinson @ 2018-08-07 10:43 UTC (permalink / raw)
To: netdev
Did you receive my email yesterday?
Just want to check if you have needs for photo editing for our studio?
We normally edit 300 images within 12-24 hours.
We take care of different kinds of e-commerce photos, jewelry images, and
portrait model images.
Including cutting out and clipping path and others, we also give retouching
for your images.
You may drop us a photo if you want to check our quality, we will provide
testing.
Thanks,
Roy
^ permalink raw reply
* [PATCH net-next] netfilter: nfnetlink_osf: fix using plain integer as NULL warning
From: Wei Yongjun @ 2018-08-08 3:23 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: Wei Yongjun, netfilter-devel, coreteam, netdev, kernel-janitors
Fixes the following sparse warning:
net/netfilter/nfnetlink_osf.c:274:24: warning:
Using plain integer as NULL pointer
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
net/netfilter/nfnetlink_osf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
index f9dba62..00db27d 100644
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -271,7 +271,7 @@ const char *nf_osf_find(const struct sk_buff *skb,
tcp = nf_osf_hdr_ctx_init(&ctx, skb, ip, opts);
if (!tcp)
- return false;
+ return NULL;
list_for_each_entry_rcu(kf, &nf_osf_fingers[ctx.df], finger_entry) {
f = &kf->finger;
^ permalink raw reply related
* [PATCH net-next] ieee802154: hwsim: fix missing unlock on error in hwsim_add_one()
From: Wei Yongjun @ 2018-08-08 3:10 UTC (permalink / raw)
To: Alexander Aring, Stefan Schmidt
Cc: Wei Yongjun, linux-wpan, netdev, kernel-janitors
Add the missing unlock before return from function hwsim_add_one()
in the error handling case.
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ieee802154/mac802154_hwsim.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index f4e9205..44d398c 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -810,8 +810,10 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
mutex_lock(&hwsim_phys_lock);
if (init) {
err = hwsim_subscribe_all_others(phy);
- if (err < 0)
+ if (err < 0) {
+ mutex_unlock(&hwsim_phys_lock);
goto err_reg;
+ }
}
list_add_tail(&phy->list, &hwsim_phys);
mutex_unlock(&hwsim_phys_lock);
^ permalink raw reply related
* [PATCH net-next] ieee802154: hwsim: fix copy-paste error in hwsim_set_edge_lqi()
From: Wei Yongjun @ 2018-08-08 2:43 UTC (permalink / raw)
To: Alexander Aring, Stefan Schmidt
Cc: Wei Yongjun, linux-wpan, netdev, kernel-janitors
The return value from kzalloc() is not checked correctly. The
test is done against a wrong variable. This patch fix it.
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ieee802154/mac802154_hwsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 1982308..04f4100 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -564,7 +564,7 @@ static int hwsim_set_edge_lqi(struct sk_buff *msg, struct genl_info *info)
}
einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
- if (!info) {
+ if (!einfo) {
mutex_unlock(&hwsim_phys_lock);
return -ENOMEM;
}
^ permalink raw reply related
* Re: [PATCH wireless-drivers] mt76x0: Remove VLA usage
From: Kalle Valo @ 2018-08-08 4:53 UTC (permalink / raw)
To: Kees Cook
Cc: Stanislaw Gruszka, David S. Miller, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20180807225040.GA2164@beast>
Kees Cook <keescook@chromium.org> writes:
> Even with "const" variables, the compiler will generate warnings about
> VLA usage. In the quest to remove all VLAs from the kernel[1], this uses
> a #define instead of a const to do the array sizing.
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>
> Fixes: e87b5039511a ("mt76x0: eeprom files")
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> Please include this for the v4.19 merge window. The VLA was introduced
> with the new source file (which I also note is missing a SPDX line), so
> I'd like to avoid the kernel ever getting released with a VLA here.
Ok, I'll push this to v4.19.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next 13/14] net: core: add new/replace rate estimator lock parameter
From: Marcelo Ricardo Leitner @ 2018-08-08 1:37 UTC (permalink / raw)
To: Vlad Buslov
Cc: netdev, davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast,
daniel, edumazet, keescook
In-Reply-To: <1533538465-23199-14-git-send-email-vladbu@mellanox.com>
On Mon, Aug 06, 2018 at 09:54:24AM +0300, Vlad Buslov wrote:
> Extend rate estimator 'new' and 'replace' APIs with additional spinlock
> parameter to be used by rtnl-unlocked actions to protect rate_est pointer
> from concurrent modification.
I'm wondering if this additional parameter is really needed. So far,
the only case in which it is not NULL, the same lock that is used to
protect the stats is also used in this new parameter.
...
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -138,7 +138,7 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
>
> if (est) {
> err = gen_replace_estimator(&police->tcf_bstats, NULL,
> - &police->tcf_rate_est,
> + &police->tcf_rate_est, NULL,
> &police->tcf_lock,
> NULL, est);
Which is here, and this new NULL arg is replaced by &police->tcf_lock
in the next patch.
Do you foresee a case in which a different lock will be used?
Or maybe it is because the current one is explicitly aimed towards the
stats?
Marcelo
^ permalink raw reply
* [PATCH net] vhost: reset metadata cache when initializing new IOTLB
From: Jason Wang @ 2018-08-08 3:43 UTC (permalink / raw)
To: mst, jasowang; +Cc: netdev, linux-kernel, kvm, virtualization
We need to reset metadata cache during new IOTLB initialization,
otherwise the stale pointers to previous IOTLB may be still accessed
which will lead a use after free.
Reported-by: syzbot+c51e6736a1bf614b3272@syzkaller.appspotmail.com
Fixes: f88949138058 ("vhost: introduce O(1) vq metadata cache")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index a502f1a..ed31145 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1560,9 +1560,12 @@ int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled)
d->iotlb = niotlb;
for (i = 0; i < d->nvqs; ++i) {
- mutex_lock(&d->vqs[i]->mutex);
- d->vqs[i]->iotlb = niotlb;
- mutex_unlock(&d->vqs[i]->mutex);
+ struct vhost_virtqueue *vq = d->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ vq->iotlb = niotlb;
+ __vhost_vq_meta_reset(vq);
+ mutex_unlock(&vq->mutex);
}
vhost_umem_clean(oiotlb);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v6 10/11] net: sched: atomically check-allocate action
From: Cong Wang @ 2018-08-08 1:20 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <1530800673-12280-11-git-send-email-vladbu@mellanox.com>
On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement function that atomically checks if action exists and either takes
> reference to it, or allocates idr slot for action index to prevent
> concurrent allocations of actions with same index. Use EBUSY error pointer
> to indicate that idr slot is reserved.
A dumb question:
How could "concurrent allocations of actions with same index" happen
as you already take idrinfo->lock for the whole tcf_idr_check_alloc()??
For me, it should be only one allocation could succeed, all others
should fail.
Maybe you are trying to prevent others treat it like existing one,
but in that case you can just hold the idinfo->lock for all idr operations.
And more importantly, upper layer is able to tell it is a creation or
just replace, you don't have to check this in this complicated way.
IOW, all of these complicated code should not exist.
^ permalink raw reply
* Re: [PATCH v8 bpf-next 02/10] veth: Add driver XDP
From: Toshiaki Makita @ 2018-08-08 0:53 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, netdev, Jesper Dangaard Brouer,
Jakub Kicinski, John Fastabend
In-Reply-To: <c84a15fe-0863-067c-417d-f20ce5e9e9bc@iogearbox.net>
Hi Daniel,
Thank you for taking a look!
On 2018/08/07 23:26, Daniel Borkmann wrote:
> On 08/03/2018 09:58 AM, Toshiaki Makita wrote:
> [...]
>> +
>> +static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
>> + struct sk_buff *skb)
>> +{
>> + u32 pktlen, headroom, act, metalen;
>> + void *orig_data, *orig_data_end;
>> + struct bpf_prog *xdp_prog;
>> + int mac_len, delta, off;
>> + struct xdp_buff xdp;
>> +
>> + rcu_read_lock();
>> + xdp_prog = rcu_dereference(priv->xdp_prog);
>> + if (unlikely(!xdp_prog)) {
>> + rcu_read_unlock();
>> + goto out;
>> + }
>> +
>> + mac_len = skb->data - skb_mac_header(skb);
>> + pktlen = skb->len + mac_len;
>> + headroom = skb_headroom(skb) - mac_len;
>> +
>> + if (skb_shared(skb) || skb_head_is_locked(skb) ||
>> + skb_is_nonlinear(skb) || headroom < XDP_PACKET_HEADROOM) {
>
> Hmm, I think this is not fully correct. What happens if you have cloned skbs as
> e.g. the case with TCP? This would also need a full expensive unclone to make the
> data private as expected by XDP (this is basically a similar issue in generic
> XDP).
Well, cloned is checked in skb_head_is_locked() so TCP packets are
always uncloned here.
> It may potentially be worth to also share the code here with generic XDP
> implementation given it's quite similar?
For now I'm not sharing the code because of two reasons.
One is that as you say generic XDP skips cloned packets. I traced the
reason and it seems it is to skip packets redirected by act_mirred.
https://patchwork.ozlabs.org/patch/750127/
The assumption that no one provides cloned skbs other than mirred breaks
when generic XDP added support for virtual devices, but it is still
valid that we should skip packets redirected by mirred if we want to be
in line with driver XDP. So I'm thinking generic XDP needs something
more than just uncloning cloned skbs.
The other reason is performance for REDIRECT. We can make use of bulk
redirection in driver XDP, but it requires xdp_frames which requires
non-kmallocked skb head. This is different from generic XDP which allows
kmallocked skb head and uses kmalloc if head reallocation is needed.
--
Toshiaki Makita
^ permalink raw reply
* Re: KASAN: use-after-free Read in iotlb_access_ok
From: Jason Wang @ 2018-08-08 2:52 UTC (permalink / raw)
To: syzbot, kvm, linux-kernel, mst, netdev, syzkaller-bugs,
virtualization
In-Reply-To: <000000000000eb92240572d68452@google.com>
On 2018年08月07日 19:16, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: e30cb13c5a09 Merge
> git://git.kernel.org/pub/scm/linux/kern..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=10a153e0400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
> dashboard link:
> https://syzkaller.appspot.com/bug?extid=c51e6736a1bf614b3272
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the
> commit:
> Reported-by: syzbot+c51e6736a1bf614b3272@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in vhost_vq_meta_fetch
> drivers/vhost/vhost.c:702 [inline]
> BUG: KASAN: use-after-free in iotlb_access_ok+0x5c9/0x600
> drivers/vhost/vhost.c:1177
> Read of size 8 at addr ffff880197df2fc0 by task vhost-8938/8941
>
> CPU: 0 PID: 8941 Comm: vhost-8938 Not tainted 4.18.0-rc7+ #174
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
> vhost_vq_meta_fetch drivers/vhost/vhost.c:702 [inline]
> iotlb_access_ok+0x5c9/0x600 drivers/vhost/vhost.c:1177
> vq_iotlb_prefetch+0x10e/0x230 drivers/vhost/vhost.c:1214
> handle_rx+0x247/0x1f80 drivers/vhost/net.c:799
> handle_rx_net+0x19/0x20 drivers/vhost/net.c:934
> vhost_worker+0x283/0x490 drivers/vhost/vhost.c:360
> kthread+0x345/0x410 kernel/kthread.c:246
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> Allocated by task 8938:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
> kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
> kmalloc include/linux/slab.h:513 [inline]
> vhost_new_umem_range+0xcb/0x7c0 drivers/vhost/vhost.c:911
> vhost_process_iotlb_msg drivers/vhost/vhost.c:1000 [inline]
> vhost_chr_write_iter+0xe53/0x1a00 drivers/vhost/vhost.c:1043
> vhost_net_chr_write_iter+0x59/0x70 drivers/vhost/net.c:1399
> call_write_iter include/linux/fs.h:1793 [inline]
> new_sync_write fs/read_write.c:474 [inline]
> __vfs_write+0x6c6/0x9f0 fs/read_write.c:487
> vfs_write+0x1f8/0x560 fs/read_write.c:549
> ksys_write+0x101/0x260 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 8950:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
> __cache_free mm/slab.c:3498 [inline]
> kfree+0xd9/0x260 mm/slab.c:3813
> vhost_umem_free+0x944/0x14d0 drivers/vhost/vhost.c:576
> vhost_umem_clean+0x83/0xf0 drivers/vhost/vhost.c:588
> vhost_init_device_iotlb+0x1d7/0x290 drivers/vhost/vhost.c:1568
> vhost_net_set_features drivers/vhost/net.c:1292 [inline]
> vhost_net_ioctl+0xff3/0x1a80 drivers/vhost/net.c:1357
> vfs_ioctl fs/ioctl.c:46 [inline]
> file_ioctl fs/ioctl.c:500 [inline]
> do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
> ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
> __do_sys_ioctl fs/ioctl.c:708 [inline]
> __se_sys_ioctl fs/ioctl.c:706 [inline]
> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff880197df2f80
> which belongs to the cache kmalloc-96 of size 96
> The buggy address is located 64 bytes inside of
> 96-byte region [ffff880197df2f80, ffff880197df2fe0)
> The buggy address belongs to the page:
> page:ffffea00065f7c80 count:1 mapcount:0 mapping:ffff8801dac004c0
> index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea0007530148 ffffea000663b888 ffff8801dac004c0
> raw: 0000000000000000 ffff880197df2000 0000000100000020 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff880197df2e80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ffff880197df2f00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>> ffff880197df2f80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ^
> ffff880197df3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff880197df3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
Looks like the metadata IOTLB needs to be reset during set_features.
Will post a fix.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH] net: nixge: Get rid of unused struct member 'last_link'
From: Moritz Fischer @ 2018-08-07 23:35 UTC (permalink / raw)
To: davem; +Cc: keescook, netdev, linux-kernel, alex.williams, Moritz Fischer
Get rid of unused struct member 'last_link'
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
drivers/net/ethernet/ni/nixge.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 5b56a2f2ff8a..5e34905bd8d8 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -183,7 +183,6 @@ struct nixge_priv {
int tx_irq;
int rx_irq;
- u32 last_link;
/* Buffer descriptors */
struct nixge_hw_dma_bd *tx_bd_v;
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-08-08 1:51 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ingo Molnar, Thomas Gleixner, linux-arch, Eric Biggers,
Linux Crypto Mailing List, LKML, Netdev, David Miller,
Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves,
Daniel J . Bernstein, Tanja Lange, Jean-Philippe Aumasson,
Karthikeyan Bhargavan
In-Reply-To: <A44CAA94-415C-476C-9523-2D19F28EDC6D@amacapital.net>
On Tue, Aug 7, 2018 at 6:49 PM Andy Lutomirski <luto@amacapital.net> wrote:
> I really wish we had a way to see that we use asm-generic’s copy of a header in all cases except where an arch opts out.
It's really not that hard to do -- symlink asm-generic to a target
called "asm" inside an otherwise empty directory, and add that
otherwise empty directory to the -I paths just after arch/include.
Since it's searched second, it's only used if the first fails. Maybe
I'm missing something though, as this seems a bit too obvious. Perhaps
a project for another day.
^ permalink raw reply
* [PATCH net-next] ieee802154: hwsim: fix rcu handling
From: Alexander Aring @ 2018-08-07 23:32 UTC (permalink / raw)
To: netdev; +Cc: linux-wpan, kernel, Alexander Aring, Stefan Schmidt
This patch adds missing rcu_assign_pointer()/rcu_dereference() to used rcu
pointers. There was already a previous commit c5d99d2b35da ("ieee802154:
hwsim: fix rcu address annotation"), but there was more which was
pointed out on my side by using newest sparse version.
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
After I installed finally the newest sparse version I found more what kbuild
was pointed out.
I hope I did it right, not sure if I really need rcu functionality in these
case because protection by mutex but I make sparse silent.
At some places the resource isn't a shared resource at this moment.
Anyway in my case I want to use this driver to create easily multi-hop
scenarios, if somebody report things (or I hit it) which smells like some
memory in this area - I will try and help to fix it.
Newest sparse version from git is happy now and I hope kbuild bot as well.
I tested this patch with RPOVE_RCU enabled and tried to update these settings
while heavy loading in rcu protected xmit hotpath is occured, without any
issues so far.
Sorry again.
drivers/net/ieee802154/mac802154_hwsim.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index f4e92054f7df..53f39435321e 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -22,6 +22,7 @@
#include <linux/module.h>
#include <linux/timer.h>
#include <linux/platform_device.h>
+#include <linux/rtnetlink.h>
#include <linux/netdevice.h>
#include <linux/device.h>
#include <linux/spinlock.h>
@@ -110,7 +111,7 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
pib->page = page;
pib->channel = channel;
- pib_old = phy->pib;
+ pib_old = rtnl_dereference(phy->pib);
rcu_assign_pointer(phy->pib, pib);
kfree_rcu(pib_old, rcu);
return 0;
@@ -406,7 +407,7 @@ static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
}
einfo->lqi = 0xff;
- e->info = einfo;
+ rcu_assign_pointer(e->info, einfo);
e->endpoint = endpoint;
return e;
@@ -414,7 +415,13 @@ static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
static void hwsim_free_edge(struct hwsim_edge *e)
{
- kfree_rcu(e->info, rcu);
+ struct hwsim_edge_info *einfo;
+
+ rcu_read_lock();
+ einfo = rcu_dereference(e->info);
+ rcu_read_unlock();
+
+ kfree_rcu(einfo, rcu);
kfree_rcu(e, rcu);
}
@@ -796,7 +803,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
goto err_pib;
}
- phy->pib = pib;
+ rcu_assign_pointer(phy->pib, pib);
phy->idx = idx;
INIT_LIST_HEAD(&phy->edges);
@@ -829,10 +836,17 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
static void hwsim_del(struct hwsim_phy *phy)
{
+ struct hwsim_pib *pib;
+
hwsim_edge_unsubscribe_me(phy);
list_del(&phy->list);
- kfree_rcu(phy->pib, rcu);
+
+ rcu_read_lock();
+ pib = rcu_dereference(phy->pib);
+ rcu_read_unlock();
+
+ kfree_rcu(pib, rcu);
ieee802154_unregister_hw(phy->hw);
ieee802154_free_hw(phy->hw);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Andy Lutomirski @ 2018-08-08 1:48 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Ingo Molnar, Thomas Gleixner, linux-arch, Eric Biggers,
Linux Crypto Mailing List, LKML, Netdev, David Miller,
Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves,
Daniel J . Bernstein, Tanja Lange, Jean-Philippe Aumasson,
Karthikeyan Bhargavan
In-Reply-To: <CAHmME9qPay3O7Yh-R66EDhon4dJz1cuy69dWUxf3Xkt8seqLZA@mail.gmail.com>
> On Aug 7, 2018, at 4:48 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hey Andy,
>
>> On Tue, Aug 7, 2018 at 12:43 PM Andy Lutomirski <luto@amacapital.net> wrote:
>> For "zinc: add simd helper", I think it should be in include/linux,
>> and include/linux/simd.h should (immediately or maybe in the future)
>> include <asm/simd.h> to pick up arch-specific stuff. And the patch
>> should get sent to linux-arch@vger.kernel.org.
>
> I guess you saw my prompt about that in the previous commit message?
> Based on your encouragement, I implemented it:
> https://git.zx2c4.com/linux-dev/commit/?h=simd This is _far_ more
> invasive than I wanted to be, as I don't want this patch submission to
> grow unwieldy and never be merged, but I guess we can roll with this
> for now...
>
I really wish we had a way to see that we use asm-generic’s copy of a header in all cases except where an arch opts out.
>> In your blake2s_arch() implementation, you're not passing in a
>> simd_context_t. Is that still a work in progress? I thought the plan
>> was to pass it in rather than doing the check in the _arch()
>> functions.
>
> I'm inclined to do the explicit context passing only when a function
> is likely to be used in that kind of environment, and adjust as
> needed. Long term, anyway, that API will be removed once the x86 guys
> figure out lazy FPU restoration and the amortization doesn't add
> anything.
Fair enough.
>
> Jason
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c
From: Pkshih @ 2018-08-08 1:48 UTC (permalink / raw)
To: yuehaibing@huawei.com, valdis.kletnieks@vt.edu,
Larry.Finger@lwfinger.net
Cc: linux-wireless@vger.kernel.org, kvalo@codeaurora.org,
linux-kernel@vger.kernel.org, colin.king@canonical.com,
netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <577a831f-45c3-a687-ca04-edc977ddb925@lwfinger.net>
On Tue, 2018-08-07 at 09:23 -0500, Larry Finger wrote:
> On 08/06/2018 04:42 PM, valdis.kletnieks@vt.edu wrote:
> > On Mon, 06 Aug 2018 12:54:40 +0800, YueHaibing said:
> >> Fix following coccinelle warning:
> >>
> >> ./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2952:2-4: WARNING: possible
> condition with no effect (if == else)
> >
> >> /* sw mechanism */
> >> if (BTC_WIFI_BW_HT40 == wifi_bw) {
> >> - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
> >> - (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> >> - btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> - false, false);
> >> - } else {
> >> - btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> - false, false);
> >> - }
> >> + btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> + false, false);
> >> } else {
> >
> > Rather than blindly fixing this, perhaps a bit of thought needs to be
> > applied to why this code looks like this in the first place.
> >
> > See commit c6821613e653a (which looks like the bletcherous "do too many
> > things at once" commit indeed), although the actual diff appears to be a
> > "no harm, no foul" against this commit, where the issue already existed.
> >
> > commit aa45a673b291fd761275493bc15316d79555ed55
> > Author: Larry Finger <Larry.Finger@lwfinger.net>
> > Date: Fri Feb 28 15:16:43 2014 -0600
> >
> > rtlwifi: btcoexist: Add new mini driver
> >
> > Larry? Can you reach back to 2014 and remember why this code
> > looked like this in the first place?
>
> The base code came from Realtek. My only part in getting it into the kernel was
> to clean up the checkpatch and Sparse errors and warnings, and submit it. I was
> a "script kiddy" just like the authors of the current patches. The only
> difference is that I was getting drivers into the kernel so that users hardware
> would work.
>
> Any record of whether these duplicate branches are the result of incorrect copy
> and paste, or just extraneous code, would be at Realtek in their version control
> history. I have never had access to such archives, nor have I ever had an
> non-disclosure agreement with Realtek.
>
> Ping-Ke Shih, who is Cc'd on these messages, might be able to answer these
> questions.
These branches is used to improve user experience according to RSSI strength, but
it has not significant improvement so the same arguments become duplicate branches.
I check the latest code of halbtc8723b2ant.c, there are more than one statements
within if-else branch to improve performance, but the statements mentioned by
this patch are still the same. So, these duplicate branches can be safely removed.
>
> For the moment, these simplifications could be applied as long as they are
> correctly done. After all, they are not changing what is already there and that
> stops any other person that knows how to run coccinelle from submitting the same
> patch in the future. Why "kick the can down the road"? If PK can find that there
> was an error in the original code, he can submit a "Fixes" patch.
>
>
^ permalink raw reply
* [PATCH 27/33] qede: qede_fp: Mark expected switch fall-through
From: Gustavo A. R. Silva @ 2018-08-07 23:27 UTC (permalink / raw)
To: David S. Miller, netdev, linux-kernel, Gustavo A. R. Silva
Cc: Ariel Elior, everest-linux-l2
In-Reply-To: <cover.1533675546.git.gustavo@embeddedor.com>
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1384501 ("Missing break in switch")
Addresses-Coverity-ID: 1398869 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 6c70239..8c9e95b 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -1119,8 +1119,10 @@ static bool qede_rx_xdp(struct qede_dev *edev,
default:
bpf_warn_invalid_xdp_action(act);
+ /* Fall through */
case XDP_ABORTED:
trace_xdp_exception(edev->ndev, prog, act);
+ /* Fall through */
case XDP_DROP:
qede_recycle_rx_bd_ring(rxq, cqe->bd_num);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v6 11/11] net: sched: change action API to use array of pointers to actions
From: Cong Wang @ 2018-08-07 23:26 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik
In-Reply-To: <1530800673-12280-12-git-send-email-vladbu@mellanox.com>
On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> attr_size = tcf_action_full_attrs_size(attr_size);
>
> if (event == RTM_GETACTION)
> - ret = tcf_get_notify(net, portid, n, &actions, event, extack);
> + ret = tcf_get_notify(net, portid, n, actions, event, extack);
> else { /* delete */
> - ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
> + ret = tcf_del_notify(net, n, actions, &acts_deleted, portid,
> + attr_size, extack);
> if (ret)
> goto err;
> return ret;
> }
> err:
> - tcf_action_put_lst(&actions);
> + tcf_action_put_many(&actions[acts_deleted]);
> return ret;
How does this even work?
You save an index in 'acts_deleted', but you pass &actions[acts_deleted]
to tcf_action_put_many(), which seems you want to start from
where it fails, but inside tcf_action_put_many() it starts from 0
to TCA_ACT_MAX_PRIO, out-of-bound access at least?
^ permalink raw reply
* Re: [PATCH bpf-next 12/13] docs: net: Fix various minor typos
From: Tobin C. Harding @ 2018-08-08 1:42 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Jonathan Corbet, David S. Miller, linux-doc,
netdev, linux-kernel
In-Reply-To: <76cf11bc-61b3-8068-5546-79d3ff3a58e9@iogearbox.net>
On Fri, Aug 03, 2018 at 10:41:12AM +0200, Daniel Borkmann wrote:
> On 08/01/2018 07:09 AM, Tobin C. Harding wrote:
> > There are a few minor typos and grammatical issues. We should however
> > try to keep the current flavour of the document.
> >
> > Fix typos and grammar if glaringly required.
> >
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
>
> Overall looks good, just some minor nits:
>
> > Documentation/networking/filter.rst | 65 +++++++++++++++--------------
> > 1 file changed, 33 insertions(+), 32 deletions(-)
> >
> > diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
> > index 99dfa74fc4f7..b989a6c882b8 100644
> > --- a/Documentation/networking/filter.rst
> > +++ b/Documentation/networking/filter.rst
> > @@ -32,10 +32,10 @@ removing the old one and placing your new one in its place, assuming your
> > filter has passed the checks, otherwise if it fails the old filter will
> > remain on that socket.
> >
> > -SO_LOCK_FILTER option allows to lock the filter attached to a socket. Once
> > -set, a filter cannot be removed or changed. This allows one process to
> > +SO_LOCK_FILTER option allows locking of the filter attached to a socket.
> > +Once set, a filter cannot be removed or changed. This allows one process to
> > setup a socket, attach a filter, lock it then drop privileges and be
> > -assured that the filter will be kept until the socket is closed.
> > +assured that the filter will be kept until the socket is closed.
>
> ^-- looks like extra whitespace slipped in?
>
> > The biggest user of this construct might be libpcap. Issuing a high-level
> > filter command like ``tcpdump -i em1 port 22`` passes through the libpcap
> > @@ -470,7 +470,7 @@ JIT compiler
> > ============
> >
> > The Linux kernel has a built-in BPF JIT compiler for x86_64, SPARC, PowerPC,
> > -ARM, ARM64, MIPS and s390 and can be enabled through CONFIG_BPF_JIT. The JIT
> > +ARM, ARM64, MIPS and s390 which can be enabled through CONFIG_BPF_JIT. The JIT
> > compiler is transparently invoked for each attached filter from user space
> > or for internal kernel users if it has been previously enabled by root::
> >
> > @@ -580,7 +580,7 @@ Internally, for the kernel interpreter, a different instruction set
> > format with similar underlying principles from BPF described in previous
> > paragraphs is being used. However, the instruction set format is modelled
> > closer to the underlying architecture to mimic native instruction sets, so
> > -that a better performance can be achieved (more details later). This new
> > +that better performance can be achieved (more details later). This new
> > ISA is called 'eBPF' or 'internal BPF' interchangeably. (Note: eBPF which
> > originates from [e]xtended BPF is not the same as BPF extensions! While
> > eBPF is an ISA, BPF extensions date back to classic BPF's 'overloading'
> > @@ -655,12 +655,12 @@ Some core changes of the new internal format:
> >
> > 32-bit architectures run 64-bit internal BPF programs via interpreter.
> > Their JITs may convert BPF programs that only use 32-bit subregisters into
> > - native instruction set and let the rest being interpreted.
> > + native instruction set and let the rest be interpreted.
> >
> > - Operation is 64-bit, because on 64-bit architectures, pointers are also
> > - 64-bit wide, and we want to pass 64-bit values in/out of kernel functions,
> > - so 32-bit eBPF registers would otherwise require to define register-pair
> > - ABI, thus, there won't be able to use a direct eBPF register to HW register
> > + Operation is 64-bit since on 64-bit architectures pointers are also
> > + 64-bit wide and we want to pass 64-bit values in/out of kernel functions.
> > + 32-bit eBPF registers would otherwise require us to define a register-pair
> > + ABI, thus we would not be able to use a direct eBPF register to HW register
> > mapping and JIT would need to do combine/split/move operations for every
> > register in and out of the function, which is complex, bug prone and slow.
> > Another reason is the use of atomic 64-bit counters.
> > @@ -694,7 +694,7 @@ Some core changes of the new internal format:
> > situations without performance penalty.
> >
> > After an in-kernel function call, R1 - R5 are reset to unreadable and R0 has
> > - a return value of the function. Since R6 - R9 are callee saved, their state
> > + the return value of the function. Since R6 - R9 are callee saved, their state
> > is preserved across the call.
> >
> > For example, consider three C functions::
> > @@ -732,7 +732,7 @@ Some core changes of the new internal format:
> > are currently not supported, but these restrictions can be lifted if necessary
> > in the future.
> >
> > - On 64-bit architectures all register map to HW registers one to one. For
> > + On 64-bit architectures all registers map to HW registers one to one. For
> > example, x86_64 JIT compiler can map them as ... ::
> >
> > R0 - rax
> > @@ -831,9 +831,10 @@ A program, that is translated internally consists of the following elements::
> >
> > op:16, jt:8, jf:8, k:32 ==> op:8, dst_reg:4, src_reg:4, off:16, imm:32
> >
> > -So far 87 internal BPF instructions were implemented. 8-bit ``op`` opcode field
> > -has room for new instructions. Some of them may use 16/24/32 byte encoding. New
> > -instructions must be multiple of 8 bytes to preserve backward compatibility.
> > +So far 87 internal BPF instructions have been implemented. 8-bit ``op``
> > +opcode field has room for new instructions. Some of them may use 16/24/32
> > +byte encoding. New instructions must be a multiple of 8 bytes to preserve
> > +backward compatibility.
> >
> > Internal BPF is a general purpose RISC instruction set. Not every register and
> > every instruction are used during translation from original BPF to new format.
> > @@ -844,11 +845,11 @@ out of registers and would have to resort to spill/fill to stack.
> >
> > Internal BPF can used as generic assembler for last step performance
> > optimizations, socket filters and seccomp are using it as assembler. Tracing
> > -filters may use it as assembler to generate code from kernel. In kernel usage
> > +filters may use it as assembler to generate code from kernel. In-kernel usage
>
> ^-- ditto
Hi Daniel,
Thanks for doing such a careful review that you noticed this. I'm
working on this more ATM and I've moved the document to use double
spaces between _all_ full stops. Currently the document uses mostly
single spaces but there are some sections with double space. The
internet tells me this is a 'style' issue not a rule. And I've seen
single and double spacing in tree and do not know if one is favoured.
Do you care? If you do not care I'll just use double spaces. If you do
care and would prefer me to uniformly use a single space I can do that
also. Oh, and FTR filter.txt looks like its going into userspace-api/
so you may care even less with that move.
If this is overly pedantic just tell me to go away :)
thanks,
Tobin.
^ permalink raw reply
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