Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Pali Rohár]

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.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Pali Rohár]

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.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Richard Weinberger]

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

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Richard Weinberger]

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

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Pali Rohár]

On Friday 21 July 2017 22:12:51 Richard Weinberger wrote:
> Pali,
> 
> On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár  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 
> > ---
> >  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 Rohár
pali.ro...@gmail.com

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Pali Rohár]

On Friday 21 July 2017 22:12:51 Richard Weinberger wrote:
> Pali,
> 
> On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár  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 
> > ---
> >  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 Rohár
pali.ro...@gmail.com

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Richard Weinberger]

Pali,

On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár  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 
> ---
>  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.

-- 
Thanks,
//richard

Re: [PATCH 5/5] ubi: Allow to use read-only UBI volume with not enough PEBs

[Richard Weinberger]

Pali,

On Fri, Jun 2, 2017 at 5:43 PM, Pali Rohár  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 
> ---
>  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.

-- 
Thanks,
//richard