[bug reportish] UBI_MAX_ERASECOUNTER not used consistently

[bug reportish] UBI_MAX_ERASECOUNTER not used consistently

[Dan Carpenter]

Hi Artem,

I normally audit for underflows and I had a question about this code.

mtd/ubi/attach.c
   665  static int early_erase_peb(struct ubi_device *ubi,
   666                             const struct ubi_attach_info *ai, int pnum, int ec)
   667  {
   668          int err;
   669          struct ubi_ec_hdr *ec_hdr;
   670  
   671          if ((long long)ec >= UBI_MAX_ERASECOUNTER) {

Casting to long long doesn't serve any purpose.  We normally say that
"> UBI_MAX_ERASECOUNTER" is invalid but "== UBI_MAX_ERASECOUNTER" is Ok.
The "ec" variable is signed and negative values are missed on this
check.

   672                  /*
   673                   * Erase counter overflow. Upgrade UBI and use 64-bit
   674                   * erase counters internally.
   675                   */
   676                  ubi_err("erase counter overflow at PEB %d, EC %d", pnum, ec);
   677                  return -EINVAL;
   678          }
   679  
   680          ec_hdr = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);

regards,
dan carpenter

Re: [bug reportish] UBI_MAX_ERASECOUNTER not used consistently

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

On Fri, 2012-10-05 at 09:57 +0300, Dan Carpenter wrote:
> Hi Artem,
> 
> I normally audit for underflows and I had a question about this code.
> 
> mtd/ubi/attach.c
>    665  static int early_erase_peb(struct ubi_device *ubi,
>    666                             const struct ubi_attach_info *ai, int pnum, int ec)
>    667  {
>    668          int err;
>    669          struct ubi_ec_hdr *ec_hdr;
>    670  
>    671          if ((long long)ec >= UBI_MAX_ERASECOUNTER) {
> 
> Casting to long long doesn't serve any purpose.  We normally say that
> "> UBI_MAX_ERASECOUNTER" is invalid but "== UBI_MAX_ERASECOUNTER" is Ok.
> The "ec" variable is signed and negative values are missed on this
> check.

Yes, the cast is bogus. I can fix it by removing the cast, but would
like to give you a chance to send the patch so that you get the credit
being the author in the git history. Let me know what you prefer.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]