Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 15 Feb 2022 13:34:14 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Farber, Eliav" <farbere@amazon.com>
Cc:     mchehab@kernel.org, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, ronenk@amazon.com, talel@amazon.com,
        hhhawa@amazon.com, jonnyc@amazon.com, hanochu@amazon.com
Subject: Re: [PATCH 1/4] EDAC: Fix calculation of returned address and next
 offset in edac_align_ptr()
Message-ID: <YgudxpqAlyQ5UqlF@zn.tnic>
References: <20220113100622.12783-1-farbere@amazon.com>
 <20220113100622.12783-2-farbere@amazon.com>
 <YfALFy7LGGIOS2Fv@zn.tnic>
 <9bd8f3c5-2281-8235-9eac-d2c371245a54@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <9bd8f3c5-2281-8235-9eac-d2c371245a54@amazon.com>
Precedence: bulk

On Thu, Jan 27, 2022 at 11:58:58AM +0200, Farber, Eliav wrote:
> One of the fields in our private-data structure is 'struct notifier_block'
> which has a next field of type 'struct notifier_block __rcu *'.

Just to make sure I understand you correctly: you're talking about some
internal version of al_mc_edac - not what's upstream?

> The size of our private-data structure is greater than 8, and it comes after
> 'struct edac_mc_layer' which has a size that is not zero modulo eight, and
> also ends at an address that is not zero modulo eight.
> Because of the bug in edac_align_ptr(), our private-data structure which
> should have been aligned to 8 wasn't (it was aligned to 4), so
> notifier_block was also not aligned to 8, and finally next wasn't aligned
> to 8.

Ok, I think I see what's going on. So this:

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

changed that remainder computation thing and it was supposed to do what
your patch does. Hell, even the commit message says so:

"The logic was checking the sizeof the structure being allocated to
determine whether an alignment fixup was required. This isn't right;
what we actually care about is the alignment of the actual pointer
that's about to be returned."

So if we really do care about the alignment of the actual pointer we're
about to return, then yes, we should check ptr - not p.

Lemme add that newly discovered info to your patch and queue it.

Thx.

-- 
Regards/Gruss,
    Boris.

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