From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752790AbcELKQn (ORCPT ); Thu, 12 May 2016 06:16:43 -0400 Received: from foss.arm.com ([217.140.101.70]:57400 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751535AbcELKQm (ORCPT ); Thu, 12 May 2016 06:16:42 -0400 Date: Thu, 12 May 2016 11:16:46 +0100 From: Will Deacon To: Catalin Marinas Cc: Colin King , AKASHI Takahiro , Lorenzo Pieralisi , Andrew Morton , Janet Liu , Jiri Slaby , Jisheng Zhang , James Morse , Mark Rutland , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, jacob.bramley@arm.com Subject: Re: [PATCH] arm64: do not enforce strict 16 byte alignment to stack pointer Message-ID: <20160512101645.GB25355@arm.com> References: <1462985814-16146-1-git-send-email-colin.king@canonical.com> <20160512092545.GC11226@e104818-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160512092545.GC11226@e104818-lin.cambridge.arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [Adding Jacob] On Thu, May 12, 2016 at 10:25:45AM +0100, Catalin Marinas wrote: > On Wed, May 11, 2016 at 05:56:54PM +0100, Colin King wrote: > > From: Colin Ian King > > > > copy_thread should not be enforcing 16 byte aligment and returning > > -EINVAL. Other architectures trap misaligned stack access with SIGBUS > > so arm64 should follow this convention, so remove the strict enforcement > > check. > > > > For example, currently clone(2) fails with -EINVAL when passing > > a misaligned stack and this gives little clue to what is wrong. Instead, > > it is arguable that a SIGBUS on the fist access to a misaligned stack > > allows one to figure out that it is a misaligned stack issue rather > > than trying to figure out why an unconventional (and undocumented) > > -EINVAL is being returned. > > > > Signed-off-by: Colin Ian King > > --- > > arch/arm64/kernel/process.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > > index 5655f756..8414971 100644 > > --- a/arch/arm64/kernel/process.c > > +++ b/arch/arm64/kernel/process.c > > @@ -258,9 +258,6 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, > > if (stack_start) { > > if (is_compat_thread(task_thread_info(p))) > > childregs->compat_sp = stack_start; > > - /* 16-byte aligned stack mandatory on AArch64 */ > > - else if (stack_start & 15) > > - return -EINVAL; > > else > > childregs->sp = stack_start; > > } > > As we discussed on the linux-man list, I don't expect this change to > break existing working user apps since they pass an aligned stack > already. I really doubt anyone relies on the -EINVAL here. > > That said, I don't think we should add a cc stable (which you haven't > anyway), at least we have a point in time where this change was made. As > the patch stands: > > Acked-by: Catalin Marinas As far as today's mainline is concerned, this looks fine to me. However, there is one nagging issue in that the JavaScript guys have started asking us to relax the strict alignment altogether (there is a bit in the SCTLR register for this). I'm not at all keen on doing this per-process, since context-switching the SCTLR is likely to be slow, so if we were to allow this relaxation it would probably be in the form of a global /sysfs knob or the like. That would then mean that you wouldn't get -EINVAL *or* SIGBUS for a misaligned stack passed to clone(). I think in that case, the options are either (a) return -EINVAL when strict alignment checking is disabled or (b) do nothing and let users debug their broken programs. None of this is a blocker for the patch (which I plan to apply), but food for thought. Will