[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56590159.4080404@hurleysoftware.com>
Date: Fri, 27 Nov 2015 20:20:25 -0500
From: Peter Hurley <peter@...leysoftware.com>
To: Paul Bolle <pebolle@...cali.nl>,
Sasha Levin <sasha.levin@...cle.com>
Cc: isdn@...ux-pingi.de, davem@...emloft.net, tilman@...p.cc,
gigaset307x-common@...ts.sourceforge.net,
LKML <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
syzkaller <syzkaller@...glegroups.com>
Subject: Re: gigaset: freeing an active object
On 11/27/2015 07:28 PM, Paul Bolle wrote:
> (A few quick notes follow. The hope here is basically that my display of
> ignorance might trigger others to speak up while I'm still pondering on
> this bug.)
>
> On vr, 2015-11-27 at 13:15 -0500, Sasha Levin wrote:
>> On 11/27/2015 12:57 PM, Paul Bolle wrote:
>>> On vr, 2015-11-27 at 10:19 -0500, Sasha Levin wrote:
>>>> Fuzzing with syzkaller on the latest -next kernel produced this
>>>> error:
>>>
>>> (syzkaller is new to me. I'll have to do some web searches.)
>>
>> It's a new fancy syscall/ioctl fuzzer,
>> https://github.com/google/syzkaller/blob/master/README.md
>
> Thanks.
>
> That fuzzer apparently requires either CONFIG_KASAN, CONFIG_KTSAN, or
> CONFIG_UBSAN, none of which I'm familiar with.
>
>>>> [ 413.536749] WARNING: CPU: 6 PID: 25400 at
>>>> lib/debugobjects.c:263
>>>> debug_print_object+0x1c4/0x1e0()
>>>> [ 413.538111] ODEBUG: free active (active state 0) object type:
>>>> timer_list hint: delayed_work_timer_fn+0x0/0x90
>
> There are two places that add "free" here, but there's no obvious way to
> distinguish between them. Annoying.
>
> Anyhow, this is interesting. ser-gigaset doesn't use timer_list while
> bas-gigaset does.
>
>>>> [ 413.539598] Modules linked
>>>> in:3470693efef57268844f02f5de3ab392d8cf5e209671ddd87163cb964c51065
>>>> 9
>
> Not sure what this means. The bug concerns ser-gigaset so it's safe to
> assume the fuzzer at one point called
> ioctl(fd, TIOCSETD, N_GIGASET_M101)
Sasha,
It would really help if you included the syzkaller-generated applet with
the bug reports; state previously established by the applet can be
crucial in understanding why the call stack looks the way it does.
Also, every generated applet that triggers a report should become
a future regression test; I'm collecting the ones pertinent to tty/serial/
ldisc (so that includes this one; if you could send me the x25 one too
would be great).
Regards,
Peter Hurley
> which would trigger the use of ser-gigaset.
>
>>>> [ 413.540448] CPU: 6 PID: 25400 Comm: syzkaller_execu Not tainted
>>>> 4.4.0-rc2-next-20151126-sasha-00005-g00d303e-dirty #2653
>>>> [ 413.547614] Call Trace:
>>>> [ 413.548077] [<ffffffffa8e6b5bb>] dump_stack+0x72/0xb7
>>>> [ 413.548765] [<ffffffffa73531d3>]
>>>> warn_slowpath_common+0x113/0x140
>>>> [ 413.551151] [<ffffffffa73532cb>] warn_slowpath_fmt+0xcb/0x100
>>>> [ 413.554295] [<ffffffffa8ed0194>]
>>>> debug_print_object+0x1c4/0x1e0
>>>> [ 413.556592] [<ffffffffa8ed1035>]
>>>> __debug_check_no_obj_freed+0x215/0x7a0
>>>> [ 413.560526] [<ffffffffa8ed2b6c>]
>>>> debug_check_no_obj_freed+0x2c/0x40
>>>> [ 413.561328] [<ffffffffa77aac4c>] kfree+0x1fc/0x2f0
>
> This should be
> kfree(cs->hw.ser)
>
> Note that cs->hw is a union of struct base_cardstate, struct
> ser_cardstate, and struct usb_cardstate. And it's obvious that struct
> ser_cardstate is much smaller that struct bas_cardstate. So we're
> probably free-ing some memory that, so to speak, includes
> bas_cardstate.timer_int_in, a struct timer_list. That's likely way
> beyond the end of struct ser_cardstate and so it should still contain
> garbage.
>
>>>> [ 413.561970] [<ffffffffae74b021>] gigaset_freecshw+0xe1/0x120
>>>> [ 413.562723] [<ffffffffae70669d>] gigaset_freecs+0x2ad/0x600
>>>> [ 413.564240] [<ffffffffae74ba60>] gigaset_tty_close+0x210/0x280
>>>> [ 413.565774] [<ffffffffa95ba6f2>]
>>>> tty_ldisc_close.isra.1+0xc2/0xd0
>>>> [ 413.566550] [<ffffffffa95ba81b>] tty_ldisc_kill+0x4b/0x170
>>>> [ 413.567253] [<ffffffffa95bbba3>] tty_ldisc_release+0x183/0x240
>>>> [ 413.568000] [<ffffffffa95a507c>] tty_release+0xd1c/0xe80
>>>> [ 413.570176] [<ffffffffa78182fa>] __fput+0x32a/0x680
>>>> [ 413.570888] [<ffffffffa78186da>] ____fput+0x1a/0x20
>>>> [ 413.571565] [<ffffffffa73adf5c>] task_work_run+0x19c/0x1e0
>>>> [ 413.572290] [<ffffffffa735cae7>] do_exit+0xdf7/0x28f0
>>>> [ 413.576188] [<ffffffffa735e805>] do_group_exit+0x1b5/0x300
>>>> [ 413.576905] [<ffffffffa7382222>] get_signal+0x1182/0x1360
>>>> [ 413.577627] [<ffffffffa717b553>] do_signal+0x93/0x1690
>>>> [ 413.584630] [<ffffffffa70063b0>]
>>>> exit_to_usermode_loop+0xc0/0x1e0
>>>> [ 413.585412] [<ffffffffa7007feb>]
>>>> prepare_exit_to_usermode+0x10b/0x140
>>>> [ 413.586187] [<ffffffffb0a12c3e>] retint_user+0x8/0x23
>>>
>
> I have no idea (yet) what triggers retint_user.
>
> Anyhow, my first hunch is to do a s/kmalloc/kzalloc/ on
> drv->cs = kmalloc(minors * sizeof *drv->cs, GFP_KERNEL)
>
> in drivers/isdn/gigaset/common.c and see if this still triggers. But to
> test that I need to know how to reproduce this.
>
> To be continued...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists