Thread (25 messages) 25 messages, 5 authors, 2015-12-07

Re: gigaset: freeing an active object

From: Peter Hurley <hidden>
Date: 2015-11-28 01:20:32
Also in: lkml

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:
quoted
On 11/27/2015 12:57 PM, Paul Bolle wrote:
quoted
On vr, 2015-11-27 at 10:19 -0500, Sasha Levin wrote:
quoted
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.
quoted
quoted
quoted
[  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.
quoted
quoted
quoted
[  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.
quoted
quoted
quoted
[  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.
quoted
quoted
quoted
[  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...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help