Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: wata2ki <wata2ki@gmail.com>, Chris Metcalf <cmetcalf@tilera.com>,
	Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: linux-edac@vger.kernel.org,
	Naoto Yamaguchi <i33399_YAMAGUCHI@aisin-aw.co.jp>
Subject: Re: [PATCH] EDAC/mc: Fix memory alignment calculation formula
Date: Wed, 17 Jun 2020 19:58:09 +0200
Message-ID: <20200617175809.GH10118@zn.tnic> (raw)
In-Reply-To: <20200516162115.16545-1-wata2ki@gmail.com>

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

      parent reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-16 16:21 wata2ki
2020-06-03 11:28 ` Borislav Petkov
2020-06-03 13:07   ` Naoto YAMAGUCHI
     [not found]   ` <CABBJnRYZTsnOjNdd9x5ZS_Vb56yvEJWsLxEERYPj-m3HfAqx1Q@mail.gmail.com>
2020-06-03 17:36     ` Borislav Petkov
2020-06-17 17:58 ` Borislav Petkov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200617175809.GH10118@zn.tnic \
    --to=bp@alien8.de \
    --cc=cmetcalf@tilera.com \
    --cc=i33399_YAMAGUCHI@aisin-aw.co.jp \
    --cc=linux-edac@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=wata2ki@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org
	public-inbox-index linux-edac

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git