From: Greg Ungerer <gerg@linux-m68k.org>
To: Damien Le Moal <damien.lemoal@wdc.com>,
uclinux-dev@uclinux.org, ugerg@linux-m68k.org,
Palmer Dabbelt <palmer@dabbelt.com>,
linux-riscv@lists.infradead.org,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-kernel@vger.kernel.org
Cc: Max Filippov <jcmvbkbc@gmail.com>,
Anup Patel <anup.patel@wdc.com>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v3 1/2] binfmt_flat: allow not offsetting data start
Date: Fri, 16 Apr 2021 00:04:15 +1000 [thread overview]
Message-ID: <8ff801ad-6e17-fdd2-6186-5443540947a9@linux-m68k.org> (raw)
In-Reply-To: <20210415061502.7248-2-damien.lemoal@wdc.com>
Hi Damien,
On 15/4/21 4:15 pm, 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 contiguous text and data sections,
> introduce the config option CONFIG_BINFMT_FLAT_NO_TEXT_DATA_GAP.
> Using this new option, the macro DATA_GAP_WORDS is conditionally
> defined in binfmt_flat.c to MAX_SHARED_LIBS for architectures
> tolerating the text-to-data gap (CONFIG_BINFMT_FLAT_NO_TEXT_DATA_GAP
> disabled case) and to 0 when CONFIG_BINFMT_FLAT_NO_TEXT_DATA_GAP is
> enabled. DATA_GAP_WORDS is used in load_flat_file() to calculate the
> data section length and start position.
>
> An architecture enabling CONFIG_BINFMT_FLAT_NO_TEXT_DATA_GAP also
> prevents the use of the separate text/data load case (when the flat file
> header flags FLAT_FLAG_RAM and FLAT_FLAG_GZIP are not set with NOMMU
> kernels) and forces the use of a single RAM region for loading
> (equivalent to FLAT_FLAG_RAM being set).
So is it the case that a flat format file on RISC-V will never have
relocations?
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> ---
> fs/Kconfig.binfmt | 3 +++
> fs/binfmt_flat.c | 21 +++++++++++++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
> index c6f1c8c1934e..c6df931d5d45 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_TEXT_DATA_GAP
> + 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..2be29bb964b8 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_TEXT_DATA_GAP
> +#define DATA_GAP_WORDS (0)
> +#else
> +#define DATA_GAP_WORDS (MAX_SHARED_LIBS)
> +#endif
> +> struct lib_info {
> struct {
> unsigned long start_code; /* Start of text segment */
> @@ -559,7 +565,10 @@ static int load_flat_file(struct linux_binprm *bprm,
> * case, and then the fully copied to RAM case which lumps
> * it all together.
> */
> - if (!IS_ENABLED(CONFIG_MMU) && !(flags & (FLAT_FLAG_RAM|FLAT_FLAG_GZIP))) {
> + if (!IS_ENABLED(CONFIG_MMU) &&
> + !IS_ENABLED(CONFIG_BINFMT_FLAT_NO_TEXT_DATA_GAP) &&
If RISC-V flat format files must always be loaded to RAM then why don't
they set the FLAT_FLAG_RAM when compiled/generated?
Regards
Greg
> + !(flags & (FLAT_FLAG_RAM|FLAT_FLAG_GZIP))) {
> +
> /*
> * this should give us a ROM ptr, but if it doesn't we don't
> * really care
> @@ -576,7 +585,7 @@ 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_GAP_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 +600,7 @@ static int load_flat_file(struct linux_binprm *bprm,
> goto err;
> }
> datapos = ALIGN(realdatastart +
> - MAX_SHARED_LIBS * sizeof(unsigned long),
> + DATA_GAP_WORDS * sizeof(unsigned long),
> FLAT_DATA_ALIGN);
>
> pr_debug("Allocated data+bss+stack (%u bytes): %lx\n",
> @@ -622,7 +631,7 @@ 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_GAP_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_GAP_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_GAP_WORDS * sizeof(u32));
> goto err;
> }
> }
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-04-15 14:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-15 6:15 [PATCH v3 0/2] Fix binfmt_flat loader for RISC-V Damien Le Moal
2021-04-15 6:15 ` [PATCH v3 1/2] binfmt_flat: allow not offsetting data start Damien Le Moal
2021-04-15 14:04 ` Greg Ungerer [this message]
2021-04-15 23:22 ` Damien Le Moal
2021-04-16 7:24 ` Greg Ungerer
2021-04-16 7:35 ` Damien Le Moal
2021-04-15 6:15 ` [PATCH v3 2/2] riscv: Disable text-data gap in flat binaries Damien Le Moal
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=8ff801ad-6e17-fdd2-6186-5443540947a9@linux-m68k.org \
--to=gerg@linux-m68k.org \
--cc=anup.patel@wdc.com \
--cc=damien.lemoal@wdc.com \
--cc=hch@lst.de \
--cc=jcmvbkbc@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=uclinux-dev@uclinux.org \
--cc=ugerg@linux-m68k.org \
--cc=viro@zeniv.linux.org.uk \
/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).