Re: [PATCH] mtd: nand: warn if hamming layout is used with too large ECC

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Fri, 09 Feb 2018 12:55:49 +0100
Stefan Agner <stefan@agner.ch> wrote:

> On 09.02.2018 10:58, Boris Brezillon wrote:
> > On Fri, 09 Feb 2018 10:20:37 +0100
> > Stefan Agner <stefan@agner.ch> wrote:
> >   
> >>
> >> But some kind of sanity check somewhere might be worthwhile, I was a bit
> >> surprised that this overflowing happens on a driver in operational use
> >> and goes unnoticed. I realize that this patch is not ideal. Maybe making
> >> length signed, then we could sanity check in
> >> mtd_ooblayout_count_bytes...  
> > 
> > Something like that should help us detect this unexpected case:
> >   
> 
> It is less generic than making length signed and checking for positive
> length, but fine for me.

It's just easier for callers if they only have to check the return
code instead of having to check both the return code and the oobregion
fields.

Would you mind modifying your patch accordingly and sending a v2?

Thanks,

Boris

> 
> --
> Stefan
> 
> > --->8---  
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 66b67014508f..ada2e709743f 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -201,6 +201,9 @@ static int nand_ooblayout_free_lp_hamming(struct
> > mtd_info *mtd, int section, oobregion->offset = 2;
> >                 oobregion->length = ecc_offset - 2;
> >         } else {
> > +               if (ecc_offset + ecc->total > mtd->oobsize)
> > +                       return -EINVAL;
> > +
> >                 oobregion->offset = ecc_offset + ecc->total;
> >                 oobregion->length = mtd->oobsize - oobregion->offset;
> >         }  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com