netdev - Re: gigaset: freeing an active object

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1448670517.6523.67.camel@tiscali.nl>
Date:	Sat, 28 Nov 2015 01:28:37 +0100
From:	Paul Bolle <pebolle@...cali.nl>
To:	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

(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)

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...


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html