Re: [PATCH v1 1/1] vsprintf: Fix potential unaligned access

[Sakari Ailus]

Hi Andy,

On Mon, Jan 10, 2022 at 10:50:49PM +0200, Andy Shevchenko wrote:
> The %p4cc specifier in some cases might get an unaligned pointer.
> Due to this we need to make copy to local variable once to avoid
> potential crashes on some architectures due to improper access.

I guess this problem exists virtually everywhere where pointers are being
handled: the pointer could be unaligned. Does this even address the false
positive compiler warning?

> 
> Fixes: af612e43de6d ("lib/vsprintf: Add support for printing V4L2 and DRM fourccs")
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  lib/vsprintf.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index c130dcaca5e2..b02f01366acb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -49,6 +49,7 @@
>  
>  #include <asm/page.h>		/* for PAGE_SIZE */
>  #include <asm/byteorder.h>	/* cpu_to_le16 */
> +#include <asm/unaligned.h>
>  
>  #include <linux/string_helpers.h>
>  #include "kstrtox.h"
> @@ -1761,7 +1762,7 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>  	char output[sizeof("0123 little-endian (0x01234567)")];
>  	char *p = output;
>  	unsigned int i;
> -	u32 val;
> +	u32 orig, val;
>  
>  	if (fmt[1] != 'c' || fmt[2] != 'c')
>  		return error_string(buf, end, "(%p4?)", spec);
> @@ -1769,21 +1770,22 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>  	if (check_pointer(&buf, end, fourcc, spec))
>  		return buf;
>  
> -	val = *fourcc & ~BIT(31);
> +	orig = get_unaligned(fourcc);
> +	val = orig & ~BIT(31);
>  
> -	for (i = 0; i < sizeof(*fourcc); i++) {
> +	for (i = 0; i < sizeof(u32); i++) {
>  		unsigned char c = val >> (i * 8);
>  
>  		/* Print non-control ASCII characters as-is, dot otherwise */
>  		*p++ = isascii(c) && isprint(c) ? c : '.';
>  	}
>  
> -	strcpy(p, *fourcc & BIT(31) ? " big-endian" : " little-endian");
> +	strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian");
>  	p += strlen(p);
>  
>  	*p++ = ' ';
>  	*p++ = '(';
> -	p = special_hex_number(p, output + sizeof(output) - 2, *fourcc, sizeof(u32));
> +	p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
>  	*p++ = ')';
>  	*p = '\0';
>  

-- 
Kind regards,

Sakari Ailus