On Tue, 21 Aug 2018 at 19:11, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> wrote:
> > >
> > > On Thu, 16 Aug 2018 at 22:06, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Wed, Aug 01, 2018 at 12:10:02AM +0530, Amit Pundir wrote:
> > > > > From: Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx>
> > > > >
> > > > > 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@xxxxxxxxxxxxxxxxxxx>
> > > > > Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> > > > > Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
> > > > > ---
> > > > > 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
