linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v10 05/18] powerpc/prom_init: don't use string functions from lib/
       [not found] ` <dc2eb0480f2c87316561435cfcf4c78254d2166b.1552428161.git.christophe.leroy@c-s.fr>
@ 2019-03-12 23:38   ` Daniel Axtens
  2019-04-26 16:24     ` Christophe Leroy
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Axtens @ 2019-03-12 23:38 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-kernel, linuxppc-dev, kasan-dev, linux-mm

Hi Christophe,

In trying to extend my KASAN implementation to Book3S 64bit, I found one
other change needed to prom_init. I don't know if you think it should go
in this patch, the next one, or somewhere else entirely - I will leave
it up to you. Just let me know if you want me to carry it separately.

Thanks again for all your work on this and the integration of my series.

Regards,
Daniel

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 7017156168e8..cebb3fc535ba 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1265,7 +1265,8 @@ static void __init prom_check_platform_support(void)
                                       "ibm,arch-vec-5-platform-support");
 
        /* First copy the architecture vec template */
-       ibm_architecture_vec = ibm_architecture_vec_template;
+       memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
+              sizeof(struct ibm_arch_vec));
 
        if (prop_len > 1) {
                int i;

> When KASAN is active, the string functions in lib/ are doing the
> KASAN checks. This is too early for prom_init.
>
> This patch implements dedicated string functions for prom_init,
> which will be compiled in with KASAN disabled.
>
> Size of prom_init before the patch:
>    text	   data	    bss	    dec	    hex	filename
>   12060	    488	   6960	  19508	   4c34	arch/powerpc/kernel/prom_init.o
>
> Size of prom_init after the patch:
>    text	   data	    bss	    dec	    hex	filename
>   12460	    488	   6960	  19908	   4dc4	arch/powerpc/kernel/prom_init.o
>
> This increases the size of prom_init a bit, but as prom_init is
> in __init section, it is freed after boot anyway.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/kernel/prom_init.c        | 211 ++++++++++++++++++++++++++-------
>  arch/powerpc/kernel/prom_init_check.sh |   2 +-
>  2 files changed, 171 insertions(+), 42 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index ecf083c46bdb..7017156168e8 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -224,6 +224,135 @@ static bool  __prombss rtas_has_query_cpu_stopped;
>  #define PHANDLE_VALID(p)	((p) != 0 && (p) != PROM_ERROR)
>  #define IHANDLE_VALID(i)	((i) != 0 && (i) != PROM_ERROR)
>  
> +/* Copied from lib/string.c and lib/kstrtox.c */
> +
> +static int __init prom_strcmp(const char *cs, const char *ct)
> +{
> +	unsigned char c1, c2;
> +
> +	while (1) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +	}
> +	return 0;
> +}
> +
> +static char __init *prom_strcpy(char *dest, const char *src)
> +{
> +	char *tmp = dest;
> +
> +	while ((*dest++ = *src++) != '\0')
> +		/* nothing */;
> +	return tmp;
> +}
> +
> +static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> +{
> +	unsigned char c1, c2;
> +
> +	while (count) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +		count--;
> +	}
> +	return 0;
> +}
> +
> +static size_t __init prom_strlen(const char *s)
> +{
> +	const char *sc;
> +
> +	for (sc = s; *sc != '\0'; ++sc)
> +		/* nothing */;
> +	return sc - s;
> +}
> +
> +static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
> +{
> +	const unsigned char *su1, *su2;
> +	int res = 0;
> +
> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +		if ((res = *su1 - *su2) != 0)
> +			break;
> +	return res;
> +}
> +
> +static char __init *prom_strstr(const char *s1, const char *s2)
> +{
> +	size_t l1, l2;
> +
> +	l2 = prom_strlen(s2);
> +	if (!l2)
> +		return (char *)s1;
> +	l1 = prom_strlen(s1);
> +	while (l1 >= l2) {
> +		l1--;
> +		if (!prom_memcmp(s1, s2, l2))
> +			return (char *)s1;
> +		s1++;
> +	}
> +	return NULL;
> +}
> +
> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
> +{
> +	size_t ret = prom_strlen(src);
> +
> +	if (size) {
> +		size_t len = (ret >= size) ? size - 1 : ret;
> +		memcpy(dest, src, len);
> +		dest[len] = '\0';
> +	}
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PPC_PSERIES
> +static int __init prom_strtobool(const char *s, bool *res)
> +{
> +	if (!s)
> +		return -EINVAL;
> +
> +	switch (s[0]) {
> +	case 'y':
> +	case 'Y':
> +	case '1':
> +		*res = true;
> +		return 0;
> +	case 'n':
> +	case 'N':
> +	case '0':
> +		*res = false;
> +		return 0;
> +	case 'o':
> +	case 'O':
> +		switch (s[1]) {
> +		case 'n':
> +		case 'N':
> +			*res = true;
> +			return 0;
> +		case 'f':
> +		case 'F':
> +			*res = false;
> +			return 0;
> +		default:
> +			break;
> +		}
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +#endif
>  
>  /* This is the one and *ONLY* place where we actually call open
>   * firmware.
> @@ -555,7 +684,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
>  	add_string(&p, tohex((u32)(unsigned long) value));
>  	add_string(&p, tohex(valuelen));
>  	add_string(&p, tohex(ADDR(pname)));
> -	add_string(&p, tohex(strlen(pname)));
> +	add_string(&p, tohex(prom_strlen(pname)));
>  	add_string(&p, "property");
>  	*p = 0;
>  	return call_prom("interpret", 1, 1, (u32)(unsigned long) cmd);
> @@ -638,23 +767,23 @@ static void __init early_cmdline_parse(void)
>  	if ((long)prom.chosen > 0)
>  		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>  	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl check */
> -		strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
> +		prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
>  	prom_printf("command line: %s\n", prom_cmd_line);
>  
>  #ifdef CONFIG_PPC64
> -	opt = strstr(prom_cmd_line, "iommu=");
> +	opt = prom_strstr(prom_cmd_line, "iommu=");
>  	if (opt) {
>  		prom_printf("iommu opt is: %s\n", opt);
>  		opt += 6;
>  		while (*opt && *opt == ' ')
>  			opt++;
> -		if (!strncmp(opt, "off", 3))
> +		if (!prom_strncmp(opt, "off", 3))
>  			prom_iommu_off = 1;
> -		else if (!strncmp(opt, "force", 5))
> +		else if (!prom_strncmp(opt, "force", 5))
>  			prom_iommu_force_on = 1;
>  	}
>  #endif
> -	opt = strstr(prom_cmd_line, "mem=");
> +	opt = prom_strstr(prom_cmd_line, "mem=");
>  	if (opt) {
>  		opt += 4;
>  		prom_memory_limit = prom_memparse(opt, (const char **)&opt);
> @@ -666,13 +795,13 @@ static void __init early_cmdline_parse(void)
>  
>  #ifdef CONFIG_PPC_PSERIES
>  	prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
> -	opt = strstr(prom_cmd_line, "disable_radix");
> +	opt = prom_strstr(prom_cmd_line, "disable_radix");
>  	if (opt) {
>  		opt += 13;
>  		if (*opt && *opt == '=') {
>  			bool val;
>  
> -			if (kstrtobool(++opt, &val))
> +			if (prom_strtobool(++opt, &val))
>  				prom_radix_disable = false;
>  			else
>  				prom_radix_disable = val;
> @@ -1025,7 +1154,7 @@ static int __init prom_count_smt_threads(void)
>  		type[0] = 0;
>  		prom_getprop(node, "device_type", type, sizeof(type));
>  
> -		if (strcmp(type, "cpu"))
> +		if (prom_strcmp(type, "cpu"))
>  			continue;
>  		/*
>  		 * There is an entry for each smt thread, each entry being
> @@ -1472,7 +1601,7 @@ static void __init prom_init_mem(void)
>  			 */
>  			prom_getprop(node, "name", type, sizeof(type));
>  		}
> -		if (strcmp(type, "memory"))
> +		if (prom_strcmp(type, "memory"))
>  			continue;
>  
>  		plen = prom_getprop(node, "reg", regbuf, sizeof(regbuf));
> @@ -1753,19 +1882,19 @@ static void __init prom_initialize_tce_table(void)
>  		prom_getprop(node, "device_type", type, sizeof(type));
>  		prom_getprop(node, "model", model, sizeof(model));
>  
> -		if ((type[0] == 0) || (strstr(type, "pci") == NULL))
> +		if ((type[0] == 0) || (prom_strstr(type, "pci") == NULL))
>  			continue;
>  
>  		/* Keep the old logic intact to avoid regression. */
>  		if (compatible[0] != 0) {
> -			if ((strstr(compatible, "python") == NULL) &&
> -			    (strstr(compatible, "Speedwagon") == NULL) &&
> -			    (strstr(compatible, "Winnipeg") == NULL))
> +			if ((prom_strstr(compatible, "python") == NULL) &&
> +			    (prom_strstr(compatible, "Speedwagon") == NULL) &&
> +			    (prom_strstr(compatible, "Winnipeg") == NULL))
>  				continue;
>  		} else if (model[0] != 0) {
> -			if ((strstr(model, "ython") == NULL) &&
> -			    (strstr(model, "peedwagon") == NULL) &&
> -			    (strstr(model, "innipeg") == NULL))
> +			if ((prom_strstr(model, "ython") == NULL) &&
> +			    (prom_strstr(model, "peedwagon") == NULL) &&
> +			    (prom_strstr(model, "innipeg") == NULL))
>  				continue;
>  		}
>  
> @@ -1914,12 +2043,12 @@ static void __init prom_hold_cpus(void)
>  
>  		type[0] = 0;
>  		prom_getprop(node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "cpu") != 0)
> +		if (prom_strcmp(type, "cpu") != 0)
>  			continue;
>  
>  		/* Skip non-configured cpus. */
>  		if (prom_getprop(node, "status", type, sizeof(type)) > 0)
> -			if (strcmp(type, "okay") != 0)
> +			if (prom_strcmp(type, "okay") != 0)
>  				continue;
>  
>  		reg = cpu_to_be32(-1); /* make sparse happy */
> @@ -1995,9 +2124,9 @@ static void __init prom_find_mmu(void)
>  		return;
>  	version[sizeof(version) - 1] = 0;
>  	/* XXX might need to add other versions here */
> -	if (strcmp(version, "Open Firmware, 1.0.5") == 0)
> +	if (prom_strcmp(version, "Open Firmware, 1.0.5") == 0)
>  		of_workarounds = OF_WA_CLAIM;
> -	else if (strncmp(version, "FirmWorks,3.", 12) == 0) {
> +	else if (prom_strncmp(version, "FirmWorks,3.", 12) == 0) {
>  		of_workarounds = OF_WA_CLAIM | OF_WA_LONGTRAIL;
>  		call_prom("interpret", 1, 1, "dev /memory 0 to allow-reclaim");
>  	} else
> @@ -2030,7 +2159,7 @@ static void __init prom_init_stdout(void)
>  	call_prom("instance-to-path", 3, 1, prom.stdout, path, 255);
>  	prom_printf("OF stdout device is: %s\n", of_stdout_device);
>  	prom_setprop(prom.chosen, "/chosen", "linux,stdout-path",
> -		     path, strlen(path) + 1);
> +		     path, prom_strlen(path) + 1);
>  
>  	/* instance-to-package fails on PA-Semi */
>  	stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
> @@ -2040,7 +2169,7 @@ static void __init prom_init_stdout(void)
>  		/* If it's a display, note it */
>  		memset(type, 0, sizeof(type));
>  		prom_getprop(stdout_node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "display") == 0)
> +		if (prom_strcmp(type, "display") == 0)
>  			prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
>  	}
>  }
> @@ -2061,19 +2190,19 @@ static int __init prom_find_machine_type(void)
>  		compat[len] = 0;
>  		while (i < len) {
>  			char *p = &compat[i];
> -			int sl = strlen(p);
> +			int sl = prom_strlen(p);
>  			if (sl == 0)
>  				break;
> -			if (strstr(p, "Power Macintosh") ||
> -			    strstr(p, "MacRISC"))
> +			if (prom_strstr(p, "Power Macintosh") ||
> +			    prom_strstr(p, "MacRISC"))
>  				return PLATFORM_POWERMAC;
>  #ifdef CONFIG_PPC64
>  			/* We must make sure we don't detect the IBM Cell
>  			 * blades as pSeries due to some firmware issues,
>  			 * so we do it here.
>  			 */
> -			if (strstr(p, "IBM,CBEA") ||
> -			    strstr(p, "IBM,CPBW-1.0"))
> +			if (prom_strstr(p, "IBM,CBEA") ||
> +			    prom_strstr(p, "IBM,CPBW-1.0"))
>  				return PLATFORM_GENERIC;
>  #endif /* CONFIG_PPC64 */
>  			i += sl + 1;
> @@ -2090,7 +2219,7 @@ static int __init prom_find_machine_type(void)
>  			   compat, sizeof(compat)-1);
>  	if (len <= 0)
>  		return PLATFORM_GENERIC;
> -	if (strcmp(compat, "chrp"))
> +	if (prom_strcmp(compat, "chrp"))
>  		return PLATFORM_GENERIC;
>  
>  	/* Default to pSeries. We need to know if we are running LPAR */
> @@ -2152,7 +2281,7 @@ static void __init prom_check_displays(void)
>  	for (node = 0; prom_next_node(&node); ) {
>  		memset(type, 0, sizeof(type));
>  		prom_getprop(node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "display") != 0)
> +		if (prom_strcmp(type, "display") != 0)
>  			continue;
>  
>  		/* It seems OF doesn't null-terminate the path :-( */
> @@ -2256,9 +2385,9 @@ static unsigned long __init dt_find_string(char *str)
>  	s = os = (char *)dt_string_start;
>  	s += 4;
>  	while (s <  (char *)dt_string_end) {
> -		if (strcmp(s, str) == 0)
> +		if (prom_strcmp(s, str) == 0)
>  			return s - os;
> -		s += strlen(s) + 1;
> +		s += prom_strlen(s) + 1;
>  	}
>  	return 0;
>  }
> @@ -2291,7 +2420,7 @@ static void __init scan_dt_build_strings(phandle node,
>  		}
>  
>   		/* skip "name" */
> - 		if (strcmp(namep, "name") == 0) {
> +		if (prom_strcmp(namep, "name") == 0) {
>   			*mem_start = (unsigned long)namep;
>   			prev_name = "name";
>   			continue;
> @@ -2303,7 +2432,7 @@ static void __init scan_dt_build_strings(phandle node,
>  			namep = sstart + soff;
>  		} else {
>  			/* Trim off some if we can */
> -			*mem_start = (unsigned long)namep + strlen(namep) + 1;
> +			*mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>  			dt_string_end = *mem_start;
>  		}
>  		prev_name = namep;
> @@ -2372,7 +2501,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>  			break;
>  
>   		/* skip "name" */
> - 		if (strcmp(pname, "name") == 0) {
> +		if (prom_strcmp(pname, "name") == 0) {
>   			prev_name = "name";
>   			continue;
>   		}
> @@ -2403,7 +2532,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>  		call_prom("getprop", 4, 1, node, pname, valp, l);
>  		*mem_start = _ALIGN(*mem_start, 4);
>  
> -		if (!strcmp(pname, "phandle"))
> +		if (!prom_strcmp(pname, "phandle"))
>  			has_phandle = 1;
>  	}
>  
> @@ -2473,8 +2602,8 @@ static void __init flatten_device_tree(void)
>  
>  	/* Add "phandle" in there, we'll need it */
>  	namep = make_room(&mem_start, &mem_end, 16, 1);
> -	strcpy(namep, "phandle");
> -	mem_start = (unsigned long)namep + strlen(namep) + 1;
> +	prom_strcpy(namep, "phandle");
> +	mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>  
>  	/* Build string array */
>  	prom_printf("Building dt strings...\n"); 
> @@ -2796,7 +2925,7 @@ static void __init fixup_device_tree_efika(void)
>  	rv = prom_getprop(node, "model", prop, sizeof(prop));
>  	if (rv == PROM_ERROR)
>  		return;
> -	if (strcmp(prop, "EFIKA5K2"))
> +	if (prom_strcmp(prop, "EFIKA5K2"))
>  		return;
>  
>  	prom_printf("Applying EFIKA device tree fixups\n");
> @@ -2804,13 +2933,13 @@ static void __init fixup_device_tree_efika(void)
>  	/* Claiming to be 'chrp' is death */
>  	node = call_prom("finddevice", 1, 1, ADDR("/"));
>  	rv = prom_getprop(node, "device_type", prop, sizeof(prop));
> -	if (rv != PROM_ERROR && (strcmp(prop, "chrp") == 0))
> +	if (rv != PROM_ERROR && (prom_strcmp(prop, "chrp") == 0))
>  		prom_setprop(node, "/", "device_type", "efika", sizeof("efika"));
>  
>  	/* CODEGEN,description is exposed in /proc/cpuinfo so
>  	   fix that too */
>  	rv = prom_getprop(node, "CODEGEN,description", prop, sizeof(prop));
> -	if (rv != PROM_ERROR && (strstr(prop, "CHRP")))
> +	if (rv != PROM_ERROR && (prom_strstr(prop, "CHRP")))
>  		prom_setprop(node, "/", "CODEGEN,description",
>  			     "Efika 5200B PowerPC System",
>  			     sizeof("Efika 5200B PowerPC System"));
> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 181fd10008ef..4cac45cb5de5 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -27,7 +27,7 @@ fi
>  WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>  _end enter_prom $MEM_FUNCS reloc_offset __secondary_hold
>  __secondary_hold_acknowledge __secondary_hold_spinloop __start
> -strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
> +logo_linux_clut224
>  reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>  __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
>  
> -- 
> 2.13.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC v3 18/18] powerpc: KASAN for 64bit Book3E
       [not found] ` <3e97aba429c769bd99ccd8d6f16eda98f7d378a7.1552428161.git.christophe.leroy@c-s.fr>
@ 2019-03-13  7:02   ` Christophe Leroy
  2019-03-13  8:30     ` Christophe Leroy
  0 siblings, 1 reply; 4+ messages in thread
From: Christophe Leroy @ 2019-03-13  7:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

Why does snowpatch reports not being able to apply it to any branch ?

I built the serie on top of merge branch, but it also cleanly applies on 
next branch.

Could it be because the begining of the series is names 'v10' while the 
end of it is 'RFC v3' as it comes from Daniel's RFC v2 ?

Christophe

Le 12/03/2019 à 23:16, Christophe Leroy a écrit :
> From: Daniel Axtens <dja@axtens.net>
> 
> Wire up KASAN. Only outline instrumentation is supported.
> 
> The KASAN shadow area is mapped into vmemmap space:
> 0x8000 0400 0000 0000 to 0x8000 0600 0000 0000.
> To do this we require that vmemmap be disabled. (This is the default
> in the kernel config that QorIQ provides for the machine in their
> SDK anyway - they use flat memory.)
> 
> Only the kernel linear mapping (0xc000...) is checked. The vmalloc and
> ioremap areas (also in 0x800...) are all mapped to the zero page. As
> with the Book3S hash series, this requires overriding the memory <->
> shadow mapping.
> 
> Also, as with both previous 64-bit series, early instrumentation is not
> supported.  It would allow us to drop the check_return_arch_not_ready()
> hook in the KASAN core, but it's tricky to get it set up early enough:
> we need it setup before the first call to instrumented code like printk().
> Perhaps in the future.
> 
> Only KASAN_MINIMAL works.
> 
> Tested on e6500. KVM, kexec and xmon have not been tested.
> 
> The test_kasan module fires warnings as expected, except for the
> following tests:
> 
>   - Expected/by design:
> kasan test: memcg_accounted_kmem_cache allocate memcg accounted object
> 
>   - Due to only supporting KASAN_MINIMAL:
> kasan test: kasan_stack_oob out-of-bounds on stack
> kasan test: kasan_global_oob out-of-bounds global variable
> kasan test: kasan_alloca_oob_left out-of-bounds to left on alloca
> kasan test: kasan_alloca_oob_right out-of-bounds to right on alloca
> kasan test: use_after_scope_test use-after-scope on int
> kasan test: use_after_scope_test use-after-scope on array
> 
> Thanks to those who have done the heavy lifting over the past several
> years:
>   - Christophe's 32 bit series: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-February/185379.html
>   - Aneesh's Book3S hash series: https://lwn.net/Articles/655642/
>   - Balbir's Book3S radix series: https://patchwork.ozlabs.org/patch/795211/
> 
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> [- Removed EXPORT_SYMBOL of the static key
>   - Fixed most checkpatch problems
>   - Replaced kasan_zero_page[] by kasan_early_shadow_page[]
>   - Reduced casting mess by using intermediate locals
>   - Fixed build failure on pmac32_defconfig]
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>   arch/powerpc/Kconfig                         |  1 +
>   arch/powerpc/Kconfig.debug                   |  2 +-
>   arch/powerpc/include/asm/kasan.h             | 71 ++++++++++++++++++++++++++++
>   arch/powerpc/mm/Makefile                     |  2 +
>   arch/powerpc/mm/kasan/Makefile               |  1 +
>   arch/powerpc/mm/kasan/kasan_init_book3e_64.c | 50 ++++++++++++++++++++
>   6 files changed, 126 insertions(+), 1 deletion(-)
>   create mode 100644 arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d9364368329b..51ef9fac6c5d 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -174,6 +174,7 @@ config PPC
>   	select HAVE_ARCH_AUDITSYSCALL
>   	select HAVE_ARCH_JUMP_LABEL
>   	select HAVE_ARCH_KASAN			if PPC32
> +	select HAVE_ARCH_KASAN			if PPC_BOOK3E_64 && !SPARSEMEM_VMEMMAP
>   	select HAVE_ARCH_KGDB
>   	select HAVE_ARCH_MMAP_RND_BITS
>   	select HAVE_ARCH_MMAP_RND_COMPAT_BITS	if COMPAT
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 61febbbdd02b..fc1f5fa7554e 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -369,5 +369,5 @@ config PPC_FAST_ENDIAN_SWITCH
>   
>   config KASAN_SHADOW_OFFSET
>   	hex
> -	depends on KASAN
> +	depends on KASAN && PPC32
>   	default 0xe0000000
> diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
> index 296e51c2f066..ae410f0e060d 100644
> --- a/arch/powerpc/include/asm/kasan.h
> +++ b/arch/powerpc/include/asm/kasan.h
> @@ -21,12 +21,15 @@
>   #define KASAN_SHADOW_START	(KASAN_SHADOW_OFFSET + \
>   				 (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
>   
> +#ifdef CONFIG_PPC32
>   #define KASAN_SHADOW_OFFSET	ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
>   
>   #define KASAN_SHADOW_END	0UL
>   
>   #define KASAN_SHADOW_SIZE	(KASAN_SHADOW_END - KASAN_SHADOW_START)
>   
> +#endif /* CONFIG_PPC32 */
> +
>   #ifdef CONFIG_KASAN
>   void kasan_early_init(void);
>   void kasan_mmu_init(void);
> @@ -36,5 +39,73 @@ static inline void kasan_init(void) { }
>   static inline void kasan_mmu_init(void) { }
>   #endif
>   
> +#ifdef CONFIG_PPC_BOOK3E_64
> +#include <asm/pgtable.h>
> +#include <linux/jump_label.h>
> +
> +/*
> + * We don't put this in Kconfig as we only support KASAN_MINIMAL, and
> + * that will be disabled if the symbol is available in Kconfig
> + */
> +#define KASAN_SHADOW_OFFSET	ASM_CONST(0x6800040000000000)
> +
> +#define KASAN_SHADOW_SIZE	(KERN_VIRT_SIZE >> KASAN_SHADOW_SCALE_SHIFT)
> +
> +extern struct static_key_false powerpc_kasan_enabled_key;
> +extern unsigned char kasan_early_shadow_page[];
> +
> +static inline bool kasan_arch_is_ready_book3e(void)
> +{
> +	if (static_branch_likely(&powerpc_kasan_enabled_key))
> +		return true;
> +	return false;
> +}
> +#define kasan_arch_is_ready kasan_arch_is_ready_book3e
> +
> +static inline void *kasan_mem_to_shadow_book3e(const void *ptr)
> +{
> +	unsigned long addr = (unsigned long)ptr;
> +
> +	if (addr >= KERN_VIRT_START && addr < KERN_VIRT_START + KERN_VIRT_SIZE)
> +		return kasan_early_shadow_page;
> +
> +	return (void *)(addr >> KASAN_SHADOW_SCALE_SHIFT) + KASAN_SHADOW_OFFSET;
> +}
> +#define kasan_mem_to_shadow kasan_mem_to_shadow_book3e
> +
> +static inline void *kasan_shadow_to_mem_book3e(const void *shadow_addr)
> +{
> +	/*
> +	 * We map the entire non-linear virtual mapping onto the zero page so if
> +	 * we are asked to map the zero page back just pick the beginning of that
> +	 * area.
> +	 */
> +	if (shadow_addr >= (void *)kasan_early_shadow_page &&
> +	    shadow_addr < (void *)(kasan_early_shadow_page + PAGE_SIZE))
> +		return (void *)KERN_VIRT_START;
> +
> +	return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET) <<
> +			KASAN_SHADOW_SCALE_SHIFT);
> +}
> +#define kasan_shadow_to_mem kasan_shadow_to_mem_book3e
> +
> +static inline bool kasan_addr_has_shadow_book3e(const void *ptr)
> +{
> +	unsigned long addr = (unsigned long)ptr;
> +
> +	/*
> +	 * We want to specifically assert that the addresses in the 0x8000...
> +	 * region have a shadow, otherwise they are considered by the kasan
> +	 * core to be wild pointers
> +	 */
> +	if (addr >= KERN_VIRT_START && addr < (KERN_VIRT_START + KERN_VIRT_SIZE))
> +		return true;
> +
> +	return (ptr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> +}
> +#define kasan_addr_has_shadow kasan_addr_has_shadow_book3e
> +
> +#endif /* CONFIG_PPC_BOOK3E_64 */
> +
>   #endif /* __ASSEMBLY */
>   #endif
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 80382a2d169b..fc49231f807c 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -8,9 +8,11 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>   CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>   
>   KASAN_SANITIZE_ppc_mmu_32.o := n
> +KASAN_SANITIZE_fsl_booke_mmu.o := n
>   
>   ifdef CONFIG_KASAN
>   CFLAGS_ppc_mmu_32.o  		+= -DDISABLE_BRANCH_PROFILING
> +CFLAGS_fsl_booke_mmu.o		+= -DDISABLE_BRANCH_PROFILING
>   endif
>   
>   obj-y				:= fault.o mem.o pgtable.o mmap.o \
> diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
> index 6577897673dd..f8f164ad8ade 100644
> --- a/arch/powerpc/mm/kasan/Makefile
> +++ b/arch/powerpc/mm/kasan/Makefile
> @@ -3,3 +3,4 @@
>   KASAN_SANITIZE := n
>   
>   obj-$(CONFIG_PPC32)           += kasan_init_32.o
> +obj-$(CONFIG_PPC_BOOK3E_64)   += kasan_init_book3e_64.o
> diff --git a/arch/powerpc/mm/kasan/kasan_init_book3e_64.c b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> new file mode 100644
> index 000000000000..f116c211d83c
> --- /dev/null
> +++ b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define DISABLE_BRANCH_PROFILING
> +
> +#include <linux/kasan.h>
> +#include <linux/printk.h>
> +#include <linux/memblock.h>
> +#include <linux/sched/task.h>
> +#include <asm/pgalloc.h>
> +
> +DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
> +
> +static void __init kasan_init_region(struct memblock_region *reg)
> +{
> +	void *start = __va(reg->base);
> +	void *end = __va(reg->base + reg->size);
> +	unsigned long k_start, k_end, k_cur;
> +
> +	if (start >= end)
> +		return;
> +
> +	k_start = (unsigned long)kasan_mem_to_shadow(start);
> +	k_end = (unsigned long)kasan_mem_to_shadow(end);
> +
> +	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
> +		void *va = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> +
> +		map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
> +	}
> +	flush_tlb_kernel_range(k_start, k_end);
> +}
> +
> +void __init kasan_init(void)
> +{
> +	struct memblock_region *reg;
> +
> +	for_each_memblock(memory, reg)
> +		kasan_init_region(reg);
> +
> +	/* map the zero page RO */
> +	map_kernel_page((unsigned long)kasan_early_shadow_page,
> +			__pa(kasan_early_shadow_page), PAGE_KERNEL_RO);
> +
> +	/* Turn on checking */
> +	static_branch_inc(&powerpc_kasan_enabled_key);
> +
> +	/* Enable error messages */
> +	init_task.kasan_depth = 0;
> +	pr_info("KASAN init done (64-bit Book3E)\n");
> +}
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC v3 18/18] powerpc: KASAN for 64bit Book3E
  2019-03-13  7:02   ` [PATCH RFC v3 18/18] powerpc: KASAN for 64bit Book3E Christophe Leroy
@ 2019-03-13  8:30     ` Christophe Leroy
  0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2019-03-13  8:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

Any, the build is clean, see 
http://kisskb.ellerman.id.au/kisskb/head/3e97aba429c769bd99ccd8d6f16eda98f7d378a7/

Only s390 defconfig and powerpc randconfig failed for unrelated reasons.

Christophe

Le 13/03/2019 à 08:02, Christophe Leroy a écrit :
> Why does snowpatch reports not being able to apply it to any branch ?
> 
> I built the serie on top of merge branch, but it also cleanly applies on 
> next branch.
> 
> Could it be because the begining of the series is names 'v10' while the 
> end of it is 'RFC v3' as it comes from Daniel's RFC v2 ?
> 
> Christophe
> 
> Le 12/03/2019 à 23:16, Christophe Leroy a écrit :
>> From: Daniel Axtens <dja@axtens.net>
>>
>> Wire up KASAN. Only outline instrumentation is supported.
>>
>> The KASAN shadow area is mapped into vmemmap space:
>> 0x8000 0400 0000 0000 to 0x8000 0600 0000 0000.
>> To do this we require that vmemmap be disabled. (This is the default
>> in the kernel config that QorIQ provides for the machine in their
>> SDK anyway - they use flat memory.)
>>
>> Only the kernel linear mapping (0xc000...) is checked. The vmalloc and
>> ioremap areas (also in 0x800...) are all mapped to the zero page. As
>> with the Book3S hash series, this requires overriding the memory <->
>> shadow mapping.
>>
>> Also, as with both previous 64-bit series, early instrumentation is not
>> supported.  It would allow us to drop the check_return_arch_not_ready()
>> hook in the KASAN core, but it's tricky to get it set up early enough:
>> we need it setup before the first call to instrumented code like 
>> printk().
>> Perhaps in the future.
>>
>> Only KASAN_MINIMAL works.
>>
>> Tested on e6500. KVM, kexec and xmon have not been tested.
>>
>> The test_kasan module fires warnings as expected, except for the
>> following tests:
>>
>>   - Expected/by design:
>> kasan test: memcg_accounted_kmem_cache allocate memcg accounted object
>>
>>   - Due to only supporting KASAN_MINIMAL:
>> kasan test: kasan_stack_oob out-of-bounds on stack
>> kasan test: kasan_global_oob out-of-bounds global variable
>> kasan test: kasan_alloca_oob_left out-of-bounds to left on alloca
>> kasan test: kasan_alloca_oob_right out-of-bounds to right on alloca
>> kasan test: use_after_scope_test use-after-scope on int
>> kasan test: use_after_scope_test use-after-scope on array
>>
>> Thanks to those who have done the heavy lifting over the past several
>> years:
>>   - Christophe's 32 bit series: 
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-February/185379.html
>>   - Aneesh's Book3S hash series: https://lwn.net/Articles/655642/
>>   - Balbir's Book3S radix series: 
>> https://patchwork.ozlabs.org/patch/795211/
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> Cc: Balbir Singh <bsingharora@gmail.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> [- Removed EXPORT_SYMBOL of the static key
>>   - Fixed most checkpatch problems
>>   - Replaced kasan_zero_page[] by kasan_early_shadow_page[]
>>   - Reduced casting mess by using intermediate locals
>>   - Fixed build failure on pmac32_defconfig]
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/Kconfig                         |  1 +
>>   arch/powerpc/Kconfig.debug                   |  2 +-
>>   arch/powerpc/include/asm/kasan.h             | 71 
>> ++++++++++++++++++++++++++++
>>   arch/powerpc/mm/Makefile                     |  2 +
>>   arch/powerpc/mm/kasan/Makefile               |  1 +
>>   arch/powerpc/mm/kasan/kasan_init_book3e_64.c | 50 ++++++++++++++++++++
>>   6 files changed, 126 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/powerpc/mm/kasan/kasan_init_book3e_64.c
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index d9364368329b..51ef9fac6c5d 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -174,6 +174,7 @@ config PPC
>>       select HAVE_ARCH_AUDITSYSCALL
>>       select HAVE_ARCH_JUMP_LABEL
>>       select HAVE_ARCH_KASAN            if PPC32
>> +    select HAVE_ARCH_KASAN            if PPC_BOOK3E_64 && 
>> !SPARSEMEM_VMEMMAP
>>       select HAVE_ARCH_KGDB
>>       select HAVE_ARCH_MMAP_RND_BITS
>>       select HAVE_ARCH_MMAP_RND_COMPAT_BITS    if COMPAT
>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
>> index 61febbbdd02b..fc1f5fa7554e 100644
>> --- a/arch/powerpc/Kconfig.debug
>> +++ b/arch/powerpc/Kconfig.debug
>> @@ -369,5 +369,5 @@ config PPC_FAST_ENDIAN_SWITCH
>>   config KASAN_SHADOW_OFFSET
>>       hex
>> -    depends on KASAN
>> +    depends on KASAN && PPC32
>>       default 0xe0000000
>> diff --git a/arch/powerpc/include/asm/kasan.h 
>> b/arch/powerpc/include/asm/kasan.h
>> index 296e51c2f066..ae410f0e060d 100644
>> --- a/arch/powerpc/include/asm/kasan.h
>> +++ b/arch/powerpc/include/asm/kasan.h
>> @@ -21,12 +21,15 @@
>>   #define KASAN_SHADOW_START    (KASAN_SHADOW_OFFSET + \
>>                    (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
>> +#ifdef CONFIG_PPC32
>>   #define KASAN_SHADOW_OFFSET    ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
>>   #define KASAN_SHADOW_END    0UL
>>   #define KASAN_SHADOW_SIZE    (KASAN_SHADOW_END - KASAN_SHADOW_START)
>> +#endif /* CONFIG_PPC32 */
>> +
>>   #ifdef CONFIG_KASAN
>>   void kasan_early_init(void);
>>   void kasan_mmu_init(void);
>> @@ -36,5 +39,73 @@ static inline void kasan_init(void) { }
>>   static inline void kasan_mmu_init(void) { }
>>   #endif
>> +#ifdef CONFIG_PPC_BOOK3E_64
>> +#include <asm/pgtable.h>
>> +#include <linux/jump_label.h>
>> +
>> +/*
>> + * We don't put this in Kconfig as we only support KASAN_MINIMAL, and
>> + * that will be disabled if the symbol is available in Kconfig
>> + */
>> +#define KASAN_SHADOW_OFFSET    ASM_CONST(0x6800040000000000)
>> +
>> +#define KASAN_SHADOW_SIZE    (KERN_VIRT_SIZE >> 
>> KASAN_SHADOW_SCALE_SHIFT)
>> +
>> +extern struct static_key_false powerpc_kasan_enabled_key;
>> +extern unsigned char kasan_early_shadow_page[];
>> +
>> +static inline bool kasan_arch_is_ready_book3e(void)
>> +{
>> +    if (static_branch_likely(&powerpc_kasan_enabled_key))
>> +        return true;
>> +    return false;
>> +}
>> +#define kasan_arch_is_ready kasan_arch_is_ready_book3e
>> +
>> +static inline void *kasan_mem_to_shadow_book3e(const void *ptr)
>> +{
>> +    unsigned long addr = (unsigned long)ptr;
>> +
>> +    if (addr >= KERN_VIRT_START && addr < KERN_VIRT_START + 
>> KERN_VIRT_SIZE)
>> +        return kasan_early_shadow_page;
>> +
>> +    return (void *)(addr >> KASAN_SHADOW_SCALE_SHIFT) + 
>> KASAN_SHADOW_OFFSET;
>> +}
>> +#define kasan_mem_to_shadow kasan_mem_to_shadow_book3e
>> +
>> +static inline void *kasan_shadow_to_mem_book3e(const void *shadow_addr)
>> +{
>> +    /*
>> +     * We map the entire non-linear virtual mapping onto the zero 
>> page so if
>> +     * we are asked to map the zero page back just pick the beginning 
>> of that
>> +     * area.
>> +     */
>> +    if (shadow_addr >= (void *)kasan_early_shadow_page &&
>> +        shadow_addr < (void *)(kasan_early_shadow_page + PAGE_SIZE))
>> +        return (void *)KERN_VIRT_START;
>> +
>> +    return (void *)(((unsigned long)shadow_addr - 
>> KASAN_SHADOW_OFFSET) <<
>> +            KASAN_SHADOW_SCALE_SHIFT);
>> +}
>> +#define kasan_shadow_to_mem kasan_shadow_to_mem_book3e
>> +
>> +static inline bool kasan_addr_has_shadow_book3e(const void *ptr)
>> +{
>> +    unsigned long addr = (unsigned long)ptr;
>> +
>> +    /*
>> +     * We want to specifically assert that the addresses in the 
>> 0x8000...
>> +     * region have a shadow, otherwise they are considered by the kasan
>> +     * core to be wild pointers
>> +     */
>> +    if (addr >= KERN_VIRT_START && addr < (KERN_VIRT_START + 
>> KERN_VIRT_SIZE))
>> +        return true;
>> +
>> +    return (ptr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
>> +}
>> +#define kasan_addr_has_shadow kasan_addr_has_shadow_book3e
>> +
>> +#endif /* CONFIG_PPC_BOOK3E_64 */
>> +
>>   #endif /* __ASSEMBLY */
>>   #endif
>> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
>> index 80382a2d169b..fc49231f807c 100644
>> --- a/arch/powerpc/mm/Makefile
>> +++ b/arch/powerpc/mm/Makefile
>> @@ -8,9 +8,11 @@ ccflags-$(CONFIG_PPC64)    := $(NO_MINIMAL_TOC)
>>   CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>>   KASAN_SANITIZE_ppc_mmu_32.o := n
>> +KASAN_SANITIZE_fsl_booke_mmu.o := n
>>   ifdef CONFIG_KASAN
>>   CFLAGS_ppc_mmu_32.o          += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_fsl_booke_mmu.o        += -DDISABLE_BRANCH_PROFILING
>>   endif
>>   obj-y                := fault.o mem.o pgtable.o mmap.o \
>> diff --git a/arch/powerpc/mm/kasan/Makefile 
>> b/arch/powerpc/mm/kasan/Makefile
>> index 6577897673dd..f8f164ad8ade 100644
>> --- a/arch/powerpc/mm/kasan/Makefile
>> +++ b/arch/powerpc/mm/kasan/Makefile
>> @@ -3,3 +3,4 @@
>>   KASAN_SANITIZE := n
>>   obj-$(CONFIG_PPC32)           += kasan_init_32.o
>> +obj-$(CONFIG_PPC_BOOK3E_64)   += kasan_init_book3e_64.o
>> diff --git a/arch/powerpc/mm/kasan/kasan_init_book3e_64.c 
>> b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
>> new file mode 100644
>> index 000000000000..f116c211d83c
>> --- /dev/null
>> +++ b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
>> @@ -0,0 +1,50 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define DISABLE_BRANCH_PROFILING
>> +
>> +#include <linux/kasan.h>
>> +#include <linux/printk.h>
>> +#include <linux/memblock.h>
>> +#include <linux/sched/task.h>
>> +#include <asm/pgalloc.h>
>> +
>> +DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
>> +
>> +static void __init kasan_init_region(struct memblock_region *reg)
>> +{
>> +    void *start = __va(reg->base);
>> +    void *end = __va(reg->base + reg->size);
>> +    unsigned long k_start, k_end, k_cur;
>> +
>> +    if (start >= end)
>> +        return;
>> +
>> +    k_start = (unsigned long)kasan_mem_to_shadow(start);
>> +    k_end = (unsigned long)kasan_mem_to_shadow(end);
>> +
>> +    for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
>> +        void *va = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>> +
>> +        map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
>> +    }
>> +    flush_tlb_kernel_range(k_start, k_end);
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +    struct memblock_region *reg;
>> +
>> +    for_each_memblock(memory, reg)
>> +        kasan_init_region(reg);
>> +
>> +    /* map the zero page RO */
>> +    map_kernel_page((unsigned long)kasan_early_shadow_page,
>> +            __pa(kasan_early_shadow_page), PAGE_KERNEL_RO);
>> +
>> +    /* Turn on checking */
>> +    static_branch_inc(&powerpc_kasan_enabled_key);
>> +
>> +    /* Enable error messages */
>> +    init_task.kasan_depth = 0;
>> +    pr_info("KASAN init done (64-bit Book3E)\n");
>> +}
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v10 05/18] powerpc/prom_init: don't use string functions from lib/
  2019-03-12 23:38   ` [PATCH v10 05/18] powerpc/prom_init: don't use string functions from lib/ Daniel Axtens
@ 2019-04-26 16:24     ` Christophe Leroy
  0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2019-04-26 16:24 UTC (permalink / raw)
  To: Daniel Axtens, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-kernel, linuxppc-dev, kasan-dev, linux-mm



Le 13/03/2019 à 00:38, Daniel Axtens a écrit :
> Hi Christophe,
> 
> In trying to extend my KASAN implementation to Book3S 64bit, I found one
> other change needed to prom_init. I don't know if you think it should go
> in this patch, the next one, or somewhere else entirely - I will leave
> it up to you. Just let me know if you want me to carry it separately.

Taken in patch 5 in v11.

Christophe

> 
> Thanks again for all your work on this and the integration of my series.
> 
> Regards,
> Daniel
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 7017156168e8..cebb3fc535ba 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1265,7 +1265,8 @@ static void __init prom_check_platform_support(void)
>                                         "ibm,arch-vec-5-platform-support");
>   
>          /* First copy the architecture vec template */
> -       ibm_architecture_vec = ibm_architecture_vec_template;
> +       memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
> +              sizeof(struct ibm_arch_vec));
>   
>          if (prop_len > 1) {
>                  int i;
> 
>> When KASAN is active, the string functions in lib/ are doing the
>> KASAN checks. This is too early for prom_init.
>>
>> This patch implements dedicated string functions for prom_init,
>> which will be compiled in with KASAN disabled.
>>
>> Size of prom_init before the patch:
>>     text	   data	    bss	    dec	    hex	filename
>>    12060	    488	   6960	  19508	   4c34	arch/powerpc/kernel/prom_init.o
>>
>> Size of prom_init after the patch:
>>     text	   data	    bss	    dec	    hex	filename
>>    12460	    488	   6960	  19908	   4dc4	arch/powerpc/kernel/prom_init.o
>>
>> This increases the size of prom_init a bit, but as prom_init is
>> in __init section, it is freed after boot anyway.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/kernel/prom_init.c        | 211 ++++++++++++++++++++++++++-------
>>   arch/powerpc/kernel/prom_init_check.sh |   2 +-
>>   2 files changed, 171 insertions(+), 42 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> index ecf083c46bdb..7017156168e8 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -224,6 +224,135 @@ static bool  __prombss rtas_has_query_cpu_stopped;
>>   #define PHANDLE_VALID(p)	((p) != 0 && (p) != PROM_ERROR)
>>   #define IHANDLE_VALID(i)	((i) != 0 && (i) != PROM_ERROR)
>>   
>> +/* Copied from lib/string.c and lib/kstrtox.c */
>> +
>> +static int __init prom_strcmp(const char *cs, const char *ct)
>> +{
>> +	unsigned char c1, c2;
>> +
>> +	while (1) {
>> +		c1 = *cs++;
>> +		c2 = *ct++;
>> +		if (c1 != c2)
>> +			return c1 < c2 ? -1 : 1;
>> +		if (!c1)
>> +			break;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static char __init *prom_strcpy(char *dest, const char *src)
>> +{
>> +	char *tmp = dest;
>> +
>> +	while ((*dest++ = *src++) != '\0')
>> +		/* nothing */;
>> +	return tmp;
>> +}
>> +
>> +static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
>> +{
>> +	unsigned char c1, c2;
>> +
>> +	while (count) {
>> +		c1 = *cs++;
>> +		c2 = *ct++;
>> +		if (c1 != c2)
>> +			return c1 < c2 ? -1 : 1;
>> +		if (!c1)
>> +			break;
>> +		count--;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static size_t __init prom_strlen(const char *s)
>> +{
>> +	const char *sc;
>> +
>> +	for (sc = s; *sc != '\0'; ++sc)
>> +		/* nothing */;
>> +	return sc - s;
>> +}
>> +
>> +static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
>> +{
>> +	const unsigned char *su1, *su2;
>> +	int res = 0;
>> +
>> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
>> +		if ((res = *su1 - *su2) != 0)
>> +			break;
>> +	return res;
>> +}
>> +
>> +static char __init *prom_strstr(const char *s1, const char *s2)
>> +{
>> +	size_t l1, l2;
>> +
>> +	l2 = prom_strlen(s2);
>> +	if (!l2)
>> +		return (char *)s1;
>> +	l1 = prom_strlen(s1);
>> +	while (l1 >= l2) {
>> +		l1--;
>> +		if (!prom_memcmp(s1, s2, l2))
>> +			return (char *)s1;
>> +		s1++;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
>> +{
>> +	size_t ret = prom_strlen(src);
>> +
>> +	if (size) {
>> +		size_t len = (ret >= size) ? size - 1 : ret;
>> +		memcpy(dest, src, len);
>> +		dest[len] = '\0';
>> +	}
>> +	return ret;
>> +}
>> +
>> +#ifdef CONFIG_PPC_PSERIES
>> +static int __init prom_strtobool(const char *s, bool *res)
>> +{
>> +	if (!s)
>> +		return -EINVAL;
>> +
>> +	switch (s[0]) {
>> +	case 'y':
>> +	case 'Y':
>> +	case '1':
>> +		*res = true;
>> +		return 0;
>> +	case 'n':
>> +	case 'N':
>> +	case '0':
>> +		*res = false;
>> +		return 0;
>> +	case 'o':
>> +	case 'O':
>> +		switch (s[1]) {
>> +		case 'n':
>> +		case 'N':
>> +			*res = true;
>> +			return 0;
>> +		case 'f':
>> +		case 'F':
>> +			*res = false;
>> +			return 0;
>> +		default:
>> +			break;
>> +		}
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +#endif
>>   
>>   /* This is the one and *ONLY* place where we actually call open
>>    * firmware.
>> @@ -555,7 +684,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
>>   	add_string(&p, tohex((u32)(unsigned long) value));
>>   	add_string(&p, tohex(valuelen));
>>   	add_string(&p, tohex(ADDR(pname)));
>> -	add_string(&p, tohex(strlen(pname)));
>> +	add_string(&p, tohex(prom_strlen(pname)));
>>   	add_string(&p, "property");
>>   	*p = 0;
>>   	return call_prom("interpret", 1, 1, (u32)(unsigned long) cmd);
>> @@ -638,23 +767,23 @@ static void __init early_cmdline_parse(void)
>>   	if ((long)prom.chosen > 0)
>>   		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>>   	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl check */
>> -		strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
>> +		prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
>>   	prom_printf("command line: %s\n", prom_cmd_line);
>>   
>>   #ifdef CONFIG_PPC64
>> -	opt = strstr(prom_cmd_line, "iommu=");
>> +	opt = prom_strstr(prom_cmd_line, "iommu=");
>>   	if (opt) {
>>   		prom_printf("iommu opt is: %s\n", opt);
>>   		opt += 6;
>>   		while (*opt && *opt == ' ')
>>   			opt++;
>> -		if (!strncmp(opt, "off", 3))
>> +		if (!prom_strncmp(opt, "off", 3))
>>   			prom_iommu_off = 1;
>> -		else if (!strncmp(opt, "force", 5))
>> +		else if (!prom_strncmp(opt, "force", 5))
>>   			prom_iommu_force_on = 1;
>>   	}
>>   #endif
>> -	opt = strstr(prom_cmd_line, "mem=");
>> +	opt = prom_strstr(prom_cmd_line, "mem=");
>>   	if (opt) {
>>   		opt += 4;
>>   		prom_memory_limit = prom_memparse(opt, (const char **)&opt);
>> @@ -666,13 +795,13 @@ static void __init early_cmdline_parse(void)
>>   
>>   #ifdef CONFIG_PPC_PSERIES
>>   	prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
>> -	opt = strstr(prom_cmd_line, "disable_radix");
>> +	opt = prom_strstr(prom_cmd_line, "disable_radix");
>>   	if (opt) {
>>   		opt += 13;
>>   		if (*opt && *opt == '=') {
>>   			bool val;
>>   
>> -			if (kstrtobool(++opt, &val))
>> +			if (prom_strtobool(++opt, &val))
>>   				prom_radix_disable = false;
>>   			else
>>   				prom_radix_disable = val;
>> @@ -1025,7 +1154,7 @@ static int __init prom_count_smt_threads(void)
>>   		type[0] = 0;
>>   		prom_getprop(node, "device_type", type, sizeof(type));
>>   
>> -		if (strcmp(type, "cpu"))
>> +		if (prom_strcmp(type, "cpu"))
>>   			continue;
>>   		/*
>>   		 * There is an entry for each smt thread, each entry being
>> @@ -1472,7 +1601,7 @@ static void __init prom_init_mem(void)
>>   			 */
>>   			prom_getprop(node, "name", type, sizeof(type));
>>   		}
>> -		if (strcmp(type, "memory"))
>> +		if (prom_strcmp(type, "memory"))
>>   			continue;
>>   
>>   		plen = prom_getprop(node, "reg", regbuf, sizeof(regbuf));
>> @@ -1753,19 +1882,19 @@ static void __init prom_initialize_tce_table(void)
>>   		prom_getprop(node, "device_type", type, sizeof(type));
>>   		prom_getprop(node, "model", model, sizeof(model));
>>   
>> -		if ((type[0] == 0) || (strstr(type, "pci") == NULL))
>> +		if ((type[0] == 0) || (prom_strstr(type, "pci") == NULL))
>>   			continue;
>>   
>>   		/* Keep the old logic intact to avoid regression. */
>>   		if (compatible[0] != 0) {
>> -			if ((strstr(compatible, "python") == NULL) &&
>> -			    (strstr(compatible, "Speedwagon") == NULL) &&
>> -			    (strstr(compatible, "Winnipeg") == NULL))
>> +			if ((prom_strstr(compatible, "python") == NULL) &&
>> +			    (prom_strstr(compatible, "Speedwagon") == NULL) &&
>> +			    (prom_strstr(compatible, "Winnipeg") == NULL))
>>   				continue;
>>   		} else if (model[0] != 0) {
>> -			if ((strstr(model, "ython") == NULL) &&
>> -			    (strstr(model, "peedwagon") == NULL) &&
>> -			    (strstr(model, "innipeg") == NULL))
>> +			if ((prom_strstr(model, "ython") == NULL) &&
>> +			    (prom_strstr(model, "peedwagon") == NULL) &&
>> +			    (prom_strstr(model, "innipeg") == NULL))
>>   				continue;
>>   		}
>>   
>> @@ -1914,12 +2043,12 @@ static void __init prom_hold_cpus(void)
>>   
>>   		type[0] = 0;
>>   		prom_getprop(node, "device_type", type, sizeof(type));
>> -		if (strcmp(type, "cpu") != 0)
>> +		if (prom_strcmp(type, "cpu") != 0)
>>   			continue;
>>   
>>   		/* Skip non-configured cpus. */
>>   		if (prom_getprop(node, "status", type, sizeof(type)) > 0)
>> -			if (strcmp(type, "okay") != 0)
>> +			if (prom_strcmp(type, "okay") != 0)
>>   				continue;
>>   
>>   		reg = cpu_to_be32(-1); /* make sparse happy */
>> @@ -1995,9 +2124,9 @@ static void __init prom_find_mmu(void)
>>   		return;
>>   	version[sizeof(version) - 1] = 0;
>>   	/* XXX might need to add other versions here */
>> -	if (strcmp(version, "Open Firmware, 1.0.5") == 0)
>> +	if (prom_strcmp(version, "Open Firmware, 1.0.5") == 0)
>>   		of_workarounds = OF_WA_CLAIM;
>> -	else if (strncmp(version, "FirmWorks,3.", 12) == 0) {
>> +	else if (prom_strncmp(version, "FirmWorks,3.", 12) == 0) {
>>   		of_workarounds = OF_WA_CLAIM | OF_WA_LONGTRAIL;
>>   		call_prom("interpret", 1, 1, "dev /memory 0 to allow-reclaim");
>>   	} else
>> @@ -2030,7 +2159,7 @@ static void __init prom_init_stdout(void)
>>   	call_prom("instance-to-path", 3, 1, prom.stdout, path, 255);
>>   	prom_printf("OF stdout device is: %s\n", of_stdout_device);
>>   	prom_setprop(prom.chosen, "/chosen", "linux,stdout-path",
>> -		     path, strlen(path) + 1);
>> +		     path, prom_strlen(path) + 1);
>>   
>>   	/* instance-to-package fails on PA-Semi */
>>   	stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
>> @@ -2040,7 +2169,7 @@ static void __init prom_init_stdout(void)
>>   		/* If it's a display, note it */
>>   		memset(type, 0, sizeof(type));
>>   		prom_getprop(stdout_node, "device_type", type, sizeof(type));
>> -		if (strcmp(type, "display") == 0)
>> +		if (prom_strcmp(type, "display") == 0)
>>   			prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
>>   	}
>>   }
>> @@ -2061,19 +2190,19 @@ static int __init prom_find_machine_type(void)
>>   		compat[len] = 0;
>>   		while (i < len) {
>>   			char *p = &compat[i];
>> -			int sl = strlen(p);
>> +			int sl = prom_strlen(p);
>>   			if (sl == 0)
>>   				break;
>> -			if (strstr(p, "Power Macintosh") ||
>> -			    strstr(p, "MacRISC"))
>> +			if (prom_strstr(p, "Power Macintosh") ||
>> +			    prom_strstr(p, "MacRISC"))
>>   				return PLATFORM_POWERMAC;
>>   #ifdef CONFIG_PPC64
>>   			/* We must make sure we don't detect the IBM Cell
>>   			 * blades as pSeries due to some firmware issues,
>>   			 * so we do it here.
>>   			 */
>> -			if (strstr(p, "IBM,CBEA") ||
>> -			    strstr(p, "IBM,CPBW-1.0"))
>> +			if (prom_strstr(p, "IBM,CBEA") ||
>> +			    prom_strstr(p, "IBM,CPBW-1.0"))
>>   				return PLATFORM_GENERIC;
>>   #endif /* CONFIG_PPC64 */
>>   			i += sl + 1;
>> @@ -2090,7 +2219,7 @@ static int __init prom_find_machine_type(void)
>>   			   compat, sizeof(compat)-1);
>>   	if (len <= 0)
>>   		return PLATFORM_GENERIC;
>> -	if (strcmp(compat, "chrp"))
>> +	if (prom_strcmp(compat, "chrp"))
>>   		return PLATFORM_GENERIC;
>>   
>>   	/* Default to pSeries. We need to know if we are running LPAR */
>> @@ -2152,7 +2281,7 @@ static void __init prom_check_displays(void)
>>   	for (node = 0; prom_next_node(&node); ) {
>>   		memset(type, 0, sizeof(type));
>>   		prom_getprop(node, "device_type", type, sizeof(type));
>> -		if (strcmp(type, "display") != 0)
>> +		if (prom_strcmp(type, "display") != 0)
>>   			continue;
>>   
>>   		/* It seems OF doesn't null-terminate the path :-( */
>> @@ -2256,9 +2385,9 @@ static unsigned long __init dt_find_string(char *str)
>>   	s = os = (char *)dt_string_start;
>>   	s += 4;
>>   	while (s <  (char *)dt_string_end) {
>> -		if (strcmp(s, str) == 0)
>> +		if (prom_strcmp(s, str) == 0)
>>   			return s - os;
>> -		s += strlen(s) + 1;
>> +		s += prom_strlen(s) + 1;
>>   	}
>>   	return 0;
>>   }
>> @@ -2291,7 +2420,7 @@ static void __init scan_dt_build_strings(phandle node,
>>   		}
>>   
>>    		/* skip "name" */
>> - 		if (strcmp(namep, "name") == 0) {
>> +		if (prom_strcmp(namep, "name") == 0) {
>>    			*mem_start = (unsigned long)namep;
>>    			prev_name = "name";
>>    			continue;
>> @@ -2303,7 +2432,7 @@ static void __init scan_dt_build_strings(phandle node,
>>   			namep = sstart + soff;
>>   		} else {
>>   			/* Trim off some if we can */
>> -			*mem_start = (unsigned long)namep + strlen(namep) + 1;
>> +			*mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>>   			dt_string_end = *mem_start;
>>   		}
>>   		prev_name = namep;
>> @@ -2372,7 +2501,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>>   			break;
>>   
>>    		/* skip "name" */
>> - 		if (strcmp(pname, "name") == 0) {
>> +		if (prom_strcmp(pname, "name") == 0) {
>>    			prev_name = "name";
>>    			continue;
>>    		}
>> @@ -2403,7 +2532,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>>   		call_prom("getprop", 4, 1, node, pname, valp, l);
>>   		*mem_start = _ALIGN(*mem_start, 4);
>>   
>> -		if (!strcmp(pname, "phandle"))
>> +		if (!prom_strcmp(pname, "phandle"))
>>   			has_phandle = 1;
>>   	}
>>   
>> @@ -2473,8 +2602,8 @@ static void __init flatten_device_tree(void)
>>   
>>   	/* Add "phandle" in there, we'll need it */
>>   	namep = make_room(&mem_start, &mem_end, 16, 1);
>> -	strcpy(namep, "phandle");
>> -	mem_start = (unsigned long)namep + strlen(namep) + 1;
>> +	prom_strcpy(namep, "phandle");
>> +	mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>>   
>>   	/* Build string array */
>>   	prom_printf("Building dt strings...\n");
>> @@ -2796,7 +2925,7 @@ static void __init fixup_device_tree_efika(void)
>>   	rv = prom_getprop(node, "model", prop, sizeof(prop));
>>   	if (rv == PROM_ERROR)
>>   		return;
>> -	if (strcmp(prop, "EFIKA5K2"))
>> +	if (prom_strcmp(prop, "EFIKA5K2"))
>>   		return;
>>   
>>   	prom_printf("Applying EFIKA device tree fixups\n");
>> @@ -2804,13 +2933,13 @@ static void __init fixup_device_tree_efika(void)
>>   	/* Claiming to be 'chrp' is death */
>>   	node = call_prom("finddevice", 1, 1, ADDR("/"));
>>   	rv = prom_getprop(node, "device_type", prop, sizeof(prop));
>> -	if (rv != PROM_ERROR && (strcmp(prop, "chrp") == 0))
>> +	if (rv != PROM_ERROR && (prom_strcmp(prop, "chrp") == 0))
>>   		prom_setprop(node, "/", "device_type", "efika", sizeof("efika"));
>>   
>>   	/* CODEGEN,description is exposed in /proc/cpuinfo so
>>   	   fix that too */
>>   	rv = prom_getprop(node, "CODEGEN,description", prop, sizeof(prop));
>> -	if (rv != PROM_ERROR && (strstr(prop, "CHRP")))
>> +	if (rv != PROM_ERROR && (prom_strstr(prop, "CHRP")))
>>   		prom_setprop(node, "/", "CODEGEN,description",
>>   			     "Efika 5200B PowerPC System",
>>   			     sizeof("Efika 5200B PowerPC System"));
>> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
>> index 181fd10008ef..4cac45cb5de5 100644
>> --- a/arch/powerpc/kernel/prom_init_check.sh
>> +++ b/arch/powerpc/kernel/prom_init_check.sh
>> @@ -27,7 +27,7 @@ fi
>>   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>>   _end enter_prom $MEM_FUNCS reloc_offset __secondary_hold
>>   __secondary_hold_acknowledge __secondary_hold_spinloop __start
>> -strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
>> +logo_linux_clut224
>>   reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>>   __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
>>   
>> -- 
>> 2.13.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-04-26 16:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1552428161.git.christophe.leroy@c-s.fr>
     [not found] ` <dc2eb0480f2c87316561435cfcf4c78254d2166b.1552428161.git.christophe.leroy@c-s.fr>
2019-03-12 23:38   ` [PATCH v10 05/18] powerpc/prom_init: don't use string functions from lib/ Daniel Axtens
2019-04-26 16:24     ` Christophe Leroy
     [not found] ` <3e97aba429c769bd99ccd8d6f16eda98f7d378a7.1552428161.git.christophe.leroy@c-s.fr>
2019-03-13  7:02   ` [PATCH RFC v3 18/18] powerpc: KASAN for 64bit Book3E Christophe Leroy
2019-03-13  8:30     ` Christophe Leroy

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).