linux-kernel - Re: BUG: KASAN: slab-out-of-bounds in perf_callchain

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: <20171206143130.GA3367@danjae.aot.lge.com>
Date:   Wed, 6 Dec 2017 23:31:30 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Fengguang Wu <fengguang.wu@...el.com>,
        linux-kernel@...r.kernel.org, Wang Nan <wangnan0@...wei.com>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Will Deacon <will.deacon@....com>, lkp@...org,
        Dmitry Vyukov <dvyukov@...gle.com>, kasan-dev@...glegroups.com,
        kernel-team@....com
Subject: Re: BUG: KASAN: slab-out-of-bounds in perf_callchain_user+0x494/0x530

Hi Peter,

On Wed, Dec 06, 2017 at 02:47:06PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 11:47:18PM +0900, Namhyung Kim wrote:
> > Sure, I mean the following code:
> > 
> > 	mutex_lock(&callchain_mutex);
> > 
> > 	count = atomic_inc_return(&nr_callchain_events);
> > 	if (WARN_ON_ONCE(count < 1)) {
> > 		err = -EINVAL;
> > 		goto exit;
> > 	}
> > 
> > 	if (count > 1) {
> > 		/* If the allocation failed, give up */
> > 		if (!callchain_cpus_entries)
> > 			err = -ENOMEM;
> > 
> > 		goto exit;
> > 	}
> > 
> > 	err = alloc_callchain_buffers();
> > exit:
> > 	if (err)
> > 		atomic_dec(&nr_callchain_events);
> > 
> > 	mutex_unlock(&callchain_mutex);
> > 
> > 
> > The callchain_cpus_entries is allocated in alloc_callchain_buffers()
> > only when the count is 1.  But if it failed to allocate, it decrease
> > the count so next event would try to allocate it again.  Thus it seems
> > not possible to see the callchain_cpus_entries being NULL in the
> > 'if (count > 1)' block.  If you want to make next event give up, it'd
> > need to take an additional count IMHO.
> 
> There's also a race against put_callchain_buffers() there, consider:
> 
> 
> 	get_callchain_buffers()		put_callchain_buffers()
> 	  mutex_lock();
> 	  inc()
> 					  dec_and_test() // false
> 
> 	  dec() // 0
> 
> 
> And the buffers leak.

Hmm.. did you mean that get_callchain_buffers() returns an error?
AFAICS it cannot fail when it sees count > 1 (and callchain_cpus_
entries is allocated).  So I think it won't decrease the count and
should be fine.

Thanks,
Namhyung