Re: [PATCH] EDAC/versalnet: Pass flags=0 to memory_failure() for UE errors

[Shubhrajyoti Datta]

On Thu, Jul 2, 2026 at 3:10 AM Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Wed, Jul 01, 2026 at 03:20:31PM +0530, Shubhrajyoti Datta wrote:
> > The MF_ACTION_REQUIRED flag is intended for synchronous exceptions
> > where the consuming process must be killed. For EDAC-reported errors
> > discovered asynchronously via polling/interrupt, flags=0 is the
> > correct choice, matching how other EDAC drivers invoke
> > memory_failure().
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
> > ---
> >
> >  drivers/edac/versalnet_edac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> > index 97ec05d68bbb..d1af5e175f7e 100644
> > --- a/drivers/edac/versalnet_edac.c
> > +++ b/drivers/edac/versalnet_edac.c
> > @@ -464,7 +464,7 @@ static void handle_error(struct mc_priv  *priv, struct ecc_status *stat,
> >               pfn = PHYS_PFN(pa);
> >
> >               if (IS_ENABLED(CONFIG_MEMORY_FAILURE)) {
> > -                     err = memory_failure(pfn, MF_ACTION_REQUIRED);
> > +                     err = memory_failure(pfn, 0);
> >                       if (err)
> >                               edac_dbg(2, "memory_failure() error: %d", err);
> >                       else
> > --
>
> A bunch of preexisting issues:
>
> https://sashiko.dev/#/patchset/20260701095040.4139675-1-shubhrajyoti.datta%40amd.com
>
> They need to be addressed first along with everything that is waiting on the
> ML. Then new patches ontop.
Hi Boris,
I have reviewed all the Sashiko comments and identified the valid ones
(details below).
I will be sending out patches to address those comments shortly.

Issue 1::
;
This is a pre-existing issue, but can this lead to a NULL pointer
dereference?

The comment is accepted and will send a patch.

Issue 2:

This is a pre-existing issue, but does remove_one_mc() also unconditionally
dereference the potentially uninitialized mci pointer?

The comment is accepted and will send a patch.

Issue 3:

This is a pre-existing issue, but is there a race condition during probe
that can crash the kernel?

In mc_probe(), register_rpmsg_driver() is called before setup_mcdi():

This comment is not valid.
The order cannot be reversed as setup_mcdi() queries the DDR configuration
  over RPMsg, so the RPMsg driver must be registered first. Also, the firmware
  does not send any  events until the DDR configuration has been
  queried, so the race window does not exist in practice.