[bug report] UBI: Unsorted Block Images

[Zhihao Cheng]

在 2025/3/31 17:46, Dan Carpenter 写道:
Hi Dan,
> Hello Artem B. Bityutskiy,
> 
> Commit 801c135ce73d ("UBI: Unsorted Block Images") from Jun 27, 2006
> (linux-next), leads to the following Smatch static checker warning:
> 
> 	drivers/mtd/ubi/vtbl.c:844 ubi_read_volume_table()
> 	warn: 'ubi->vtbl' can also be NULL
> 
[...]
>     431          if (!leb_corrupted[0]) {
>     432                  /* LEB 0 is OK */
>     433                  if (leb[1])
>     434                          leb_corrupted[1] = memcmp(leb[0], leb[1],
>     435                                                    ubi->vtbl_size);
>     436                  if (leb_corrupted[1]) {
>     437                          ubi_warn(ubi, "volume table copy #2 is corrupted");
>     438                          err = create_vtbl(ubi, ai, 1, leb[0]);
>     439                          if (err)
>     440                                  goto out_free;
>     441                          ubi_msg(ubi, "volume table was restored");
>     442                  }
>     443
>     444                  /* Both LEB 1 and LEB 2 are OK and consistent */
>     445                  vfree(leb[1]);
>     446                  return leb[0];
>     447          } else {
>     448                  /* LEB 0 is corrupted or does not exist */
>     449                  if (leb[1]) {
>                              ^^^^^^
> Is this NULL check required?

Yes, leb 1 could be corrupted(eg. bad ec header), then the leb 1 won't 
be scanned during attaching, which means 'leb[1]' won't be initialized 
by process_lvol().
> 
>     450                          leb_corrupted[1] = vtbl_check(ubi, leb[1]);
>     451                          if (leb_corrupted[1] < 0)
>     452                                  goto out_free;
>     453                  }
>     454                  if (leb_corrupted[1]) {
>     455                          /* Both LEB 0 and LEB 1 are corrupted */
>     456                          ubi_err(ubi, "both volume tables are corrupted");
>     457                          goto out_free;
>     458                  }
>     459
>     460                  ubi_warn(ubi, "volume table copy #1 is corrupted");
>                                                           ^^
> This should say #0 I think?

I guess that '#1' refers to leb 0 and '#2' refers to leb 1.
> 
>     461                  err = create_vtbl(ubi, ai, 0, leb[1]);
>                                                        ^^^^^^
>     462                  if (err)
>     463                          goto out_free;
>     464                  ubi_msg(ubi, "volume table was restored");
>     465
>     466                  vfree(leb[0]);
>     467                  return leb[1];
>                                 ^^^^^^
> The NULL check on line 449 makes Smatch think this can be NULL.

In theory, the 'NULL' won't be a return value, these cases are avoided 
by leb_corrupted checking branches.
> 
>     468          }
>     469
>     470  out_free:
>     471          vfree(leb[0]);
>     472          vfree(leb[1]);
>     473          return ERR_PTR(err);
>     474  }
> 
> 
> regards,
> dan carpenter
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> .
>