| Message ID | 1533062405-32524-3-git-send-email-amit.pundir@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net/sched: init failure fixes | expand |
On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote: > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > commit e89d469e3be3ed3d7124a803211a463ff83d0964 upstream. > > The below commit added a call to ->destroy() on init failure, but multiq > still frees ->queues on error in init, but ->queues is also freed by > ->destroy() thus we get double free and corrupted memory. > > Very easy to reproduce (eth0 not multiqueue): > $ tc qdisc add dev eth0 root multiq > RTNETLINK answers: Operation not supported > $ ip l add dumdum type dummy > (crash) > > Trace log: > [ 3929.467747] general protection fault: 0000 [#1] SMP > [ 3929.468083] Modules linked in: > [ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56 > [ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 > [ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000 > [ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be > [ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246 > [ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df > [ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020 > [ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000 > [ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564 > [ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00 > [ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000 > [ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0 > [ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 3929.474873] Call Trace: > [ 3929.475337] ? kstrdup_const+0x23/0x25 > [ 3929.475863] kstrdup+0x2e/0x4b > [ 3929.476338] kstrdup_const+0x23/0x25 > [ 3929.478084] __kernfs_new_node+0x28/0xbc > [ 3929.478478] kernfs_new_node+0x35/0x55 > [ 3929.478929] kernfs_create_link+0x23/0x76 > [ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7 > [ 3929.480096] sysfs_create_link+0x33/0x35 > [ 3929.480649] device_add+0x200/0x589 > [ 3929.481184] netdev_register_kobject+0x7c/0x12f > [ 3929.481711] register_netdevice+0x373/0x471 > [ 3929.482174] rtnl_newlink+0x614/0x729 > [ 3929.482610] ? rtnl_newlink+0x17f/0x729 > [ 3929.483080] rtnetlink_rcv_msg+0x188/0x197 > [ 3929.483533] ? rcu_read_unlock+0x3e/0x5f > [ 3929.483984] ? rtnl_newlink+0x729/0x729 > [ 3929.484420] netlink_rcv_skb+0x6c/0xce > [ 3929.484858] rtnetlink_rcv+0x23/0x2a > [ 3929.485291] netlink_unicast+0x103/0x181 > [ 3929.485735] netlink_sendmsg+0x326/0x337 > [ 3929.486181] sock_sendmsg_nosec+0x14/0x3f > [ 3929.486614] sock_sendmsg+0x29/0x2e > [ 3929.486973] ___sys_sendmsg+0x209/0x28b > [ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8 > [ 3929.487719] ? _raw_spin_unlock+0x27/0x31 > [ 3929.488092] ? __handle_mm_fault+0x651/0xdb1 > [ 3929.488471] ? check_chain_key+0xb0/0xfd > [ 3929.488847] __sys_sendmsg+0x45/0x63 > [ 3929.489206] ? __sys_sendmsg+0x45/0x63 > [ 3929.489576] SyS_sendmsg+0x19/0x1b > [ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2 > [ 3929.490172] RIP: 0033:0x7f0b6fb93690 > [ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > [ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690 > [ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003 > [ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000 > [ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002 > [ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000 > [ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf > [ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44 > 89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d > 8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01 > [ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0 > > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation") > Fixes: f07d1501292b ("multiq: Further multiqueue cleanup") > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > --- > net/sched/sch_multiq.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c > index 9ffbb025b37e..a0a3c8b4586c 100644 > --- a/net/sched/sch_multiq.c > +++ b/net/sched/sch_multiq.c > @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) > for (i = 0; i < q->max_bands; i++) > q->queues[i] = &noop_qdisc; > > - err = multiq_tune(sch, opt); > - > - if (err) > - kfree(q->queues); > - > - return err; > + return multiq_tune(sch, opt); > } > > static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb) > -- > 2.7.4 This patch adds a build warning to the build, and you never fix that up :( Are you sure you tested this? Can you find the needed fix for this and send an updated series? Odds are your 4.4.y series also has the same problem, right? thanks, greg k-h
On Thu, 16 Aug 2018 at 22:06, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote: > > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > commit e89d469e3be3ed3d7124a803211a463ff83d0964 upstream. > > > > The below commit added a call to ->destroy() on init failure, but multiq > > still frees ->queues on error in init, but ->queues is also freed by > > ->destroy() thus we get double free and corrupted memory. > > > > Very easy to reproduce (eth0 not multiqueue): > > $ tc qdisc add dev eth0 root multiq > > RTNETLINK answers: Operation not supported > > $ ip l add dumdum type dummy > > (crash) > > > > Trace log: > > [ 3929.467747] general protection fault: 0000 [#1] SMP > > [ 3929.468083] Modules linked in: > > [ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56 > > [ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 > > [ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000 > > [ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be > > [ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246 > > [ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df > > [ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020 > > [ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000 > > [ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564 > > [ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00 > > [ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000 > > [ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > [ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0 > > [ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > [ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > [ 3929.474873] Call Trace: > > [ 3929.475337] ? kstrdup_const+0x23/0x25 > > [ 3929.475863] kstrdup+0x2e/0x4b > > [ 3929.476338] kstrdup_const+0x23/0x25 > > [ 3929.478084] __kernfs_new_node+0x28/0xbc > > [ 3929.478478] kernfs_new_node+0x35/0x55 > > [ 3929.478929] kernfs_create_link+0x23/0x76 > > [ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7 > > [ 3929.480096] sysfs_create_link+0x33/0x35 > > [ 3929.480649] device_add+0x200/0x589 > > [ 3929.481184] netdev_register_kobject+0x7c/0x12f > > [ 3929.481711] register_netdevice+0x373/0x471 > > [ 3929.482174] rtnl_newlink+0x614/0x729 > > [ 3929.482610] ? rtnl_newlink+0x17f/0x729 > > [ 3929.483080] rtnetlink_rcv_msg+0x188/0x197 > > [ 3929.483533] ? rcu_read_unlock+0x3e/0x5f > > [ 3929.483984] ? rtnl_newlink+0x729/0x729 > > [ 3929.484420] netlink_rcv_skb+0x6c/0xce > > [ 3929.484858] rtnetlink_rcv+0x23/0x2a > > [ 3929.485291] netlink_unicast+0x103/0x181 > > [ 3929.485735] netlink_sendmsg+0x326/0x337 > > [ 3929.486181] sock_sendmsg_nosec+0x14/0x3f > > [ 3929.486614] sock_sendmsg+0x29/0x2e > > [ 3929.486973] ___sys_sendmsg+0x209/0x28b > > [ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8 > > [ 3929.487719] ? _raw_spin_unlock+0x27/0x31 > > [ 3929.488092] ? __handle_mm_fault+0x651/0xdb1 > > [ 3929.488471] ? check_chain_key+0xb0/0xfd > > [ 3929.488847] __sys_sendmsg+0x45/0x63 > > [ 3929.489206] ? __sys_sendmsg+0x45/0x63 > > [ 3929.489576] SyS_sendmsg+0x19/0x1b > > [ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2 > > [ 3929.490172] RIP: 0033:0x7f0b6fb93690 > > [ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > > [ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690 > > [ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003 > > [ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000 > > [ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002 > > [ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000 > > [ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf > > [ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44 > > 89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d > > 8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01 > > [ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0 > > > > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation") > > Fixes: f07d1501292b ("multiq: Further multiqueue cleanup") > > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > Signed-off-by: David S. Miller <davem@davemloft.net> > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > > --- > > net/sched/sch_multiq.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c > > index 9ffbb025b37e..a0a3c8b4586c 100644 > > --- a/net/sched/sch_multiq.c > > +++ b/net/sched/sch_multiq.c > > @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) > > for (i = 0; i < q->max_bands; i++) > > q->queues[i] = &noop_qdisc; > > > > - err = multiq_tune(sch, opt); > > - > > - if (err) > > - kfree(q->queues); > > - > > - return err; > > + return multiq_tune(sch, opt); > > } > > > > static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb) > > -- > > 2.7.4 > > This patch adds a build warning to the build, and you never fix that up > :( > > Are you sure you tested this? > > Can you find the needed fix for this and send an updated series? Odds > are your 4.4.y series also has the same problem, right? Sorry about that. I'll build test both the series and resend. Thanks. Regards, Amit Pundir > > thanks, > > greg k-h
On Thu, 16 Aug 2018 at 22:19, Amit Pundir <amit.pundir@linaro.org> wrote: > > On Thu, 16 Aug 2018 at 22:06, Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote: > > > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > > commit e89d469e3be3ed3d7124a803211a463ff83d0964 upstream. > > > > > > The below commit added a call to ->destroy() on init failure, but multiq > > > still frees ->queues on error in init, but ->queues is also freed by > > > ->destroy() thus we get double free and corrupted memory. > > > > > > Very easy to reproduce (eth0 not multiqueue): > > > $ tc qdisc add dev eth0 root multiq > > > RTNETLINK answers: Operation not supported > > > $ ip l add dumdum type dummy > > > (crash) > > > > > > Trace log: > > > [ 3929.467747] general protection fault: 0000 [#1] SMP > > > [ 3929.468083] Modules linked in: > > > [ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56 > > > [ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 > > > [ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000 > > > [ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be > > > [ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246 > > > [ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df > > > [ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020 > > > [ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000 > > > [ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564 > > > [ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00 > > > [ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000 > > > [ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > [ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0 > > > [ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > [ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > [ 3929.474873] Call Trace: > > > [ 3929.475337] ? kstrdup_const+0x23/0x25 > > > [ 3929.475863] kstrdup+0x2e/0x4b > > > [ 3929.476338] kstrdup_const+0x23/0x25 > > > [ 3929.478084] __kernfs_new_node+0x28/0xbc > > > [ 3929.478478] kernfs_new_node+0x35/0x55 > > > [ 3929.478929] kernfs_create_link+0x23/0x76 > > > [ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7 > > > [ 3929.480096] sysfs_create_link+0x33/0x35 > > > [ 3929.480649] device_add+0x200/0x589 > > > [ 3929.481184] netdev_register_kobject+0x7c/0x12f > > > [ 3929.481711] register_netdevice+0x373/0x471 > > > [ 3929.482174] rtnl_newlink+0x614/0x729 > > > [ 3929.482610] ? rtnl_newlink+0x17f/0x729 > > > [ 3929.483080] rtnetlink_rcv_msg+0x188/0x197 > > > [ 3929.483533] ? rcu_read_unlock+0x3e/0x5f > > > [ 3929.483984] ? rtnl_newlink+0x729/0x729 > > > [ 3929.484420] netlink_rcv_skb+0x6c/0xce > > > [ 3929.484858] rtnetlink_rcv+0x23/0x2a > > > [ 3929.485291] netlink_unicast+0x103/0x181 > > > [ 3929.485735] netlink_sendmsg+0x326/0x337 > > > [ 3929.486181] sock_sendmsg_nosec+0x14/0x3f > > > [ 3929.486614] sock_sendmsg+0x29/0x2e > > > [ 3929.486973] ___sys_sendmsg+0x209/0x28b > > > [ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8 > > > [ 3929.487719] ? _raw_spin_unlock+0x27/0x31 > > > [ 3929.488092] ? __handle_mm_fault+0x651/0xdb1 > > > [ 3929.488471] ? check_chain_key+0xb0/0xfd > > > [ 3929.488847] __sys_sendmsg+0x45/0x63 > > > [ 3929.489206] ? __sys_sendmsg+0x45/0x63 > > > [ 3929.489576] SyS_sendmsg+0x19/0x1b > > > [ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2 > > > [ 3929.490172] RIP: 0033:0x7f0b6fb93690 > > > [ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > > > [ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690 > > > [ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003 > > > [ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000 > > > [ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002 > > > [ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000 > > > [ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf > > > [ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44 > > > 89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d > > > 8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01 > > > [ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0 > > > > > > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation") > > > Fixes: f07d1501292b ("multiq: Further multiqueue cleanup") > > > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > Signed-off-by: David S. Miller <davem@davemloft.net> > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > > > --- > > > net/sched/sch_multiq.c | 7 +------ > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c > > > index 9ffbb025b37e..a0a3c8b4586c 100644 > > > --- a/net/sched/sch_multiq.c > > > +++ b/net/sched/sch_multiq.c > > > @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) > > > for (i = 0; i < q->max_bands; i++) > > > q->queues[i] = &noop_qdisc; > > > > > > - err = multiq_tune(sch, opt); > > > - > > > - if (err) > > > - kfree(q->queues); > > > - > > > - return err; > > > + return multiq_tune(sch, opt); > > > } > > > > > > static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb) > > > -- > > > 2.7.4 > > > > This patch adds a build warning to the build, and you never fix that up > > :( > > > > Are you sure you tested this? > > > > Can you find the needed fix for this and send an updated series? Odds > > are your 4.4.y series also has the same problem, right? Hi Greg, So this is the only problematic patch in the series, which left an unused variable behind and hence the build warning. Should I just resend this patch alone for both 4.9.y and 4.4.y instead of the entire series? > > Sorry about that. I'll build test both the series and resend. Thanks. > > Regards, > Amit Pundir > > > > > thanks, > > > > greg k-h
On Tue, Aug 21, 2018 at 12:57:42PM +0530, Amit Pundir wrote: > On Thu, 16 Aug 2018 at 22:19, Amit Pundir <amit.pundir@linaro.org> wrote: > > > > On Thu, 16 Aug 2018 at 22:06, Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote: > > > > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > > > > commit e89d469e3be3ed3d7124a803211a463ff83d0964 upstream. > > > > > > > > The below commit added a call to ->destroy() on init failure, but multiq > > > > still frees ->queues on error in init, but ->queues is also freed by > > > > ->destroy() thus we get double free and corrupted memory. > > > > > > > > Very easy to reproduce (eth0 not multiqueue): > > > > $ tc qdisc add dev eth0 root multiq > > > > RTNETLINK answers: Operation not supported > > > > $ ip l add dumdum type dummy > > > > (crash) > > > > > > > > Trace log: > > > > [ 3929.467747] general protection fault: 0000 [#1] SMP > > > > [ 3929.468083] Modules linked in: > > > > [ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56 > > > > [ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 > > > > [ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000 > > > > [ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be > > > > [ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246 > > > > [ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df > > > > [ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020 > > > > [ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000 > > > > [ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564 > > > > [ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00 > > > > [ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000 > > > > [ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > [ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0 > > > > [ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > > [ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > > [ 3929.474873] Call Trace: > > > > [ 3929.475337] ? kstrdup_const+0x23/0x25 > > > > [ 3929.475863] kstrdup+0x2e/0x4b > > > > [ 3929.476338] kstrdup_const+0x23/0x25 > > > > [ 3929.478084] __kernfs_new_node+0x28/0xbc > > > > [ 3929.478478] kernfs_new_node+0x35/0x55 > > > > [ 3929.478929] kernfs_create_link+0x23/0x76 > > > > [ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7 > > > > [ 3929.480096] sysfs_create_link+0x33/0x35 > > > > [ 3929.480649] device_add+0x200/0x589 > > > > [ 3929.481184] netdev_register_kobject+0x7c/0x12f > > > > [ 3929.481711] register_netdevice+0x373/0x471 > > > > [ 3929.482174] rtnl_newlink+0x614/0x729 > > > > [ 3929.482610] ? rtnl_newlink+0x17f/0x729 > > > > [ 3929.483080] rtnetlink_rcv_msg+0x188/0x197 > > > > [ 3929.483533] ? rcu_read_unlock+0x3e/0x5f > > > > [ 3929.483984] ? rtnl_newlink+0x729/0x729 > > > > [ 3929.484420] netlink_rcv_skb+0x6c/0xce > > > > [ 3929.484858] rtnetlink_rcv+0x23/0x2a > > > > [ 3929.485291] netlink_unicast+0x103/0x181 > > > > [ 3929.485735] netlink_sendmsg+0x326/0x337 > > > > [ 3929.486181] sock_sendmsg_nosec+0x14/0x3f > > > > [ 3929.486614] sock_sendmsg+0x29/0x2e > > > > [ 3929.486973] ___sys_sendmsg+0x209/0x28b > > > > [ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8 > > > > [ 3929.487719] ? _raw_spin_unlock+0x27/0x31 > > > > [ 3929.488092] ? __handle_mm_fault+0x651/0xdb1 > > > > [ 3929.488471] ? check_chain_key+0xb0/0xfd > > > > [ 3929.488847] __sys_sendmsg+0x45/0x63 > > > > [ 3929.489206] ? __sys_sendmsg+0x45/0x63 > > > > [ 3929.489576] SyS_sendmsg+0x19/0x1b > > > > [ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2 > > > > [ 3929.490172] RIP: 0033:0x7f0b6fb93690 > > > > [ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > > > > [ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690 > > > > [ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003 > > > > [ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000 > > > > [ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002 > > > > [ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000 > > > > [ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf > > > > [ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44 > > > > 89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d > > > > 8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01 > > > > [ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0 > > > > > > > > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation") > > > > Fixes: f07d1501292b ("multiq: Further multiqueue cleanup") > > > > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > Signed-off-by: David S. Miller <davem@davemloft.net> > > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > > > > --- > > > > net/sched/sch_multiq.c | 7 +------ > > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > > > diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c > > > > index 9ffbb025b37e..a0a3c8b4586c 100644 > > > > --- a/net/sched/sch_multiq.c > > > > +++ b/net/sched/sch_multiq.c > > > > @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) > > > > for (i = 0; i < q->max_bands; i++) > > > > q->queues[i] = &noop_qdisc; > > > > > > > > - err = multiq_tune(sch, opt); > > > > - > > > > - if (err) > > > > - kfree(q->queues); > > > > - > > > > - return err; > > > > + return multiq_tune(sch, opt); > > > > } > > > > > > > > static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb) > > > > -- > > > > 2.7.4 > > > > > > This patch adds a build warning to the build, and you never fix that up > > > :( > > > > > > Are you sure you tested this? > > > > > > Can you find the needed fix for this and send an updated series? Odds > > > are your 4.4.y series also has the same problem, right? > > Hi Greg, > > So this is the only problematic patch in the series, which left an > unused variable behind and hence the build warning. Should I just > resend this patch alone for both 4.9.y and 4.4.y instead of the entire > series? Please resend the whole series. And why does the warning not show up upstream as well? Where was it fixed there? thanks, greg k-h
On Tue, 21 Aug 2018 at 19:11, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Tue, Aug 21, 2018 at 12:57:42PM +0530, Amit Pundir wrote: > > On Thu, 16 Aug 2018 at 22:19, Amit Pundir <amit.pundir@linaro.org> wrote: > > > > > > On Thu, 16 Aug 2018 at 22:06, Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote: > > > > > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > > > > > > commit e89d469e3be3ed3d7124a803211a463ff83d0964 upstream. > > > > > > > > > > The below commit added a call to ->destroy() on init failure, but multiq > > > > > still frees ->queues on error in init, but ->queues is also freed by > > > > > ->destroy() thus we get double free and corrupted memory. > > > > > > > > > > Very easy to reproduce (eth0 not multiqueue): > > > > > $ tc qdisc add dev eth0 root multiq > > > > > RTNETLINK answers: Operation not supported > > > > > $ ip l add dumdum type dummy > > > > > (crash) > > > > > > > > > > Trace log: > > > > > [ 3929.467747] general protection fault: 0000 [#1] SMP > > > > > [ 3929.468083] Modules linked in: > > > > > [ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56 > > > > > [ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 > > > > > [ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000 > > > > > [ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be > > > > > [ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246 > > > > > [ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df > > > > > [ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020 > > > > > [ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000 > > > > > [ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564 > > > > > [ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00 > > > > > [ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000 > > > > > [ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > > [ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0 > > > > > [ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > > > [ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > > > [ 3929.474873] Call Trace: > > > > > [ 3929.475337] ? kstrdup_const+0x23/0x25 > > > > > [ 3929.475863] kstrdup+0x2e/0x4b > > > > > [ 3929.476338] kstrdup_const+0x23/0x25 > > > > > [ 3929.478084] __kernfs_new_node+0x28/0xbc > > > > > [ 3929.478478] kernfs_new_node+0x35/0x55 > > > > > [ 3929.478929] kernfs_create_link+0x23/0x76 > > > > > [ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7 > > > > > [ 3929.480096] sysfs_create_link+0x33/0x35 > > > > > [ 3929.480649] device_add+0x200/0x589 > > > > > [ 3929.481184] netdev_register_kobject+0x7c/0x12f > > > > > [ 3929.481711] register_netdevice+0x373/0x471 > > > > > [ 3929.482174] rtnl_newlink+0x614/0x729 > > > > > [ 3929.482610] ? rtnl_newlink+0x17f/0x729 > > > > > [ 3929.483080] rtnetlink_rcv_msg+0x188/0x197 > > > > > [ 3929.483533] ? rcu_read_unlock+0x3e/0x5f > > > > > [ 3929.483984] ? rtnl_newlink+0x729/0x729 > > > > > [ 3929.484420] netlink_rcv_skb+0x6c/0xce > > > > > [ 3929.484858] rtnetlink_rcv+0x23/0x2a > > > > > [ 3929.485291] netlink_unicast+0x103/0x181 > > > > > [ 3929.485735] netlink_sendmsg+0x326/0x337 > > > > > [ 3929.486181] sock_sendmsg_nosec+0x14/0x3f > > > > > [ 3929.486614] sock_sendmsg+0x29/0x2e > > > > > [ 3929.486973] ___sys_sendmsg+0x209/0x28b > > > > > [ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8 > > > > > [ 3929.487719] ? _raw_spin_unlock+0x27/0x31 > > > > > [ 3929.488092] ? __handle_mm_fault+0x651/0xdb1 > > > > > [ 3929.488471] ? check_chain_key+0xb0/0xfd > > > > > [ 3929.488847] __sys_sendmsg+0x45/0x63 > > > > > [ 3929.489206] ? __sys_sendmsg+0x45/0x63 > > > > > [ 3929.489576] SyS_sendmsg+0x19/0x1b > > > > > [ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2 > > > > > [ 3929.490172] RIP: 0033:0x7f0b6fb93690 > > > > > [ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > > > > > [ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690 > > > > > [ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003 > > > > > [ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000 > > > > > [ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002 > > > > > [ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000 > > > > > [ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf > > > > > [ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44 > > > > > 89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d > > > > > 8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01 > > > > > [ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0 > > > > > > > > > > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation") > > > > > Fixes: f07d1501292b ("multiq: Further multiqueue cleanup") > > > > > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > Signed-off-by: David S. Miller <davem@davemloft.net> > > > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org> > > > > > --- > > > > > net/sched/sch_multiq.c | 7 +------ > > > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > > > > > diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c > > > > > index 9ffbb025b37e..a0a3c8b4586c 100644 > > > > > --- a/net/sched/sch_multiq.c > > > > > +++ b/net/sched/sch_multiq.c > > > > > @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) > > > > > for (i = 0; i < q->max_bands; i++) > > > > > q->queues[i] = &noop_qdisc; > > > > > > > > > > - err = multiq_tune(sch, opt); > > > > > - > > > > > - if (err) > > > > > - kfree(q->queues); > > > > > - > > > > > - return err; > > > > > + return multiq_tune(sch, opt); > > > > > } > > > > > > > > > > static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb) > > > > > -- > > > > > 2.7.4 > > > > > > > > This patch adds a build warning to the build, and you never fix that up > > > > :( > > > > > > > > Are you sure you tested this? > > > > > > > > Can you find the needed fix for this and send an updated series? Odds > > > > are your 4.4.y series also has the same problem, right? > > > > Hi Greg, > > > > So this is the only problematic patch in the series, which left an > > unused variable behind and hence the build warning. Should I just > > resend this patch alone for both 4.9.y and 4.4.y instead of the entire > > series? > > Please resend the whole series. And why does the warning not show up > upstream as well? Where was it fixed there? Thanks I'll resend the whole series. Upstream was making use of that variable to track tcf_blocks as well, so no "unused variable" warning there. Regards, Amit Pundir > > thanks, > > greg k-h
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c index 9ffbb025b37e..a0a3c8b4586c 100644 --- a/net/sched/sch_multiq.c +++ b/net/sched/sch_multiq.c @@ -249,12 +249,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt) for (i = 0; i < q->max_bands; i++) q->queues[i] = &noop_qdisc; - err = multiq_tune(sch, opt); - - if (err) - kfree(q->queues); - - return err; + return multiq_tune(sch, opt); } static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb)