Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: yVk+BXoWEKySZq9ivtuj/KdXb21/27D9Vwm6HmH3ynf0N+WYgMB0SOcqgiNu1P8NF8lmAGSFvj
 X6AW/p2bt99XOFwcANQ3ciKzSUasZOmY7SAfEHc2DW6d9V3Etdrie2vMV9Tie5IMRWNaH7lC6e
 /VfkXdM79jnuXm03u55XfbGH740M+Mj8TchuCTsRNuCfhe8vZdt7f4D0irLSm2KzVm3zkBTCtn
 wSzLldDB3TFFqU9J3A1XYS7LMG1PmemB6c3zFNysvx59IupFuXsoh2oRACWQMJEaoZ3Q93sZqQ
 5YE=
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
Thread-Topic: [PATCH v4 1/2] binfmt_flat: allow not offsetting data start
Thread-Index: AQHXMyZv8axGA7lx3kGvgyyYuv6fjQ==
Date:   Thu, 22 Apr 2021 12:00:32 +0000
Message-ID: <BL0PR04MB651434FAFF5566A575FDBE76E7469@BL0PR04MB6514.namprd04.prod.outlook.com>
References: <20210417011009.54569-1-damien.lemoal@wdc.com>
 <20210417011009.54569-2-damien.lemoal@wdc.com>
 <5227b984-f415-98b5-dae8-0cf84a71bb46@linux-m68k.org>
 <BL0PR04MB651453DC7A8BE4E050B11893E74B9@BL0PR04MB6514.namprd04.prod.outlook.com>
 <b83b023e-abab-35d2-22c7-5d259af2ac97@linux-m68k.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7d8be2b6-81f7-429a-e24d-08d905863af9
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Apr 2021 12:00:32.1921
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: VNM3GJJVzmopOUPONv+Sx41lKiRu87/pyfm9P1g7Ut+H76E5hWwbw+6JpqUsDmtBWkdG+z6LHhgrBeufFygZ/g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR04MB4450
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2021/04/18 11:38, Greg Ungerer wrote:=0A=
> =0A=
> =0A=
> On 17/4/21 2:54 pm, Damien Le Moal wrote:=0A=
>> On 2021/04/17 13:52, Greg Ungerer wrote:=0A=
>>>=0A=
>>> On 17/4/21 11:10 am, Damien Le Moal wrote:=0A=
>>>> Commit 2217b9826246 ("binfmt_flat: revert "binfmt_flat: don't offset=
=0A=
>>>> the data start"") restored offsetting the start of the data section by=
=0A=
>>>> a number of words defined by MAX_SHARED_LIBS. As a result, since=0A=
>>>> MAX_SHARED_LIBS is never 0, a gap between the text and data sections=
=0A=
>>>> always exists. For architectures which cannot support a such gap=0A=
>>>> between the text and data sections (e.g. riscv nommu), flat binary=0A=
>>>> programs cannot be executed.=0A=
>>>>=0A=
>>>> To allow an architecture to request no data start offset to allow for=
=0A=
>>>> contiguous text and data sections for binaries flagged with=0A=
>>>> FLAT_FLAG_RAM, introduce the new config option=0A=
>>>> CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET. Using this new option, the=0A=
>>>> macro DATA_START_OFFSET_WORDS is conditionally defined in binfmt_flat.=
c=0A=
>>>> to MAX_SHARED_LIBS for architectures tolerating or needing the data=0A=
>>>> start offset (CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET disabled case)=
=0A=
>>>> and to 0 when CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET is enabled.=0A=
>>>> DATA_START_OFFSET_WORDS is used in load_flat_file() to calculate the=
=0A=
>>>> data section length and start position.=0A=
>>>>=0A=
>>>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>=0A=
>>>> ---=0A=
>>>>    fs/Kconfig.binfmt |  3 +++=0A=
>>>>    fs/binfmt_flat.c  | 19 ++++++++++++++-----=0A=
>>>>    2 files changed, 17 insertions(+), 5 deletions(-)=0A=
>>>>=0A=
>>>> diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt=0A=
>>>> index c6f1c8c1934e..06fb7a93a1bd 100644=0A=
>>>> --- a/fs/Kconfig.binfmt=0A=
>>>> +++ b/fs/Kconfig.binfmt=0A=
>>>> @@ -112,6 +112,9 @@ config BINFMT_FLAT_ARGVP_ENVP_ON_STACK=0A=
>>>>    config BINFMT_FLAT_OLD_ALWAYS_RAM=0A=
>>>>    	bool=0A=
>>>>    =0A=
>>>> +config BINFMT_FLAT_NO_DATA_START_OFFSET=0A=
>>>> +	bool=0A=
>>>> +=0A=
>>>>    config BINFMT_FLAT_OLD=0A=
>>>>    	bool "Enable support for very old legacy flat binaries"=0A=
>>>>    	depends on BINFMT_FLAT=0A=
>>>> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c=0A=
>>>> index b9c658e0548e..1dc68dfba3e0 100644=0A=
>>>> --- a/fs/binfmt_flat.c=0A=
>>>> +++ b/fs/binfmt_flat.c=0A=
>>>> @@ -74,6 +74,12 @@=0A=
>>>>    #define	MAX_SHARED_LIBS			(1)=0A=
>>>>    #endif=0A=
>>>>    =0A=
>>>> +#ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET=0A=
>>>> +#define DATA_START_OFFSET_WORDS		(0)=0A=
>>>> +#else=0A=
>>>> +#define DATA_START_OFFSET_WORDS		(MAX_SHARED_LIBS)=0A=
>>>> +#endif=0A=
>>>> +=0A=
>>>>    struct lib_info {=0A=
>>>>    	struct {=0A=
>>>>    		unsigned long start_code;		/* Start of text segment */=0A=
>>>> @@ -560,6 +566,7 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    	 * it all together.=0A=
>>>>    	 */=0A=
>>>>    	if (!IS_ENABLED(CONFIG_MMU) && !(flags & (FLAT_FLAG_RAM|FLAT_FLAG_=
GZIP))) {=0A=
>>>> +=0A=
>>>=0A=
>>> Random white space change...=0A=
>>> Don't worry about re-spinning though, I will just edit this chunk out.=
=0A=
>>=0A=
>> Oops. Sorry about that. I should have better checked :)=0A=
>>=0A=
>>>=0A=
>>>=0A=
>>>>    		/*=0A=
>>>>    		 * this should give us a ROM ptr,  but if it doesn't we don't=0A=
>>>>    		 * really care=0A=
>>>> @@ -576,7 +583,8 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    			goto err;=0A=
>>>>    		}=0A=
>>>>    =0A=
>>>> -		len =3D data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long);=
=0A=
>>>> +		len =3D data_len + extra +=0A=
>>>> +			DATA_START_OFFSET_WORDS * sizeof(unsigned long);=0A=
>>>>    		len =3D PAGE_ALIGN(len);=0A=
>>>>    		realdatastart =3D vm_mmap(NULL, 0, len,=0A=
>>>>    			PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, 0);=0A=
>>>> @@ -591,7 +599,7 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    			goto err;=0A=
>>>>    		}=0A=
>>>>    		datapos =3D ALIGN(realdatastart +=0A=
>>>> -				MAX_SHARED_LIBS * sizeof(unsigned long),=0A=
>>>> +				DATA_START_OFFSET_WORDS * sizeof(unsigned long),=0A=
>>>>    				FLAT_DATA_ALIGN);=0A=
>>>>    =0A=
>>>>    		pr_debug("Allocated data+bss+stack (%u bytes): %lx\n",=0A=
>>>> @@ -622,7 +630,8 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    		memp_size =3D len;=0A=
>>>>    	} else {=0A=
>>>>    =0A=
>>>> -		len =3D text_len + data_len + extra + MAX_SHARED_LIBS * sizeof(u32)=
;=0A=
>>>> +		len =3D text_len + data_len + extra +=0A=
>>>> +			DATA_START_OFFSET_WORDS * sizeof(u32);=0A=
>>>>    		len =3D PAGE_ALIGN(len);=0A=
>>>>    		textpos =3D vm_mmap(NULL, 0, len,=0A=
>>>>    			PROT_READ | PROT_EXEC | PROT_WRITE, MAP_PRIVATE, 0);=0A=
>>>> @@ -638,7 +647,7 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    =0A=
>>>>    		realdatastart =3D textpos + ntohl(hdr->data_start);=0A=
>>>>    		datapos =3D ALIGN(realdatastart +=0A=
>>>> -				MAX_SHARED_LIBS * sizeof(u32),=0A=
>>>> +				DATA_START_OFFSET_WORDS * sizeof(u32),=0A=
>>>>    				FLAT_DATA_ALIGN);=0A=
>>>>    =0A=
>>>>    		reloc =3D (__be32 __user *)=0A=
>>>> @@ -714,7 +723,7 @@ static int load_flat_file(struct linux_binprm *bpr=
m,=0A=
>>>>    			ret =3D result;=0A=
>>>>    			pr_err("Unable to read code+data+bss, errno %d\n", ret);=0A=
>>>>    			vm_munmap(textpos, text_len + data_len + extra +=0A=
>>>> -				MAX_SHARED_LIBS * sizeof(u32));=0A=
>>>> +				  DATA_START_OFFSET_WORDS * sizeof(u32));=0A=
>>>>    			goto err;=0A=
>>>>    		}=0A=
>>>>    	}=0A=
>>>>=0A=
>>>=0A=
>>> Thanks, otherwise looks good.=0A=
>>>=0A=
>>> Acked-by: Greg Ungerer <gerg@linux-m68k.org>=0A=
>>>=0A=
>>> I will push this into my m68knommu tree, for-next branch.=0A=
>>> I just carry the flat format changes in that tree now to make my life e=
asier.=0A=
>>=0A=
>> Great. Thanks !=0A=
>> Are you taking both patches or should Plamer take the riscv Kconfig chan=
ge=0A=
>> through his tree ?=0A=
> =0A=
> I am happy to take both.=0A=
> Palmer?=0A=
=0A=
Palmer,=0A=
=0A=
Ping !=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=