* [PATCH net] net: openvswitch: fix possible memleak on create vport fails
From: xiangxia.m.yue @ 2019-09-17 15:40 UTC (permalink / raw)
To: gvrose8192, pshelar; +Cc: netdev, Tonghao Zhang, Taehee Yoo
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
If we register a net device which name is not valid
(dev_get_valid_name), register_netdevice will return err
codes and will not run dev->priv_destructor. The memory
will leak. This patch adds check in ovs_vport_free and
set the vport NULL.
Fixes: 309b66970ee2 ("net: openvswitch: do not free vport if register_netdevice() is failed.")
Cc: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
net/openvswitch/vport-internal_dev.c | 8 ++------
net/openvswitch/vport.c | 9 +++++++++
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index d2437b5..074c43f 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -159,7 +159,6 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
struct internal_dev *internal_dev;
struct net_device *dev;
int err;
- bool free_vport = true;
vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
if (IS_ERR(vport)) {
@@ -190,10 +189,8 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
rtnl_lock();
err = register_netdevice(vport->dev);
- if (err) {
- free_vport = false;
+ if (err)
goto error_unlock;
- }
dev_set_promiscuity(vport->dev, 1);
rtnl_unlock();
@@ -207,8 +204,7 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
error_free_netdev:
free_netdev(dev);
error_free_vport:
- if (free_vport)
- ovs_vport_free(vport);
+ ovs_vport_free(vport);
error:
return ERR_PTR(err);
}
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 3fc38d1..281259a 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -157,11 +157,20 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
*/
void ovs_vport_free(struct vport *vport)
{
+ /* We should check whether vport is NULL.
+ * We may free it again, for example in internal_dev_create
+ * if register_netdevice fails, vport may have been freed via
+ * internal_dev_destructor.
+ */
+ if (unlikely(!vport))
+ return;
+
/* vport is freed from RCU callback or error path, Therefore
* it is safe to use raw dereference.
*/
kfree(rcu_dereference_raw(vport->upcall_portids));
kfree(vport);
+ vport = NULL;
}
EXPORT_SYMBOL_GPL(ovs_vport_free);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: dsa: Use the correct style for SPDX License Identifier
From: Vivien Didelot @ 2019-09-21 16:01 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Hauke Mehrtens, Florian Fainelli, Andrew Lunn, David S. Miller,
Greg Kroah-Hartman, Joe Perches, Uwe Kleine-König, netdev,
linux-kernel
In-Reply-To: <20190921134522.GA3575@nishad>
On Sat, 21 Sep 2019 19:15:25 +0530, Nishad Kamdar <nishadkamdar@gmail.com> wrote:
> This patch corrects the SPDX License Identifier style
> in header file for Distributed Switch Architecture drivers.
> For C header files Documentation/process/license-rules.rst
> mandates C-like comments (opposed to C source files where
> C++ style should be used)
>
> Changes made by using a script provided by Joe Perches here:
> https://lkml.org/lkml/2019/2/7/46.
>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH] net: dsa: b53: Use the correct style for SPDX License Identifier
From: Vivien Didelot @ 2019-09-21 16:01 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Florian Fainelli, Andrew Lunn, David S. Miller,
Greg Kroah-Hartman, Joe Perches, Uwe Kleine-König, netdev,
linux-kernel
In-Reply-To: <20190921133011.GA2994@nishad>
On Sat, 21 Sep 2019 19:00:16 +0530, Nishad Kamdar <nishadkamdar@gmail.com> wrote:
> This patch corrects the SPDX License Identifier style
> in header file for Broadcom BCM53xx managed switch driver.
> For C header files Documentation/process/license-rules.rst
> mandates C-like comments (opposed to C source files where
> C++ style should be used)
>
> Changes made by using a script provided by Joe Perches here:
> https://lkml.org/lkml/2019/2/7/46.
>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: INFO: task hung in p9_fd_close
From: syzbot @ 2019-09-21 16:19 UTC (permalink / raw)
To: asmadeus, davem, ericvh, linux-kernel, lucho, netdev,
syzkaller-bugs, v9fs-developer
In-Reply-To: <00000000000039af4d05915a9f56@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: f97c81dc Merge tag 'armsoc-late' of git://git.kernel.org/p..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=171a1555600000
kernel config: https://syzkaller.appspot.com/x/.config?x=61f948934213449f
dashboard link: https://syzkaller.appspot.com/bug?extid=8b41a1365f1106fd0f33
compiler: clang version 9.0.0 (/home/glider/llvm/clang
80fee25776c2fb61e74c1ecb1a523375c2500b69)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1641d429600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11eff9ad600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com
INFO: task syz-executor635:8085 blocked for more than 143 seconds.
Not tainted 5.3.0+ #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor635 D27848 8085 8070 0x00004004
Call Trace:
context_switch kernel/sched/core.c:3384 [inline]
__schedule+0x82e/0xc50 kernel/sched/core.c:4056
schedule+0x131/0x1e0 kernel/sched/core.c:4123
schedule_timeout+0x46/0x240 kernel/time/timer.c:1869
do_wait_for_common+0x2e7/0x4d0 kernel/sched/completion.c:83
__wait_for_common kernel/sched/completion.c:104 [inline]
wait_for_common kernel/sched/completion.c:115 [inline]
wait_for_completion+0x47/0x60 kernel/sched/completion.c:136
__flush_work+0xd4/0x150 kernel/workqueue.c:3040
__cancel_work_timer+0x420/0x570 kernel/workqueue.c:3127
cancel_work_sync+0x17/0x20 kernel/workqueue.c:3163
p9_conn_destroy net/9p/trans_fd.c:868 [inline]
p9_fd_close+0x297/0x3c0 net/9p/trans_fd.c:898
p9_client_create+0x974/0xee0 net/9p/client.c:1068
v9fs_session_init+0x192/0x18e0 fs/9p/v9fs.c:406
v9fs_mount+0x82/0x860 fs/9p/vfs_super.c:124
legacy_get_tree+0xf9/0x1a0 fs/fs_context.c:659
vfs_get_tree+0x8f/0x380 fs/super.c:1542
do_new_mount fs/namespace.c:2825 [inline]
do_mount+0x16c7/0x2510 fs/namespace.c:3145
ksys_mount+0xcc/0x100 fs/namespace.c:3354
__do_sys_mount fs/namespace.c:3368 [inline]
__se_sys_mount fs/namespace.c:3365 [inline]
__x64_sys_mount+0xbf/0xd0 fs/namespace.c:3365
do_syscall_64+0xf7/0x1c0 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447909
Code: e8 5c 14 03 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 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 9b 0c fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007eff1aea0db8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00000000006ddc78 RCX: 0000000000447909
RDX: 00000000200005c0 RSI: 0000000020000540 RDI: 0000000000000000
RBP: 00000000006ddc70 R08: 0000000020000680 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006ddc7c
R13: 00007ffc4e6377ef R14: 00007eff1aea19c0 R15: 000000000000002d
Showing all locks held in the system:
2 locks held by kworker/1:1/22:
#0: ffff8880aa4278e8 ((wq_completion)events){+.+.}, at: spin_unlock_irq
include/linux/spinlock.h:388 [inline]
#0: ffff8880aa4278e8 ((wq_completion)events){+.+.}, at:
process_one_work+0x75d/0x10e0 kernel/workqueue.c:2242
#1: ffff8880a9a3fd78 ((work_completion)(&m->wq)){+.+.}, at:
process_one_work+0x79f/0x10e0 kernel/workqueue.c:2244
1 lock held by khungtaskd/1053:
#0: ffffffff888d3900 (rcu_read_lock){....}, at: rcu_lock_acquire+0x4/0x30
include/linux/rcupdate.h:207
1 lock held by rsyslogd/7959:
#0: ffff8880a99f1e20 (&f->f_pos_lock){+.+.}, at: __fdget_pos+0x243/0x2e0
fs/file.c:801
2 locks held by getty/8049:
#0: ffff8880a602d450 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f212e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8050:
#0: ffff8880a902ac50 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f092e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8051:
#0: ffff88809c0f1750 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f312e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8052:
#0: ffff88809e344b90 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f152e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8053:
#0: ffff88809e344310 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f1d2e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8054:
#0: ffff88809c8aa410 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f2d2e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
2 locks held by getty/8055:
#0: ffff8880a0267310 (&tty->ldisc_sem){++++}, at:
tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:272
#1: ffffc90005f012e0 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x221/0x1b00 drivers/tty/n_tty.c:2156
=============================================
NMI backtrace for cpu 0
CPU: 0 PID: 1053 Comm: khungtaskd Not tainted 5.3.0+ #0
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+0x1d8/0x2f8 lib/dump_stack.c:113
nmi_cpu_backtrace+0xaf/0x1a0 lib/nmi_backtrace.c:101
nmi_trigger_cpumask_backtrace+0x174/0x290 lib/nmi_backtrace.c:62
arch_trigger_cpumask_backtrace+0x10/0x20 arch/x86/kernel/apic/hw_nmi.c:38
trigger_all_cpu_backtrace+0x17/0x20 include/linux/nmi.h:146
check_hung_uninterruptible_tasks kernel/hung_task.c:205 [inline]
watchdog+0xbb9/0xbd0 kernel/hung_task.c:289
kthread+0x332/0x350 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.3.0+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:check_preemption_disabled+0x6/0x2a0 lib/smp_processor_id.c:13
Code: 90 90 90 90 55 48 89 e5 e8 a7 b2 2e fe 48 c7 c7 79 91 38 88 48 c7 c6
10 72 4e 88 e8 04 00 00 00 5d c3 66 90 55 48 89 e5 41 57 <41> 56 41 55 41
54 53 48 83 ec 10 49 89 f6 49 89 ff e8 74 b2 2e fe
RSP: 0018:ffff8880aeb09ee0 EFLAGS: 00000093
RAX: ffffffff83448fc9 RBX: 0000000000000004 RCX: ffff8880a98c2340
RDX: 0000000000000000 RSI: ffffffff884e7210 RDI: ffffffff88389179
RBP: ffff8880aeb09ee8 R08: ffffffff816701e0 R09: fffffbfff117c3cd
R10: fffffbfff117c3cd R11: 0000000000000000 R12: 1ffff11015d64eaf
R13: ffff8880aeb00000 R14: dffffc0000000000 R15: ffff8880aeb2757c
FS: 0000000000000000(0000) GS:ffff8880aeb00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffff600400 CR3: 00000000a3d38000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
debug_smp_processor_id+0x1c/0x20 lib/smp_processor_id.c:57
tick_nohz_stop_idle kernel/time/tick-sched.c:535 [inline]
tick_nohz_irq_enter kernel/time/tick-sched.c:1255 [inline]
tick_irq_enter+0xbd/0x3e0 kernel/time/tick-sched.c:1274
irq_enter+0x52/0xc0 kernel/softirq.c:354
scheduler_ipi+0xb3/0x4a0 kernel/sched/core.c:2337
smp_reschedule_interrupt+0x7a/0xa0 arch/x86/kernel/smp.c:244
reschedule_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:854
</IRQ>
RIP: 0010:native_safe_halt+0xe/0x10 arch/x86/include/asm/irqflags.h:61
Code: 3c fa eb ae 89 d9 80 e1 07 80 c1 03 38 c1 7c ba 48 89 df e8 94 ab 3c
fa eb b0 90 90 e9 07 00 00 00 0f 00 2d b6 99 52 00 fb f4 <c3> 90 e9 07 00
00 00 0f 00 2d a6 99 52 00 f4 c3 90 90 55 48 89 e5
RSP: 0018:ffff8880a98cfe10 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff02
RAX: 1ffffffff1115179 RBX: ffff8880a98c2340 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: ffffffff812ba81a RDI: ffff8880a98c2b80
RBP: ffff8880a98cfe18 R08: ffff8880a98c2b98 R09: ffffed1015318469
R10: ffffed1015318469 R11: 0000000000000000 R12: dffffc0000000000
R13: 1ffff11015318468 R14: dffffc0000000000 R15: 1ffffffff1115177
arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:571
default_idle_call+0x59/0xa0 kernel/sched/idle.c:94
cpuidle_idle_call kernel/sched/idle.c:154 [inline]
do_idle+0x140/0x670 kernel/sched/idle.c:264
cpu_startup_entry+0x25/0x30 kernel/sched/idle.c:356
start_secondary+0x384/0x410 arch/x86/kernel/smpboot.c:264
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:241
^ permalink raw reply
* [next] regression: WiFi doesn't work after "nl80211: Add support for EDMG channels"
From: Dmitry Osipenko @ 2019-09-21 17:12 UTC (permalink / raw)
To: Johannes Berg, Alexei Avshalom Lazar, Kalle Valo; +Cc: linux-wireless, netdev
Hello,
Patch "nl80211: Add support for EDMG channels" broke brcmfmac driver on one of my devices, the newly
introduced cfg80211_chandef_is_edmg() erroneously returns "true" because chandef->edmg.bw_config !=
NULL. Apparently this happens because "chandef" is allocated on stack in nl80211.c (and other places),
the "edmg" field isn't zeroed while it should be. Please fix, thanks in advance.
WARNING: CPU: 0 PID: 409 at net/wireless/nl80211.c:3118 nl80211_send_chandef+0xd3/0xd8
Modules linked in:
CPU: 0 PID: 409 Comm: NetworkManager Tainted: G W
5.3.0-next-20190920-00177-g89b36954c24f #2381
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
(show_stack+0x11/0x14)
(dump_stack+0x7f/0x8c)
(__warn+0xc1/0xc4)
(warn_slowpath_fmt+0x45/0x78)
(nl80211_send_chandef+0xd3/0xd8)
(nl80211_send_iface+0x4db/0x6a0)
(nl80211_get_interface+0x39/0x68)
(genl_rcv_msg+0x14d/0x33c)
(netlink_rcv_skb+0x85/0xb8)
(genl_rcv+0x21/0x30)
(netlink_unicast+0xf3/0x144)
(netlink_sendmsg+0x12b/0x27c)
(sock_sendmsg+0x11/0x1c)
(___sys_sendmsg+0x1bb/0x1d0)
(__sys_sendmsg+0x39/0x58)
(ret_fast_syscall+0x1/0x28)
Exception stack(0xe1621fa8 to 0xe1621ff0)
1fa0: 00000000 bea1b830 00000009 bea1b830 00000000 00000000
1fc0: 00000000 bea1b830 00000009 00000128 00644084 b6fd6930 00683d30 00645330
1fe0: 00000128 bea1b808 b6a1b3a5 b6a1c6b6
^ permalink raw reply
* Re: KASAN: use-after-free Read in rxrpc_release_call
From: syzbot @ 2019-09-21 17:41 UTC (permalink / raw)
To: MAILER_DAEMON, davem, dhowells, hdanton, linux-afs, linux-kernel,
netdev, syzkaller-bugs
In-Reply-To: <000000000000727bd10590c9cf6c@google.com>
syzbot has bisected this bug to:
commit 2baec2c3f854d1f79c7bb28386484e144e864a14
Author: David Howells <dhowells@redhat.com>
Date: Wed May 24 16:02:32 2017 +0000
rxrpc: Support network namespacing
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16240b09600000
start commit: f97c81dc Merge tag 'armsoc-late' of git://git.kernel.org/p..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=15240b09600000
console output: https://syzkaller.appspot.com/x/log.txt?x=11240b09600000
kernel config: https://syzkaller.appspot.com/x/.config?x=61f948934213449f
dashboard link: https://syzkaller.appspot.com/bug?extid=eed305768ece6682bb7f
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16cf8ea1600000
Reported-by: syzbot+eed305768ece6682bb7f@syzkaller.appspotmail.com
Fixes: 2baec2c3f854 ("rxrpc: Support network namespacing")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* [PATCH] net: dsa: microchip: Always set regmap stride to 1
From: Marek Vasut @ 2019-09-21 17:53 UTC (permalink / raw)
To: netdev
Cc: Marek Vasut, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Tristram Ha, Vivien Didelot, Woojung Huh
The regmap stride is set to 1 for regmap describing 8bit registers already.
However, for 16/32/64bit registers, the stride is 2/4/8 respectively. This
is not correct, as the switch protocol supports unaligned register reads
and writes and the KSZ87xx even uses such unaligned register accesses to
read e.g. MIB counter.
This patch fixes MIB counter access on KSZ87xx.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
---
drivers/net/dsa/microchip/ksz_common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a24d8e61fbe7..dd60d0837fc6 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -303,7 +303,7 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
{ \
.name = #width, \
.val_bits = (width), \
- .reg_stride = (width) / 8, \
+ .reg_stride = 1, \
.reg_bits = (regbits) + (regalign), \
.pad_bits = (regpad), \
.max_register = BIT(regbits) - 1, \
--
2.23.0.rc1
^ permalink raw reply related
* Re: [PATCH] dt-bindings: net: dwmac: fix 'mac-mode' type
From: Jakub Kicinski @ 2019-09-21 18:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: Alexandru Ardelean, netdev, devicetree, linux-kernel, davem,
robh+dt, peppe.cavallaro, alexandre.torgue, andrew
In-Reply-To: <f189cdbc-b399-7700-a39a-ba185df4af49@gmail.com>
On Fri, 20 Sep 2019 20:02:58 -0700, Florian Fainelli wrote:
> On 9/20/2019 6:11 PM, Jakub Kicinski wrote:
> > On Tue, 17 Sep 2019 13:30:52 +0300, Alexandru Ardelean wrote:
> >> The 'mac-mode' property is similar to 'phy-mode' and 'phy-connection-type',
> >> which are enums of mode strings.
> >>
> >> The 'dwmac' driver supports almost all modes declared in the 'phy-mode'
> >> enum (except for 1 or 2). But in general, there may be a case where
> >> 'mac-mode' becomes more generic and is moved as part of phylib or netdev.
> >>
> >> In any case, the 'mac-mode' field should be made an enum, and it also makes
> >> sense to just reference the 'phy-connection-type' from
> >> 'ethernet-controller.yaml'. That will also make it more future-proof for new
> >> modes.
> >>
> >> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> >
> > Applied, thank you!
> >
> > FWIW I had to add the Fixes tag by hand, either ozlabs patchwork or my
> > git-pw doesn't have the automagic handling there, yet.
>
> AFAICT the ozlabs patchwork instance does not do it, but if you have
> patchwork administrative rights (the jango administration panel I mean)
> then it is simple to add the regular expression to the list of tags that
> patchwork already recognized. Had tried getting that included by
> default, but it also counted all of those tags and therefore was not
> particularly fine grained:
>
> https://lists.ozlabs.org/pipermail/patchwork/2017-January/003910.html
Curious, it did seem to have counted the Fixes in the 'F' field on the
web UI but git-pw didn't pull it down 🤔
linux$ git checkout 92974a1d006ad8b30d53047c70974c9e065eb7df
Note: checking out '92974a1d006ad8b30d53047c70974c9e065eb7df'.
[...]
linux$ git pw patch apply 1163199 --signoff
11:41 linux$ git show
commit ac964661384b93ff3c9839c6d56f293195d54b4e (HEAD)
Author: Alexandru Ardelean <alexandru.ardelean@analog.com>
Date: Tue Sep 17 13:30:52 2019 +0300
dt-bindings: net: dwmac: fix 'mac-mode' type
The 'mac-mode' property is similar to 'phy-mode' and 'phy-connection-type',
which are enums of mode strings.
The 'dwmac' driver supports almost all modes declared in the 'phy-mode'
enum (except for 1 or 2). But in general, there may be a case where
'mac-mode' becomes more generic and is moved as part of phylib or netdev.
In any case, the 'mac-mode' field should be made an enum, and it also makes
sense to just reference the 'phy-connection-type' from
'ethernet-controller.yaml'. That will also make it more future-proof for new
modes.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
11:41 linux$
^ permalink raw reply
* Re: [PATCH net,stable] usbnet: ignore endpoints with invalid wMaxPacketSize
From: Jakub Kicinski @ 2019-09-21 19:30 UTC (permalink / raw)
To: Bjørn Mork; +Cc: netdev, linux-usb, Oliver Neukum
In-Reply-To: <87h855g68k.fsf@miraculix.mork.no>
On Sat, 21 Sep 2019 14:54:03 +0200, Bjørn Mork wrote:
> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> > On Wed, 18 Sep 2019 14:17:38 +0200, Bjørn Mork wrote:
> >> Endpoints with zero wMaxPacketSize are not usable for transferring
> >> data. Ignore such endpoints when looking for valid in, out and
> >> status pipes, to make the drivers more robust against invalid and
> >> meaningless descriptors.
> >>
> >> The wMaxPacketSize of these endpoints are used for memory allocations
> >> and as divisors in many usbnet minidrivers. Avoiding zero is therefore
> >> critical.
> >>
> >> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> >
> > Fixes tag would be useful. I'm not sure how far into stable we should
> > backport this.
>
> That would be commit 1da177e4c3f4 ("Linux-2.6.12-rc2"), so I don't think
> a Fixes tag is very useful...
It's slightly useful to add it anyway, IMHO, even if it's 2.6.12,
because it may save people processing the patch checking how far
it applies. You already did the research, anyway.
Granted, that's a little `process-centric`, rather than `merit-centric`
view.
> I haven't verified how deep into the code you have been able to get with
> wMaxPacketSize being zero. But I don't think there ever has been much
> protection since it's so obviously "insane". There was no point in
> protecting against this as long as we considered the USB port a security
> barrier.
>
> I see that the v2.6.12-rc2 version of drivers/usb/net/usbnet.c (sic)
> already had this in it's genelink_tx_fixup():
<snip>
Thanks for the detailed analysis!
> So, to summarize: I believe the fix is valid for all stable versions.
>
> I'll leave it up to the more competent stable maintainers to decide how
> many, if any, it should be backported to. I will not cry if the answer
> is none.
Right, I'll put it in the stable queue, we'll see if it passes Dave's
and Greg's filters :)
> > Is this something that occurs on real devices or protection from
> > malicious ones?
>
> Only malicious ones AFAICS.
>
> I don't necessarily agree, but I believe the current policy makes this a
> "security" issue. CVEs have previously been allocated for similar
> crashes triggered by buggy USB descriptors. For some reason we are
> supposed to protect the system against *some* types of malicious
> hardware.
I see, the patch is fairly intrusive and very unlikely to cause to lead
to regressions on real devices, so regardless of the practical impact
shouldn't hurt.
> I am looking forward to the fixes coming up next to protect against
> malicious CPUs and microcode ;-)
I hope not before we retire..
Applied, queued for stable, thank you!
^ permalink raw reply
* Re: [PATCH net,stable] cdc_ncm: fix divide-by-zero caused by invalid wMaxPacketSize
From: Jakub Kicinski @ 2019-09-21 19:30 UTC (permalink / raw)
To: Bjørn Mork
Cc: netdev, linux-usb, Oliver Neukum, syzbot+ce366e2b8296e25d84f5
In-Reply-To: <20190918120147.4520-1-bjorn@mork.no>
On Wed, 18 Sep 2019 14:01:46 +0200, Bjørn Mork wrote:
> Endpoints with zero wMaxPacketSize are not usable for transferring
> data. Ignore such endpoints when looking for valid in, out and
> status pipes, to make the driver more robust against invalid and
> meaningless descriptors.
>
> The wMaxPacketSize of the out pipe is used as divisor. So this change
> fixes a divide-by-zero bug.
>
> Reported-by: syzbot+ce366e2b8296e25d84f5@syzkaller.appspotmail.com
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
Applied, queued, thank you!
^ permalink raw reply
* Re: [PATCH] net: dsa: b53: Use the correct style for SPDX License Identifier
From: Florian Fainelli @ 2019-09-21 19:39 UTC (permalink / raw)
To: Nishad Kamdar, Andrew Lunn, Vivien Didelot, David S. Miller,
Greg Kroah-Hartman, Joe Perches, Uwe Kleine-König
Cc: netdev, linux-kernel
In-Reply-To: <20190921133011.GA2994@nishad>
On 9/21/2019 6:30 AM, Nishad Kamdar wrote:
> This patch corrects the SPDX License Identifier style
> in header file for Broadcom BCM53xx managed switch driver.
> For C header files Documentation/process/license-rules.rst
> mandates C-like comments (opposed to C source files where
> C++ style should be used)
>
> Changes made by using a script provided by Joe Perches here:
> https://lkml.org/lkml/2019/2/7/46.
>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH] net: dsa: Use the correct style for SPDX License Identifier
From: Florian Fainelli @ 2019-09-21 19:39 UTC (permalink / raw)
To: Nishad Kamdar, Hauke Mehrtens, Andrew Lunn, Vivien Didelot,
David S. Miller, Greg Kroah-Hartman, Joe Perches,
Uwe Kleine-König
Cc: netdev, linux-kernel
In-Reply-To: <20190921134522.GA3575@nishad>
On 9/21/2019 6:45 AM, Nishad Kamdar wrote:
> This patch corrects the SPDX License Identifier style
> in header file for Distributed Switch Architecture drivers.
> For C header files Documentation/process/license-rules.rst
> mandates C-like comments (opposed to C source files where
> C++ style should be used)
>
> Changes made by using a script provided by Joe Perches here:
> https://lkml.org/lkml/2019/2/7/46.
>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix possible memleak on create vport fails
From: Jakub Kicinski @ 2019-09-22 1:06 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: gvrose8192, pshelar, netdev, Taehee Yoo
In-Reply-To: <1568734808-42628-1-git-send-email-xiangxia.m.yue@gmail.com>
On Tue, 17 Sep 2019 23:40:08 +0800, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> If we register a net device which name is not valid
> (dev_get_valid_name), register_netdevice will return err
> codes and will not run dev->priv_destructor. The memory
> will leak. This patch adds check in ovs_vport_free and
> set the vport NULL.
>
> Fixes: 309b66970ee2 ("net: openvswitch: do not free vport if register_netdevice() is failed.")
> Cc: Taehee Yoo <ap420073@gmail.com>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Thanks for the patch, I see what you're trying to do, but..
> diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
> index d2437b5..074c43f 100644
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -159,7 +159,6 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
> struct internal_dev *internal_dev;
> struct net_device *dev;
> int err;
> - bool free_vport = true;
>
> vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
> if (IS_ERR(vport)) {
> @@ -190,10 +189,8 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
>
> rtnl_lock();
> err = register_netdevice(vport->dev);
> - if (err) {
> - free_vport = false;
> + if (err)
> goto error_unlock;
> - }
>
> dev_set_promiscuity(vport->dev, 1);
> rtnl_unlock();
> @@ -207,8 +204,7 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
> error_free_netdev:
> free_netdev(dev);
> error_free_vport:
> - if (free_vport)
> - ovs_vport_free(vport);
> + ovs_vport_free(vport);
> error:
> return ERR_PTR(err);
> }
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 3fc38d1..281259a 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -157,11 +157,20 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
> */
> void ovs_vport_free(struct vport *vport)
> {
> + /* We should check whether vport is NULL.
> + * We may free it again, for example in internal_dev_create
> + * if register_netdevice fails, vport may have been freed via
> + * internal_dev_destructor.
> + */
> + if (unlikely(!vport))
> + return;
> +
> /* vport is freed from RCU callback or error path, Therefore
> * it is safe to use raw dereference.
> */
> kfree(rcu_dereference_raw(vport->upcall_portids));
> kfree(vport);
> + vport = NULL;
vport here is a function argument, seems like setting it to NULL
right before the function ends will do nothing. Should we rather
set internal_dev->vport to NULL somehow?
Perhaps someone more familiar with OvS can chime in and review..
> }
> EXPORT_SYMBOL_GPL(ovs_vport_free);
>
^ permalink raw reply
* Re: [PATCH net v3] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Jakub Kicinski @ 2019-09-22 1:29 UTC (permalink / raw)
To: Peter Mamonov, andrew
Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20190918162755.24024-1-pmamonov@gmail.com>
On Wed, 18 Sep 2019 19:27:55 +0300, Peter Mamonov wrote:
> According to the DP83865 datasheet "the 10 Mbps HDX loopback can be
> disabled in the expanded memory register 0x1C0.1". The driver erroneously
> used bit 0 instead of bit 1.
>
> Fixes: 4621bf129856 ("phy: Add file missed in previous commit.")
> Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
Applied, queued, thank you!
^ permalink raw reply
* Re: [PATCH] drivers/net/fjes: fix a potential NULL pointer dereference
From: Jakub Kicinski @ 2019-09-22 1:40 UTC (permalink / raw)
To: Allen Pais; +Cc: netdev, davem, linux-kernel
In-Reply-To: <1568824395-4162-1-git-send-email-allen.pais@oracle.com>
On Wed, 18 Sep 2019 22:03:15 +0530, Allen Pais wrote:
> alloc_workqueue is not checked for errors and as a result,
> a potential NULL dereference could occur.
>
> Signed-off-by: Allen Pais <allen.pais@oracle.com>
If I'm looking at this right you are jumping to err_free_netdev without
setting the err variable. It must had been set to 0 from the return of
fjes_sw_init(). This means we will free the netdev, and return 0. This
means probe will not fail and driver's remove function will be run
at some point. fjes_remove it will try to free the netdev again.
Looks like there's another existing bug here in that the work queues
are not free when something fails in fjes_probe, just the netdev.
Once you untangle that, and before you post a v2, could you please try
to identify which commit introduced the regression and provide an
appropriate "Fixes" tag?
> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
> index bbbc1dc..2d04104 100644
> --- a/drivers/net/fjes/fjes_main.c
> +++ b/drivers/net/fjes/fjes_main.c
> @@ -1237,8 +1237,15 @@ static int fjes_probe(struct platform_device *plat_dev)
> adapter->open_guard = false;
>
> adapter->txrx_wq = alloc_workqueue(DRV_NAME "/txrx", WQ_MEM_RECLAIM, 0);
> + if (unlikely(!adapter->txrx_wq))
> + goto err_free_netdev;
> +
> adapter->control_wq = alloc_workqueue(DRV_NAME "/control",
> WQ_MEM_RECLAIM, 0);
> + if (unlikely(!adapter->control_wq)) {
> + destroy_workqueue(adapter->txrx_wq);
> + goto err_free_netdev;
> + }
>
> INIT_WORK(&adapter->tx_stall_task, fjes_tx_stall_task);
> INIT_WORK(&adapter->raise_intr_rxdata_task,
^ permalink raw reply
* Re: [PATCH] net: qrtr: Stop rx_worker before freeing node
From: Jakub Kicinski @ 2019-09-22 1:54 UTC (permalink / raw)
To: Bjorn Andersson
Cc: David S. Miller, netdev, linux-kernel, linux-arm-msm,
Srinivas Kandagatla, stable
In-Reply-To: <20190918172117.4116-1-bjorn.andersson@linaro.org>
On Wed, 18 Sep 2019 10:21:17 -0700, Bjorn Andersson wrote:
> As the endpoint is unregistered there might still be work pending to
> handle incoming messages, which will result in a use after free
> scenario. The plan is to remove the rx_worker, but until then (and for
> stable@) ensure that the work is stopped before the node is freed.
>
> Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Thanks! Applied, and queued for 4.9+.
FWIW Dave handles the stable submission himself, so no need to CC
stable on networking patches:
https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#q-i-see-a-network-patch-and-i-think-it-should-be-backported-to-stable
^ permalink raw reply
* Re: [PATCH net] net: sched: fix possible crash in tcf_action_destroy()
From: Jakub Kicinski @ 2019-09-22 2:08 UTC (permalink / raw)
To: Cong Wang, Eric Dumazet
Cc: Eric Dumazet, David S . Miller, netdev, syzbot, Vlad Buslov,
Jiri Pirko
In-Reply-To: <CAM_iQpVyJDeScQDL6vHNAN9gu5a3c0forQ2Ko7eQihawRO_Sdw@mail.gmail.com>
On Wed, 18 Sep 2019 14:37:21 -0700, Cong Wang wrote:
> On Wed, Sep 18, 2019 at 12:57 PM 'Eric Dumazet' via syzkaller
> <syzkaller@googlegroups.com> wrote:
> >
> > If the allocation done in tcf_exts_init() failed,
> > we end up with a NULL pointer in exts->actions.
> ...
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index efd3cfb80a2ad775dc8ab3c4900bd73d52c7aaad..9aef93300f1c11791acbb9262dfe77996872eafe 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -3027,8 +3027,10 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
> > void tcf_exts_destroy(struct tcf_exts *exts)
> > {
> > #ifdef CONFIG_NET_CLS_ACT
> > - tcf_action_destroy(exts->actions, TCA_ACT_UNBIND);
> > - kfree(exts->actions);
> > + if (exts->actions) {
>
> I think it is _slightly_ better to check exts->nr_actions!=0 here,
> as it would help exts->actions!=NULL&& exts->nr_actions==0
> cases too.
>
> What do you think?
Alternatively, since tcf_exts_destroy() now takes NULL, and so
obviously does kfree() - perhaps tcf_action_destroy() should
return early if actions are NULL?
^ permalink raw reply
* Re: [PATCH] net: remove netx ethernet driver
From: Jakub Kicinski @ 2019-09-22 2:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Uwe Kleine-König, Sascha Hauer,
linux-kernel, netdev
In-Reply-To: <20190918202243.2880006-1-arnd@arndb.de>
On Wed, 18 Sep 2019 22:21:24 +0200, Arnd Bergmann wrote:
> The ARM netx platform got removed in 5.3, so this driver
> is now useless.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thank you!
FWIW there seems to be a lone mention of something called netx_timer in
linux-next, not sure if that's related too..
linux $ git grep netx_timer
Documentation/admin-guide/kernel-parameters.txt: [ARM] imx_timer1,OSTS,netx_timer,mpu_timer2,
^ permalink raw reply
* Re: [Patch net] net_sched: add max len check for TCA_KIND
From: Jakub Kicinski @ 2019-09-22 2:24 UTC (permalink / raw)
To: Cong Wang
Cc: David Ahern, Linux Kernel Network Developers, syzbot,
Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <CAM_iQpXa=Kru2tXKwrErM9VsO40coBf9gKLRfwC3e8owKZG+0w@mail.gmail.com>
On Wed, 18 Sep 2019 22:15:24 -0700, Cong Wang wrote:
> On Wed, Sep 18, 2019 at 7:41 PM David Ahern <dsahern@gmail.com> wrote:
> > On 9/18/19 5:24 PM, Cong Wang wrote:
> > > The TCA_KIND attribute is of NLA_STRING which does not check
> > > the NUL char. KMSAN reported an uninit-value of TCA_KIND which
> > > is likely caused by the lack of NUL.
> > >
> > > Change it to NLA_NUL_STRING and add a max len too.
> > >
> > > Fixes: 8b4c3cdd9dd8 ("net: sched: Add policy validation for tc attributes")
> >
> > The commit referenced here did not introduce the ability to go beyond
> > memory boundaries with string comparisons. Rather, it was not complete
> > solution for attribute validation. I say that wrt to the fix getting
> > propagated to the correct stable releases.
>
> I think this patch should be backported to wherever commit 8b4c3cdd9dd8
> goes, this is why I picked it as Fixes.
Applied, queued for 4.14+, thanks!
> >
> > > Reported-and-tested-by: syzbot+618aacd49e8c8b8486bd@syzkaller.appspotmail.com
> >
> > What is the actual sysbot report?
>
> https://marc.info/?l=linux-kernel&m=156862916112881&w=2
^ permalink raw reply
* Re: [Patch net] net_sched: add policy validation for action attributes
From: Jakub Kicinski @ 2019-09-22 2:38 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David Ahern, Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <20190919014443.32581-1-xiyou.wangcong@gmail.com>
On Wed, 18 Sep 2019 18:44:43 -0700, Cong Wang wrote:
> Similar to commit 8b4c3cdd9dd8
> ("net: sched: Add policy validation for tc attributes"), we need
> to add proper policy validation for TC action attributes too.
>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thank you!
Queued for stable 5.2+ let me know if we should backport to long term
kernels, or not at all.
^ permalink raw reply
* Re: [PATCH net] net: dsa: sja1105: Add dependency for NET_DSA_SJA1105_TAS
From: Jakub Kicinski @ 2019-09-22 2:43 UTC (permalink / raw)
To: Mao Wenan
Cc: olteanv, andrew, vivien.didelot, f.fainelli, davem, linux-kernel,
netdev, kernel-janitors
In-Reply-To: <20190919063819.164826-1-maowenan@huawei.com>
On Thu, 19 Sep 2019 14:38:19 +0800, Mao Wenan wrote:
> If CONFIG_NET_DSA_SJA1105_TAS=y and CONFIG_NET_SCH_TAPRIO=n,
> below error can be found:
> drivers/net/dsa/sja1105/sja1105_tas.o: In function `sja1105_setup_tc_taprio':
> sja1105_tas.c:(.text+0x318): undefined reference to `taprio_offload_free'
> sja1105_tas.c:(.text+0x590): undefined reference to `taprio_offload_get'
> drivers/net/dsa/sja1105/sja1105_tas.o: In function `sja1105_tas_teardown':
> sja1105_tas.c:(.text+0x610): undefined reference to `taprio_offload_free'
> make: *** [vmlinux] Error 1
>
> sja1105_tas needs tc-taprio, so this patch add the dependency for it.
>
> Fixes: 317ab5b86c8e ("net: dsa: sja1105: Configure the Time-Aware Scheduler via tc-taprio offload")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Applied, thank you!
^ permalink raw reply
* Re: [PATCH] [v2] net: stmmac: selftest: avoid large stack usage
From: Jakub Kicinski @ 2019-09-22 2:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190919123416.3070938-1-arnd@arndb.de>
On Thu, 19 Sep 2019 14:33:43 +0200, Arnd Bergmann wrote:
> Putting a struct stmmac_rss object on the stack is a bad idea,
> as it exceeds the warning limit for a stack frame on 32-bit architectures:
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1221:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l3filt' [-Werror,-Wframe-larger-than=]
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1338:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l4filt' [-Werror,-Wframe-larger-than=]
>
> As the object is the trivial empty case, change the called function
> to accept a NULL pointer to mean the same thing and remove the
> large variable in the two callers.
>
> Fixes: 4647e021193d ("net: stmmac: selftests: Add selftest for L3/L4 Filters")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thank you!
^ permalink raw reply
* Re: [PATCH] dimlib: make DIMLIB a hidden symbol
From: Jakub Kicinski @ 2019-09-22 3:06 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Tal Gilboa, Saeed Mahameed, linux-kernel, netdev
In-Reply-To: <20190920133115.12802-1-uwe@kleine-koenig.org>
On Fri, 20 Sep 2019 15:31:15 +0200, Uwe Kleine-König wrote:
> According to Tal Gilboa the only benefit from DIM comes from a driver
> that uses it. So it doesn't make sense to make this symbol user visible,
> instead all drivers that use it should select it (as is already the case
> AFAICT).
>
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
> lib/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/Kconfig b/lib/Kconfig
> index cc04124ed8f7..9fe8a21fd183 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -555,8 +555,7 @@ config SIGNATURE
> Implementation is done using GnuPG MPI library
>
> config DIMLIB
> - bool "DIM library"
> - default y
> + bool
> help
> Dynamic Interrupt Moderation library.
> Implements an algorithm for dynamically change CQ moderation values
Hi Uwe! Looks like in the net tree there is a spelling mistake and
moderation is spelled "modertion":
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/tree/lib/Kconfig#n562
I'm not seeing any patch to fix that anywhere, is it possible you have
some local change in your tree?
^ permalink raw reply
* Re: [PATCH net] net: ena: Add dependency for ENA_ETHERNET
From: Jakub Kicinski @ 2019-09-22 3:07 UTC (permalink / raw)
To: Mao Wenan
Cc: netanel, saeedb, zorik, davem, netdev, linux-kernel,
kernel-janitors, Uwe Kleine-König
In-Reply-To: <20190920084405.140750-1-maowenan@huawei.com>
On Fri, 20 Sep 2019 16:44:05 +0800, Mao Wenan wrote:
> If CONFIG_ENA_ETHERNET=y and CONFIG_DIMLIB=n,
> below erros can be found:
> drivers/net/ethernet/amazon/ena/ena_netdev.o: In function `ena_dim_work':
> ena_netdev.c:(.text+0x21cc): undefined reference to `net_dim_get_rx_moderation'
> ena_netdev.c:(.text+0x21cc): relocation truncated to
> fit: R_AARCH64_CALL26 against undefined symbol `net_dim_get_rx_moderation'
> drivers/net/ethernet/amazon/ena/ena_netdev.o: In function `ena_io_poll':
> ena_netdev.c:(.text+0x7bd4): undefined reference to `net_dim'
> ena_netdev.c:(.text+0x7bd4): relocation truncated to fit:
> R_AARCH64_CALL26 against undefined symbol `net_dim'
>
> After commit 282faf61a053 ("net: ena: switch to dim algorithm for rx adaptive
> interrupt moderation"), it introduces dim algorithm, which configured by CONFIG_DIMLIB.
>
> Fixes: 282faf61a053 ("net: ena: switch to dim algorithm for rx adaptive interrupt moderation")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Thank you Mao, shortly after you posted your patch Uwe proposed to make
DIMLIB a hidden symbol:
https://lore.kernel.org/netdev/a85be675-ce56-f7ba-29e9-b749073aab6c@kleine-koenig.org/T/#t
That patch will likely be applied soon, could you please rework your
patch to use the "select" keyword instead of "depends". That's what
other users do.
> diff --git a/drivers/net/ethernet/amazon/Kconfig b/drivers/net/ethernet/amazon/Kconfig
> index 69ca99d..fe46df4 100644
> --- a/drivers/net/ethernet/amazon/Kconfig
> +++ b/drivers/net/ethernet/amazon/Kconfig
> @@ -18,7 +18,7 @@ if NET_VENDOR_AMAZON
>
> config ENA_ETHERNET
> tristate "Elastic Network Adapter (ENA) support"
> - depends on PCI_MSI && !CPU_BIG_ENDIAN
> + depends on PCI_MSI && !CPU_BIG_ENDIAN && DIMLIB
> ---help---
> This driver supports Elastic Network Adapter (ENA)"
>
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix possible memleak on create vport fails
From: Pravin Shelar @ 2019-09-22 3:17 UTC (permalink / raw)
To: Tonghao Zhang, Hillf Danton
Cc: Greg Rose, Linux Kernel Network Developers, Taehee Yoo
In-Reply-To: <1568734808-42628-1-git-send-email-xiangxia.m.yue@gmail.com>
On Sat, Sep 21, 2019 at 8:07 AM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> If we register a net device which name is not valid
> (dev_get_valid_name), register_netdevice will return err
> codes and will not run dev->priv_destructor. The memory
> will leak. This patch adds check in ovs_vport_free and
> set the vport NULL.
>
> Fixes: 309b66970ee2 ("net: openvswitch: do not free vport if register_netdevice() is failed.")
> Cc: Taehee Yoo <ap420073@gmail.com>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> net/openvswitch/vport-internal_dev.c | 8 ++------
> net/openvswitch/vport.c | 9 +++++++++
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
> index d2437b5..074c43f 100644
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -159,7 +159,6 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
> struct internal_dev *internal_dev;
> struct net_device *dev;
> int err;
> - bool free_vport = true;
>
> vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
> if (IS_ERR(vport)) {
> @@ -190,10 +189,8 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
>
> rtnl_lock();
> err = register_netdevice(vport->dev);
> - if (err) {
> - free_vport = false;
> + if (err)
> goto error_unlock;
> - }
>
> dev_set_promiscuity(vport->dev, 1);
> rtnl_unlock();
> @@ -207,8 +204,7 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
> error_free_netdev:
> free_netdev(dev);
> error_free_vport:
> - if (free_vport)
> - ovs_vport_free(vport);
> + ovs_vport_free(vport);
> error:
> return ERR_PTR(err);
> }
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 3fc38d1..281259a 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -157,11 +157,20 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
> */
> void ovs_vport_free(struct vport *vport)
> {
> + /* We should check whether vport is NULL.
> + * We may free it again, for example in internal_dev_create
> + * if register_netdevice fails, vport may have been freed via
> + * internal_dev_destructor.
> + */
> + if (unlikely(!vport))
> + return;
> +
> /* vport is freed from RCU callback or error path, Therefore
> * it is safe to use raw dereference.
> */
> kfree(rcu_dereference_raw(vport->upcall_portids));
> kfree(vport);
> + vport = NULL;
> }
> EXPORT_SYMBOL_GPL(ovs_vport_free);
>
> --
There is already patch a patch to fix this memory leak.
https://patchwork.ozlabs.org/patch/1144316/
Can you or Hillf post it on netdev list?
Thanks.
^ 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