From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 55563C27C79 for ; Mon, 17 Jun 2024 15:52:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=w+91/Cqh5QZLzG3EVqhdPfjbo2kSMf0h+Es3cy/BtcE=; b=MRlSAZgW26wecuEndvr79rUKiF FrucwhsKduPo6W4VWax7Aa6C+WKKlR9pynF3ZU1Dram7T8xKIo0iQ6qnFfvV5Q6SICyzdYM7SQI6n 8KqbtBkLsXHBCQOtRrKV7JGI1RO+ID1JkixlBh+JV4+6FC0Ebk7sgk4EExVl8Dw7/C6k+ENA+iJp/ +oFTL6s4Dg9n2ZHSWDSMyxUdTBlkWX++tUc89BDAyxZ0Hu1dKwiP0XUVGzA+2JX6plebISFvtWPvR ab+W2topvFisxL1UZ1mJYbYbQ7dfAZanlRPT0VbBABUVYUqMgqsZt/XXR/TE/5ymFrDNN1HJ0UAmT dsRzgyRw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sJEew-0000000BaT1-4AoP; Mon, 17 Jun 2024 15:52:43 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sJEeu-0000000BaRV-1Geo for linux-arm-kernel@lists.infradead.org; Mon, 17 Jun 2024 15:52:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0CDA7DA7; Mon, 17 Jun 2024 08:53:03 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6C1F13F64C; Mon, 17 Jun 2024 08:52:34 -0700 (PDT) Date: Mon, 17 Jun 2024 16:52:15 +0100 From: Mark Rutland To: Yuntao Liu Cc: x86@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org, linux-hardening@vger.kernel.org, catalin.marinas@arm.com, will@kernel.org, hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, kees@kernel.org, gustavoars@kernel.org, arnd@arndb.de, leobras@redhat.com, broonie@kernel.org, imbrenda@linux.ibm.com, pawan.kumar.gupta@linux.intel.com Subject: Re: [PATCH] remove AND operation in choose_random_kstack_offset() Message-ID: References: <20240617133721.377540-1-liuyuntao12@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240617133721.377540-1-liuyuntao12@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240617_085240_619827_BE54BBC5 X-CRM114-Status: GOOD ( 18.82 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jun 17, 2024 at 01:37:21PM +0000, Yuntao Liu wrote: > Since the offset would be bitwise ANDed with 0x3FF in > add_random_kstack_offset(), so just remove AND operation here. > > Signed-off-by: Yuntao Liu The comments in arm64 and x86 say that they're deliberately capping the offset at fewer bits than the result of KSTACK_OFFSET_MAX() masking the value with 0x3FF. Maybe it's ok to expand that, but if that's the case the commit message needs to explain why it's safe add extra bits (2 on arm64, 3 on s39 and x86), and those comments need to be updated accordingly. As-is, I do not think this patch is ok. Mark. > --- > arch/arm64/kernel/syscall.c | 2 +- > arch/s390/include/asm/entry-common.h | 2 +- > arch/x86/include/asm/entry-common.h | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c > index ad198262b981..43f555f7cd2d 100644 > --- a/arch/arm64/kernel/syscall.c > +++ b/arch/arm64/kernel/syscall.c > @@ -63,7 +63,7 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, > * > * The resulting 5 bits of entropy is seen in SP[8:4]. > */ > - choose_random_kstack_offset(get_random_u16() & 0x1FF); > + choose_random_kstack_offset(get_random_u16()); > } > > static inline bool has_syscall_work(unsigned long flags) > diff --git a/arch/s390/include/asm/entry-common.h b/arch/s390/include/asm/entry-common.h > index 7f5004065e8a..35555c944630 100644 > --- a/arch/s390/include/asm/entry-common.h > +++ b/arch/s390/include/asm/entry-common.h > @@ -54,7 +54,7 @@ static __always_inline void arch_exit_to_user_mode(void) > static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs, > unsigned long ti_work) > { > - choose_random_kstack_offset(get_tod_clock_fast() & 0xff); > + choose_random_kstack_offset(get_tod_clock_fast()); > } > > #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare > diff --git a/arch/x86/include/asm/entry-common.h b/arch/x86/include/asm/entry-common.h > index 7e523bb3d2d3..b28a307f2014 100644 > --- a/arch/x86/include/asm/entry-common.h > +++ b/arch/x86/include/asm/entry-common.h > @@ -85,7 +85,7 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs, > * Therefore, final stack offset entropy will be 5 (x86_64) or > * 6 (ia32) bits. > */ > - choose_random_kstack_offset(rdtsc() & 0xFF); > + choose_random_kstack_offset(rdtsc()); > } > #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare > > -- > 2.34.1 >