| Message ID | 1496418222-23483-6-git-send-email-pali.rohar@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Boris Brezillon |
| Headers | show |
Pali, On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár <pali.rohar@gmail.com> wrote: > In read-only mode is skipped auto-resize. For pre-build images ready for > auto-resize there can be reserved more PEBs as whole size of pre-build > image. In read-only we do not do any write operation therefore this would > allow to use read-only UBI volume which is not auto-resized yet. > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > --- > drivers/mtd/ubi/vtbl.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c > index 263743e..1d708c5 100644 > --- a/drivers/mtd/ubi/vtbl.c > +++ b/drivers/mtd/ubi/vtbl.c > @@ -240,8 +240,10 @@ static int vtbl_check(const struct ubi_device *ubi, > if (reserved_pebs > ubi->good_peb_count) { > ubi_err(ubi, "too large reserved_pebs %d, good PEBs %d", > reserved_pebs, ubi->good_peb_count); > - err = 9; > - goto bad; > + if (!ubi->ro_mode) { > + err = 9; > + goto bad; > + } I fear this is not correct, it will disable a legit self-check of UBI volumes. If the read-only volume is corrupted/truncated and you miss PEBs, this check will no longer trigger. Especially when dealing with nanddumps, truncation is a common problem.
On Friday 21 July 2017 22:12:51 Richard Weinberger wrote: > Pali, > > On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár <pali.rohar@gmail.com> wrote: > > In read-only mode is skipped auto-resize. For pre-build images ready for > > auto-resize there can be reserved more PEBs as whole size of pre-build > > image. In read-only we do not do any write operation therefore this would > > allow to use read-only UBI volume which is not auto-resized yet. > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > > --- > > drivers/mtd/ubi/vtbl.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c > > index 263743e..1d708c5 100644 > > --- a/drivers/mtd/ubi/vtbl.c > > +++ b/drivers/mtd/ubi/vtbl.c > > @@ -240,8 +240,10 @@ static int vtbl_check(const struct ubi_device *ubi, > > if (reserved_pebs > ubi->good_peb_count) { > > ubi_err(ubi, "too large reserved_pebs %d, good PEBs %d", > > reserved_pebs, ubi->good_peb_count); > > - err = 9; > > - goto bad; > > + if (!ubi->ro_mode) { > > + err = 9; > > + goto bad; > > + } > > I fear this is not correct, it will disable a legit self-check of UBI volumes. > If the read-only volume is corrupted/truncated and you miss PEBs, this > check will no longer > trigger. > > Especially when dealing with nanddumps, truncation is a common problem. Any idea how to fix it? Or how to handle read-only images which are marked for auto-resize?
Pali, Am 25.07.2017 um 16:27 schrieb Pali Rohár: >> I fear this is not correct, it will disable a legit self-check of UBI volumes. >> If the read-only volume is corrupted/truncated and you miss PEBs, this >> check will no longer >> trigger. >> >> Especially when dealing with nanddumps, truncation is a common problem. > > Any idea how to fix it? Or how to handle read-only images which are > marked for auto-resize? I'd vote for rejecting images that have auto-resize set when the MTD is read-only. In fact, using UBI on top of a read-only MTD is very uncommon and not recommended (for NAND). The auto-resize flag should be also only set when you just have created it using mkfs.ubifs. Why would you inspect such an image with the kernel UBIFS unless you're hunting down a bug in mkfs.ubifs? Thanks, //richard
On Sunday 06 August 2017 11:43:25 Richard Weinberger wrote: > Pali, > > Am 25.07.2017 um 16:27 schrieb Pali Rohár: > >> I fear this is not correct, it will disable a legit self-check of > >> UBI volumes. If the read-only volume is corrupted/truncated and > >> you miss PEBs, this check will no longer > >> trigger. > >> > >> Especially when dealing with nanddumps, truncation is a common > >> problem. > > > > Any idea how to fix it? Or how to handle read-only images which are > > marked for auto-resize? > > I'd vote for rejecting images that have auto-resize set when the MTD > is read-only. In fact, using UBI on top of a read-only MTD is very > uncommon and not recommended (for NAND). The auto-resize flag should > be also only set when you just have created it using mkfs.ubifs. Why > would you inspect such an image with the kernel UBIFS unless you're > hunting down a bug in mkfs.ubifs? > > Thanks, > //richard E.g. because when I get UBIFS image and I want to unpack it. IMO UBIFS image which have auto-resize set is also valid UBIFS image and kernel should be able to read it too, even in R/O mode.
diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c index 263743e..1d708c5 100644 --- a/drivers/mtd/ubi/vtbl.c +++ b/drivers/mtd/ubi/vtbl.c @@ -240,8 +240,10 @@ static int vtbl_check(const struct ubi_device *ubi, if (reserved_pebs > ubi->good_peb_count) { ubi_err(ubi, "too large reserved_pebs %d, good PEBs %d", reserved_pebs, ubi->good_peb_count); - err = 9; - goto bad; + if (!ubi->ro_mode) { + err = 9; + goto bad; + } } if (name_len > UBI_VOL_NAME_MAX) { @@ -652,10 +654,12 @@ static int init_volumes(struct ubi_device *ubi, if (ubi->corr_peb_count) ubi_err(ubi, "%d PEBs are corrupted and not used", ubi->corr_peb_count); - return -ENOSPC; + if (!ubi->ro_mode) + return -ENOSPC; + } else { + ubi->rsvd_pebs += reserved_pebs; + ubi->avail_pebs -= reserved_pebs; } - ubi->rsvd_pebs += reserved_pebs; - ubi->avail_pebs -= reserved_pebs; return 0; }
In read-only mode is skipped auto-resize. For pre-build images ready for auto-resize there can be reserved more PEBs as whole size of pre-build image. In read-only we do not do any write operation therefore this would allow to use read-only UBI volume which is not auto-resized yet. Signed-off-by: Pali Rohár <pali.rohar@gmail.com> --- drivers/mtd/ubi/vtbl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)