Re: [PATCH] EDAC/mc: Fix memory alignment calculation formula

[Borislav Petkov <bp@alien8.de>]

On Sun, May 17, 2020 at 01:21:15AM +0900, wata2ki wrote:
> From: Naoto Yamaguchi <i33399_YAMAGUCHI@aisin-aw.co.jp>
> 
> During the development of the off-tree driver, we found a bug that
> causes alignment fault exception in mutex_lock.
> 
> Line of the code:
> ffffffc010536ce4: c85ffe62 ldaxr x2, [x19]
> 
> Register value:
> x19: ffffff800e90f6c4
> 
> This problem was caused by the alignment error of pvt_info
> in struct mem_ctl_info.  It is caused by a calculation formula
> error in edac_align_ptr.
> 
> Existing calculation formula is using variable p, but this
> variable is address of the pointer variable not memory offset.
> In this calculation formula should use *p.
> 
> Signed-off-by: Naoto Yamaguchi <i33399_YAMAGUCHI@aisin-aw.co.jp>
> ---
>  drivers/edac/edac_mc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  mode change 100644 => 100755 drivers/edac/edac_mc.c
> 
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> old mode 100644
> new mode 100755
> index 75ede27bdf6a..70929f136dd7
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -271,7 +271,7 @@ void *edac_align_ptr(void **p, unsigned int size, int n_elems)
>  	else
>  		return (char *)ptr;
>  
> -	r = (unsigned long)p % align;
> +	r = (unsigned long)(*p) % align;

Looks about right to me.

Btw, you don't need the () around *p - that's evaluated right-to-left so
the dereference happens first and *then* the typecast, i.e., what you
want here.

In any case, this line comes from

  8447c4d15e35 ("edac: Do alignment logic properly in edac_align_ptr()")

and I believe it was wrong to use 'p' as that function works with the
memory offsets - not with the pointer to the pointer. It's a whole
different story whether I think this whole thing makes sense and it is
ugly...

Anyway, adding the gentlemen from that commit to Cc.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette