[PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[Michael Ellerman]

In a subsequent patch we'd like to have something like a strscpy_pad()
implementation usable in prom_init.c.

Currently we have a strcpy() implementation with only one caller, so
convert it into strscpy_pad() and update the caller.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/prom_init.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 523b31685c4c..c18d55f8b951 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -242,13 +242,31 @@ static int __init prom_strcmp(const char *cs, const char *ct)
 	return 0;
 }
 
-static char __init *prom_strcpy(char *dest, const char *src)
+static ssize_t __init prom_strscpy_pad(char *dest, const char *src, size_t n)
 {
-	char *tmp = dest;
+	ssize_t rc;
+	size_t i;
 
-	while ((*dest++ = *src++) != '\0')
-		/* nothing */;
-	return tmp;
+	if (n == 0 || n > INT_MAX)
+		return -E2BIG;
+
+	// Copy up to n bytes
+	for (i = 0; i < n && src[i] != '\0'; i++)
+		dest[i] = src[i];
+
+	rc = i;
+
+	// If we copied all n then we have run out of space for the nul
+	if (rc == n) {
+		// Rewind by one character to ensure nul termination
+		i--;
+		rc = -E2BIG;
+	}
+
+	for (; i < n; i++)
+		dest[i] = '\0';
+
+	return rc;
 }
 
 static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
@@ -2701,7 +2719,7 @@ static void __init flatten_device_tree(void)
 
 	/* Add "phandle" in there, we'll need it */
 	namep = make_room(&mem_start, &mem_end, 16, 1);
-	prom_strcpy(namep, "phandle");
+	prom_strscpy_pad(namep, "phandle", sizeof("phandle"));
 	mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
 
 	/* Build string array */
-- 
2.25.1

[PATCH 2/2] powerpc/prom_init: Pass linux_banner to firmware via option vector 7

[Michael Ellerman]

Pass the value of linux_banner to firmware via option vector 7.

Option vector 7 is described in "LoPAR" Linux on Power Architecture
Reference v2.9, in table B.7 on page 824:

  An ASCII character formatted null terminated string that describes
  the client operating system. The string shall be human readable and
  may be displayed on the console.

The string can be up to 256 bytes total, including the nul terminator.

linux_banner contains lots of information, and should make it possible
to identify the exact kernel version that is running:

  const char linux_banner[] =
  "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
  LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";

For example:
  Linux version 4.15.0-144-generic (buildd@bos02-ppc64el-018) (gcc
  version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)) #148-Ubuntu SMP Sat May 8
  02:32:13 UTC 2021 (Ubuntu 4.15.0-144.148-generic 4.15.18)

It's also printed at boot to the console/dmesg, which should make it
possible to correlate what firmware receives with the console/dmesg on
the machine.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

NB. linux_banner is already allowed by prom_init_check.sh

LoPAR: https://openpowerfoundation.org/?resource_lib=linux-on-power-architecture-reference-a-papr-linux-subset-review-draft
---
 arch/powerpc/kernel/prom_init.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index c18d55f8b951..7343076b261c 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
 #include <linux/initrd.h>
 #include <linux/bitops.h>
 #include <linux/pgtable.h>
+#include <linux/printk.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/page.h>
@@ -944,6 +945,10 @@ struct option_vector6 {
 	u8 os_name;
 } __packed;
 
+struct option_vector7 {
+	u8 os_id[256];
+} __packed;
+
 struct ibm_arch_vec {
 	struct { u32 mask, val; } pvrs[14];
 
@@ -966,6 +971,9 @@ struct ibm_arch_vec {
 
 	u8 vec6_len;
 	struct option_vector6 vec6;
+
+	u8 vec7_len;
+	struct option_vector7 vec7;
 } __packed;
 
 static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
@@ -1112,6 +1120,9 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
 		.secondary_pteg = 0,
 		.os_name = OV6_LINUX,
 	},
+
+	/* option vector 7: OS Identification */
+	.vec7_len = VECTOR_LENGTH(sizeof(struct option_vector7)),
 };
 
 static struct ibm_arch_vec __prombss ibm_architecture_vec  ____cacheline_aligned;
@@ -1340,6 +1351,10 @@ static void __init prom_check_platform_support(void)
 	memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
 	       sizeof(ibm_architecture_vec));
 
+	prom_strscpy_pad(ibm_architecture_vec.vec7.os_id, linux_banner, 256);
+	// Ensure nul termination
+	ibm_architecture_vec.vec7.os_id[255] = '\0';
+
 	if (prop_len > 1) {
 		int i;
 		u8 vec[8];
-- 
2.25.1

Re: [PATCH 2/2] powerpc/prom_init: Pass linux_banner to firmware via option vector 7

[Tyrel Datwyler]

On 6/20/21 11:49 PM, Michael Ellerman wrote:
> Pass the value of linux_banner to firmware via option vector 7.
> 
> Option vector 7 is described in "LoPAR" Linux on Power Architecture
> Reference v2.9, in table B.7 on page 824:
> 
>   An ASCII character formatted null terminated string that describes
>   the client operating system. The string shall be human readable and
>   may be displayed on the console.
> 
> The string can be up to 256 bytes total, including the nul terminator.
> 
> linux_banner contains lots of information, and should make it possible
> to identify the exact kernel version that is running:
> 
>   const char linux_banner[] =
>   "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
>   LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
> 
> For example:
>   Linux version 4.15.0-144-generic (buildd@bos02-ppc64el-018) (gcc
>   version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)) #148-Ubuntu SMP Sat May 8
>   02:32:13 UTC 2021 (Ubuntu 4.15.0-144.148-generic 4.15.18)
> 
> It's also printed at boot to the console/dmesg, which should make it
> possible to correlate what firmware receives with the console/dmesg on
> the machine.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> 
> NB. linux_banner is already allowed by prom_init_check.sh
> 
> LoPAR: https://openpowerfoundation.org/?resource_lib=linux-on-power-architecture-reference-a-papr-linux-subset-review-draft
> ---
>  arch/powerpc/kernel/prom_init.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index c18d55f8b951..7343076b261c 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -27,6 +27,7 @@
>  #include <linux/initrd.h>
>  #include <linux/bitops.h>
>  #include <linux/pgtable.h>
> +#include <linux/printk.h>
>  #include <asm/prom.h>
>  #include <asm/rtas.h>
>  #include <asm/page.h>
> @@ -944,6 +945,10 @@ struct option_vector6 {
>  	u8 os_name;
>  } __packed;
> 
> +struct option_vector7 {
> +	u8 os_id[256];
> +} __packed;
> +
>  struct ibm_arch_vec {
>  	struct { u32 mask, val; } pvrs[14];
> 
> @@ -966,6 +971,9 @@ struct ibm_arch_vec {
> 
>  	u8 vec6_len;
>  	struct option_vector6 vec6;
> +
> +	u8 vec7_len;
> +	struct option_vector7 vec7;
>  } __packed;
> 
>  static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
> @@ -1112,6 +1120,9 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
>  		.secondary_pteg = 0,
>  		.os_name = OV6_LINUX,
>  	},
> +
> +	/* option vector 7: OS Identification */
> +	.vec7_len = VECTOR_LENGTH(sizeof(struct option_vector7)),
>  };
> 
>  static struct ibm_arch_vec __prombss ibm_architecture_vec  ____cacheline_aligned;
> @@ -1340,6 +1351,10 @@ static void __init prom_check_platform_support(void)
>  	memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
>  	       sizeof(ibm_architecture_vec));
> 
> +	prom_strscpy_pad(ibm_architecture_vec.vec7.os_id, linux_banner, 256);
> +	// Ensure nul termination
> +	ibm_architecture_vec.vec7.os_id[255] = '\0';
> +

Doesn't the implementation of prom_strscpy_pad() in patch 1 ensure nul termination?

-Tyrel

>  	if (prop_len > 1) {
>  		int i;
>  		u8 vec[8];
> 

Re: [PATCH 2/2] powerpc/prom_init: Pass linux_banner to firmware via option vector 7

[Michael Ellerman]

Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 6/20/21 11:49 PM, Michael Ellerman wrote:
>> Pass the value of linux_banner to firmware via option vector 7.
>> 
>> Option vector 7 is described in "LoPAR" Linux on Power Architecture
>> Reference v2.9, in table B.7 on page 824:
>> 
>>   An ASCII character formatted null terminated string that describes
>>   the client operating system. The string shall be human readable and
>>   may be displayed on the console.
>> 
>> The string can be up to 256 bytes total, including the nul terminator.
>> 
>> linux_banner contains lots of information, and should make it possible
>> to identify the exact kernel version that is running:
>> 
>>   const char linux_banner[] =
>>   "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
>>   LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
>> 
>> For example:
>>   Linux version 4.15.0-144-generic (buildd@bos02-ppc64el-018) (gcc
>>   version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)) #148-Ubuntu SMP Sat May 8
>>   02:32:13 UTC 2021 (Ubuntu 4.15.0-144.148-generic 4.15.18)
>> 
>> It's also printed at boot to the console/dmesg, which should make it
>> possible to correlate what firmware receives with the console/dmesg on
>> the machine.
>> 
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>> 
>> NB. linux_banner is already allowed by prom_init_check.sh
>> 
>> LoPAR: https://openpowerfoundation.org/?resource_lib=linux-on-power-architecture-reference-a-papr-linux-subset-review-draft
>> ---
>>  arch/powerpc/kernel/prom_init.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> index c18d55f8b951..7343076b261c 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
...
>> @@ -1340,6 +1351,10 @@ static void __init prom_check_platform_support(void)
>>  	memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
>>  	       sizeof(ibm_architecture_vec));
>> 
>> +	prom_strscpy_pad(ibm_architecture_vec.vec7.os_id, linux_banner, 256);
>> +	// Ensure nul termination
>> +	ibm_architecture_vec.vec7.os_id[255] = '\0';
>> +
>
> Doesn't the implementation of prom_strscpy_pad() in patch 1 ensure nul termination?

Yes! I was originally using strncpy(), but forgot to drop this when I
switched to strscpy_pad(). I dropped it when applying.

Thanks for reviewing.

cheers

Re: [PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[Daniel Axtens]

Hi

> -static char __init *prom_strcpy(char *dest, const char *src)
> +static ssize_t __init prom_strscpy_pad(char *dest, const char *src, size_t n)
>  {
> -	char *tmp = dest;
> +	ssize_t rc;
> +	size_t i;
>  
> -	while ((*dest++ = *src++) != '\0')
> -		/* nothing */;
> -	return tmp;
> +	if (n == 0 || n > INT_MAX)
> +		return -E2BIG;
> +
> +	// Copy up to n bytes
> +	for (i = 0; i < n && src[i] != '\0'; i++)
> +		dest[i] = src[i];
> +
> +	rc = i;
> +
> +	// If we copied all n then we have run out of space for the nul
> +	if (rc == n) {
> +		// Rewind by one character to ensure nul termination
> +		i--;
> +		rc = -E2BIG;
> +	}
> +
> +	for (; i < n; i++)
> +		dest[i] = '\0';
> +
> +	return rc;
>  }
>  

This implementation seems good to me.

I copied it into a new C file and added the following:

int main() {
	char longstr[255]="abcdefghijklmnopqrstuvwxyz";
	char shortstr[5];
	assert(prom_strscpy_pad(longstr, "", 0) == -E2BIG);
	assert(prom_strscpy_pad(longstr, "hello", 255) == 5);
	assert(prom_strscpy_pad(shortstr, "hello", 5) == -E2BIG);
	assert(memcmp(shortstr, "hell", 5) == 0);
	assert(memcmp(longstr, "hello\0\0\0\0\0\0\0\0\0", 6) == 0);
	return 0;
}

All the assertions pass. I believe this covers all the conditions from
the strscpy_pad docstring.

Reviewed-by: Daniel Axtens <dja@axtens.net>

Kind regards,
Daniel

>  static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> @@ -2701,7 +2719,7 @@ static void __init flatten_device_tree(void)
>  
>  	/* Add "phandle" in there, we'll need it */
>  	namep = make_room(&mem_start, &mem_end, 16, 1);
> -	prom_strcpy(namep, "phandle");
> +	prom_strscpy_pad(namep, "phandle", sizeof("phandle"));
>  	mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>  
>  	/* Build string array */
> -- 
> 2.25.1

Re: [PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:
> Hi
>
>> -static char __init *prom_strcpy(char *dest, const char *src)
>> +static ssize_t __init prom_strscpy_pad(char *dest, const char *src, size_t n)
>>  {
>> -	char *tmp = dest;
>> +	ssize_t rc;
>> +	size_t i;
>>  
>> -	while ((*dest++ = *src++) != '\0')
>> -		/* nothing */;
>> -	return tmp;
>> +	if (n == 0 || n > INT_MAX)
>> +		return -E2BIG;
>> +
>> +	// Copy up to n bytes
>> +	for (i = 0; i < n && src[i] != '\0'; i++)
>> +		dest[i] = src[i];
>> +
>> +	rc = i;
>> +
>> +	// If we copied all n then we have run out of space for the nul
>> +	if (rc == n) {
>> +		// Rewind by one character to ensure nul termination
>> +		i--;
>> +		rc = -E2BIG;
>> +	}
>> +
>> +	for (; i < n; i++)
>> +		dest[i] = '\0';
>> +
>> +	return rc;
>>  }
>>  
>
> This implementation seems good to me.
>
> I copied it into a new C file and added the following:
>
> int main() {
> 	char longstr[255]="abcdefghijklmnopqrstuvwxyz";
> 	char shortstr[5];
> 	assert(prom_strscpy_pad(longstr, "", 0) == -E2BIG);
> 	assert(prom_strscpy_pad(longstr, "hello", 255) == 5);
> 	assert(prom_strscpy_pad(shortstr, "hello", 5) == -E2BIG);
> 	assert(memcmp(shortstr, "hell", 5) == 0);
> 	assert(memcmp(longstr, "hello\0\0\0\0\0\0\0\0\0", 6) == 0);
> 	return 0;
> }
>
> All the assertions pass. I believe this covers all the conditions from
> the strscpy_pad docstring.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>

Thanks.

I'll also drop the explicit nul termination in patch 2, which is a
leftover from when I was using strncpy().

cheers

Re: [PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[Tyrel Datwyler]

On 6/21/21 9:11 PM, Michael Ellerman wrote:
> Daniel Axtens <dja@axtens.net> writes:
>> Hi
>>
>>> -static char __init *prom_strcpy(char *dest, const char *src)
>>> +static ssize_t __init prom_strscpy_pad(char *dest, const char *src, size_t n)
>>>  {
>>> -	char *tmp = dest;
>>> +	ssize_t rc;
>>> +	size_t i;
>>>  
>>> -	while ((*dest++ = *src++) != '\0')
>>> -		/* nothing */;
>>> -	return tmp;
>>> +	if (n == 0 || n > INT_MAX)
>>> +		return -E2BIG;
>>> +
>>> +	// Copy up to n bytes
>>> +	for (i = 0; i < n && src[i] != '\0'; i++)
>>> +		dest[i] = src[i];
>>> +
>>> +	rc = i;
>>> +
>>> +	// If we copied all n then we have run out of space for the nul
>>> +	if (rc == n) {
>>> +		// Rewind by one character to ensure nul termination
>>> +		i--;
>>> +		rc = -E2BIG;
>>> +	}
>>> +
>>> +	for (; i < n; i++)
>>> +		dest[i] = '\0';
>>> +
>>> +	return rc;
>>>  }
>>>  
>>
>> This implementation seems good to me.
>>
>> I copied it into a new C file and added the following:
>>
>> int main() {
>> 	char longstr[255]="abcdefghijklmnopqrstuvwxyz";
>> 	char shortstr[5];
>> 	assert(prom_strscpy_pad(longstr, "", 0) == -E2BIG);
>> 	assert(prom_strscpy_pad(longstr, "hello", 255) == 5);
>> 	assert(prom_strscpy_pad(shortstr, "hello", 5) == -E2BIG);
>> 	assert(memcmp(shortstr, "hell", 5) == 0);
>> 	assert(memcmp(longstr, "hello\0\0\0\0\0\0\0\0\0", 6) == 0);
>> 	return 0;
>> }
>>
>> All the assertions pass. I believe this covers all the conditions from
>> the strscpy_pad docstring.
>>
>> Reviewed-by: Daniel Axtens <dja@axtens.net>
> 
> Thanks.
> 
> I'll also drop the explicit nul termination in patch 2, which is a
> leftover from when I was using strncpy().

I guess you can ignore my other email questioning this.

-Tyrel

> 
> cheers
> 

Re: [PATCH 1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()

[Michael Ellerman]

On Mon, 21 Jun 2021 16:49:37 +1000, Michael Ellerman wrote:
> In a subsequent patch we'd like to have something like a strscpy_pad()
> implementation usable in prom_init.c.
> 
> Currently we have a strcpy() implementation with only one caller, so
> convert it into strscpy_pad() and update the caller.

Applied to powerpc/next.

[1/2] powerpc/prom_init: Convert prom_strcpy() into prom_strscpy_pad()
      https://git.kernel.org/powerpc/c/f47d5a4fc254e62ea5af5cbb2fc3e68901def434
[2/2] powerpc/prom_init: Pass linux_banner to firmware via option vector 7
      https://git.kernel.org/powerpc/c/ffaacd97fd37b9f4e825d8107f5cba5470458f0e

cheers