From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Greg Ungerer <gerg@linux-m68k.org>,
"uclinux-dev@uclinux.org" <uclinux-dev@uclinux.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
"linux-riscv@lists.infradead.org"
<linux-riscv@lists.infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Anup Patel <Anup.Patel@wdc.com>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v4 1/2] binfmt_flat: allow not offsetting data start
Date: Sat, 17 Apr 2021 04:54:48 +0000 [thread overview]
Message-ID: <BL0PR04MB651453DC7A8BE4E050B11893E74B9@BL0PR04MB6514.namprd04.prod.outlook.com> (raw)
In-Reply-To: 5227b984-f415-98b5-dae8-0cf84a71bb46@linux-m68k.org
On 2021/04/17 13:52, Greg Ungerer wrote:
>
> 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 <damien.lemoal@wdc.com>
>> ---
>> 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.
Oops. Sorry about that. I should have better checked :)
>
>
>> /*
>> * 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 <gerg@linux-m68k.org>
>
> 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.
Great. Thanks !
Are you taking both patches or should Plamer take the riscv Kconfig change
through his tree ?
>
> Regards
> Greg
>
>
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2021-04-17 4:54 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-17 1:10 [PATCH v4 0/2] Fix binfmt_flat loader for RISC-V Damien Le Moal
2021-04-17 1:10 ` [PATCH v4 1/2] binfmt_flat: allow not offsetting data start Damien Le Moal
2021-04-17 4:52 ` Greg Ungerer
2021-04-17 4:54 ` Damien Le Moal [this message]
2021-04-18 2:38 ` Greg Ungerer
2021-04-22 12:00 ` Damien Le Moal
2021-04-23 4:04 ` Palmer Dabbelt
2021-04-23 5:38 ` Greg Ungerer
2021-04-23 6:44 ` Palmer Dabbelt
2021-04-17 1:10 ` [PATCH v4 2/2] riscv: Disable data start offset in flat binaries Damien Le Moal
2021-04-17 4:56 ` Greg Ungerer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BL0PR04MB651453DC7A8BE4E050B11893E74B9@BL0PR04MB6514.namprd04.prod.outlook.com \
--to=damien.lemoal@wdc.com \
--cc=Anup.Patel@wdc.com \
--cc=gerg@linux-m68k.org \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=uclinux-dev@uclinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).