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 X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A2B9C433ED for ; Sat, 17 Apr 2021 04:52:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6960360FE8 for ; Sat, 17 Apr 2021 04:52:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229737AbhDQEwj (ORCPT ); Sat, 17 Apr 2021 00:52:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:53032 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229547AbhDQEwf (ORCPT ); Sat, 17 Apr 2021 00:52:35 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 9EB4860FE8; Sat, 17 Apr 2021 04:52:07 +0000 (UTC) Subject: Re: [PATCH v4 1/2] binfmt_flat: allow not offsetting data start To: Damien Le Moal , uclinux-dev@uclinux.org, Palmer Dabbelt , linux-riscv@lists.infradead.org Cc: linux-kernel@vger.kernel.org, Anup Patel , Christoph Hellwig References: <20210417011009.54569-1-damien.lemoal@wdc.com> <20210417011009.54569-2-damien.lemoal@wdc.com> From: Greg Ungerer Message-ID: <5227b984-f415-98b5-dae8-0cf84a71bb46@linux-m68k.org> Date: Sat, 17 Apr 2021 14:52:05 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210417011009.54569-2-damien.lemoal@wdc.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17/4/21 11:10 am, Damien Le Moal wrote: > Commit 2217b9826246 ("binfmt_flat: revert "binfmt_flat: don't offset > the data start"") restored offsetting the start of the data section by > a number of words defined by MAX_SHARED_LIBS. As a result, since > MAX_SHARED_LIBS is never 0, a gap between the text and data sections > always exists. For architectures which cannot support a such gap > between the text and data sections (e.g. riscv nommu), flat binary > programs cannot be executed. > > To allow an architecture to request no data start offset to allow for > contiguous text and data sections for binaries flagged with > FLAT_FLAG_RAM, introduce the new config option > CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET. Using this new option, the > macro DATA_START_OFFSET_WORDS is conditionally defined in binfmt_flat.c > to MAX_SHARED_LIBS for architectures tolerating or needing the data > start offset (CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET disabled case) > and to 0 when CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET is enabled. > DATA_START_OFFSET_WORDS is used in load_flat_file() to calculate the > data section length and start position. > > Signed-off-by: Damien Le Moal > --- > fs/Kconfig.binfmt | 3 +++ > fs/binfmt_flat.c | 19 ++++++++++++++----- > 2 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index c6f1c8c1934e..06fb7a93a1bd 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -112,6 +112,9 @@ config BINFMT_FLAT_ARGVP_ENVP_ON_STACK > config BINFMT_FLAT_OLD_ALWAYS_RAM > bool > > +config BINFMT_FLAT_NO_DATA_START_OFFSET > + bool > + > config BINFMT_FLAT_OLD > bool "Enable support for very old legacy flat binaries" > depends on BINFMT_FLAT > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > index b9c658e0548e..1dc68dfba3e0 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -74,6 +74,12 @@ > #define MAX_SHARED_LIBS (1) > #endif > > +#ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET > +#define DATA_START_OFFSET_WORDS (0) > +#else > +#define DATA_START_OFFSET_WORDS (MAX_SHARED_LIBS) > +#endif > + > struct lib_info { > struct { > unsigned long start_code; /* Start of text segment */ > @@ -560,6 +566,7 @@ static int load_flat_file(struct linux_binprm *bprm, > * it all together. > */ > if (!IS_ENABLED(CONFIG_MMU) && !(flags & (FLAT_FLAG_RAM|FLAT_FLAG_GZIP))) { > + Random white space change... Don't worry about re-spinning though, I will just edit this chunk out. > /* > * this should give us a ROM ptr, but if it doesn't we don't > * really care > @@ -576,7 +583,8 @@ static int load_flat_file(struct linux_binprm *bprm, > goto err; > } > > - len = data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long); > + len = data_len + extra + > + DATA_START_OFFSET_WORDS * sizeof(unsigned long); > len = PAGE_ALIGN(len); > realdatastart = vm_mmap(NULL, 0, len, > PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, 0); > @@ -591,7 +599,7 @@ static int load_flat_file(struct linux_binprm *bprm, > goto err; > } > datapos = ALIGN(realdatastart + > - MAX_SHARED_LIBS * sizeof(unsigned long), > + DATA_START_OFFSET_WORDS * sizeof(unsigned long), > FLAT_DATA_ALIGN); > > pr_debug("Allocated data+bss+stack (%u bytes): %lx\n", > @@ -622,7 +630,8 @@ static int load_flat_file(struct linux_binprm *bprm, > memp_size = len; > } else { > > - len = text_len + data_len + extra + MAX_SHARED_LIBS * sizeof(u32); > + len = text_len + data_len + extra + > + DATA_START_OFFSET_WORDS * sizeof(u32); > len = PAGE_ALIGN(len); > textpos = vm_mmap(NULL, 0, len, > PROT_READ | PROT_EXEC | PROT_WRITE, MAP_PRIVATE, 0); > @@ -638,7 +647,7 @@ static int load_flat_file(struct linux_binprm *bprm, > > realdatastart = textpos + ntohl(hdr->data_start); > datapos = ALIGN(realdatastart + > - MAX_SHARED_LIBS * sizeof(u32), > + DATA_START_OFFSET_WORDS * sizeof(u32), > FLAT_DATA_ALIGN); > > reloc = (__be32 __user *) > @@ -714,7 +723,7 @@ static int load_flat_file(struct linux_binprm *bprm, > ret = result; > pr_err("Unable to read code+data+bss, errno %d\n", ret); > vm_munmap(textpos, text_len + data_len + extra + > - MAX_SHARED_LIBS * sizeof(u32)); > + DATA_START_OFFSET_WORDS * sizeof(u32)); > goto err; > } > } > Thanks, otherwise looks good. Acked-by: Greg Ungerer I will push this into my m68knommu tree, for-next branch. I just carry the flat format changes in that tree now to make my life easier. Regards Greg