[U-Boot] [PATCH v3 11/11] mtd: sf: Make sf_mtd.c more robust
Boris Brezillon
boris.brezillon at bootlin.com
Mon Nov 26 12:37:46 UTC 2018
On Mon, 26 Nov 2018 16:42:48 +0530
Jagan Teki <jagan at openedev.com> wrote:
> On 26/11/18 2:12 PM, Boris Brezillon wrote:
> > Hi Jagan,
> >
> > On Thu, 22 Nov 2018 09:40:56 +0100
> > Boris Brezillon <boris.brezillon at bootlin.com> wrote:
> >
> >>>> + /*
> >>>> + * Setting mtd->priv to NULL is the best we can do. Thanks to that,
> >>>> + * the MTD layer can still call mtd hooks without risking a
> >>>> + * use-after-free bug. Still, things should be fixed to prevent the
> >>>> + * spi_flash object from being destroyed when del_mtd_device() fails.
> >>>> + */
> >>>> + sf_mtd_info.priv = NULL;
> >>>> + printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!",
> >>>> + sf_mtd_info.name);
> >>>
> >>> Why do we need this print?
> >>
> >> Yes we do, just to keep the user informed that something bad happened
> >> and its spi-flash is no longer usable (at least through the MTD layer).
> >>
> >>> can't we do the same thing in MTD core
> >>> itself, so-that it can be generic for all flash objects.
> >>
> >> del_mtd_device() can fail, so it's the caller responsibility to decide
> >> what to do when that happens. Some users will propagate the error to
> >> the upper layer and maybe cancel the device removal (AFAICT,
> >> driver->remove() can return an error, not sure what happens in this
> >> case though). For others, like spi-flash, the device will go away, and
> >> all subsequent accesses will fail.
> >
> > I'm about to send a new version fixing the problem I mentioned in patch
> > 3, but before doing that, I'd like to know if my answer convinced you or
> > if you'd still prefer this message to go away (or be placed in
> > mtdcore/mtdpart.c).
>
>
> I'm thinking of having the message still in MTD by showing which
> interface it would belongs, along with the details.
Then we'd need something less
More information about the U-Boot
mailing list