From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753064AbcBJSMO (ORCPT ); Wed, 10 Feb 2016 13:12:14 -0500 Received: from mail-pa0-f53.google.com ([209.85.220.53]:32894 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752115AbcBJSMN (ORCPT ); Wed, 10 Feb 2016 13:12:13 -0500 Subject: Re: [PATCH] arm64: use raw_smp_processor_id in stack backtrace dump To: Will Deacon , James Morse References: <1455053182-31404-1-git-send-email-yang.shi@linaro.org> <20160210102939.GD1052@arm.com> <56BB247F.6040202@arm.com> <20160210121030.GH1052@arm.com> Cc: catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linaro-kernel@lists.linaro.org From: "Shi, Yang" Message-ID: <56BB7D7B.4060002@linaro.org> Date: Wed, 10 Feb 2016 10:12:11 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160210121030.GH1052@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/10/2016 4:10 AM, Will Deacon wrote: > On Wed, Feb 10, 2016 at 11:52:31AM +0000, James Morse wrote: >> On 10/02/16 10:29, Will Deacon wrote: >>> On Tue, Feb 09, 2016 at 01:26:22PM -0800, Yang Shi wrote: >>>> dump_backtrace may be called in kthread context, which is not bound to a single >>>> cpu, i.e. khungtaskd, then calling smp_processor_id may trigger the below bug >>>> report: >>> >>> If we're preemptible here, it means that our irq_stack_ptr is potentially >>> bogus. Whilst this isn't an issue for kthreads, it does feel like we >>> could make this slightly more robust in the face of potential frame >>> corruption. Maybe just zero the IRQ stack pointer if we're in preemptible >>> context? >> >> Switching between stacks is only valid if we are tracing ourselves while on the >> irq_stack, we should probably prevent it for other tasks too. >> >> Something like (untested): >> --------------------- >> if (tsk == current && in_atomic()) >> irq_stack_ptr = IRQ_STACK_PTR(smp_processor_id()); One follow up question, is it possible to have both tsk != current and on_irq_stack is true at the same time? If it is possible, this may be a problem in unwind_frame called by profile_pc which has tsk being NULL. Thanks, Yang >> else >> irq_stack_ptr = 0; >> --------------------- >> >> This would work when we trace ourselves while on the irq_stack, but break* >> tracing a running task on a remote cpu (khungtaskd doesn't do this). >> >> The same fix would apply to unwind_frame(), we have 'tsk' in both functions. >> >> Thoughts? > > in_atomic is a misnomer: > > https://lwn.net/Articles/274695/ > > ;) > > So we might be better off zeroing the pointer if tsk != current || > preemptible(). But yeah, I think we're in general agreement about this. > > Will >