[PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Zhihao Cheng]

Following process will make ubi attaching failed since commit
1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):

ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
modprobe nandsim id_bytes=$ID
flash_eraseall /dev/mtd0
modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
(dmesg):
  ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
  UBI error: cannot attach mtd0
  UBI error: cannot initialize UBI, error -22

Rework original solution, the key point is making sure
'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
so we should check vid_hdr_shift rather not vid_hdr_offset.
Then, ubi still support (sub)page aligined VID header offset.

Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
 drivers/mtd/ubi/build.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 0904eb40c95f..ad025b2ee417 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -666,12 +666,6 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
 	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
 
-	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
-	    ubi->vid_hdr_alsize)) {
-		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
-		return -EINVAL;
-	}
-
 	dbg_gen("min_io_size      %d", ubi->min_io_size);
 	dbg_gen("max_write_size   %d", ubi->max_write_size);
 	dbg_gen("hdrs_min_io_size %d", ubi->hdrs_min_io_size);
@@ -689,6 +683,21 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 						ubi->vid_hdr_aloffset;
 	}
 
+	/*
+	 * Memory allocation for VID header is ubi->vid_hdr_alsize
+	 * which is described in comments in io.c.
+	 * Make sure VID header shift + UBI_VID_HDR_SIZE not exceeds
+	 * ubi->vid_hdr_alsize, so that all vid header operations
+	 * won't access memory out of bounds.
+	 */
+	if ((ubi->vid_hdr_shift + UBI_VID_HDR_SIZE) > ubi->vid_hdr_alsize) {
+		ubi_err(ubi, "Invalid VID header offset %d, VID header shift(%d)"
+			" + VID header size(%zu) > VID header aligned size(%d).",
+			ubi->vid_hdr_offset, ubi->vid_hdr_shift,
+			UBI_VID_HDR_SIZE, ubi->vid_hdr_alsize);
+		return -EINVAL;
+	}
+
 	/* Similar for the data offset */
 	ubi->leb_start = ubi->vid_hdr_offset + UBI_VID_HDR_SIZE;
 	ubi->leb_start = ALIGN(ubi->leb_start, ubi->min_io_size);
-- 
2.31.1

[PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Zhihao Cheng]

Following process will make ubi attaching failed since commit
1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):

ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
modprobe nandsim id_bytes=$ID
flash_eraseall /dev/mtd0
modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
(dmesg):
  ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
  UBI error: cannot attach mtd0
  UBI error: cannot initialize UBI, error -22

Rework original solution, the key point is making sure
'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
so we should check vid_hdr_shift rather not vid_hdr_offset.
Then, ubi still support (sub)page aligined VID header offset.

Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
 drivers/mtd/ubi/build.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 0904eb40c95f..ad025b2ee417 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -666,12 +666,6 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
 	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
 
-	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
-	    ubi->vid_hdr_alsize)) {
-		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
-		return -EINVAL;
-	}
-
 	dbg_gen("min_io_size      %d", ubi->min_io_size);
 	dbg_gen("max_write_size   %d", ubi->max_write_size);
 	dbg_gen("hdrs_min_io_size %d", ubi->hdrs_min_io_size);
@@ -689,6 +683,21 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 						ubi->vid_hdr_aloffset;
 	}
 
+	/*
+	 * Memory allocation for VID header is ubi->vid_hdr_alsize
+	 * which is described in comments in io.c.
+	 * Make sure VID header shift + UBI_VID_HDR_SIZE not exceeds
+	 * ubi->vid_hdr_alsize, so that all vid header operations
+	 * won't access memory out of bounds.
+	 */
+	if ((ubi->vid_hdr_shift + UBI_VID_HDR_SIZE) > ubi->vid_hdr_alsize) {
+		ubi_err(ubi, "Invalid VID header offset %d, VID header shift(%d)"
+			" + VID header size(%zu) > VID header aligned size(%d).",
+			ubi->vid_hdr_offset, ubi->vid_hdr_shift,
+			UBI_VID_HDR_SIZE, ubi->vid_hdr_alsize);
+		return -EINVAL;
+	}
+
 	/* Similar for the data offset */
 	ubi->leb_start = ubi->vid_hdr_offset + UBI_VID_HDR_SIZE;
 	ubi->leb_start = ALIGN(ubi->leb_start, ubi->min_io_size);
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Nicolas Schichan]

> Following process will make ubi attaching failed since commit
> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
>
> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
> modprobe nandsim id_bytes=$ID
> flash_eraseall /dev/mtd0
> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
> (dmesg):
>   ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
>   UBI error: cannot attach mtd0
>   UBI error: cannot initialize UBI, error -22
>
> Rework original solution, the key point is making sure
> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
> so we should check vid_hdr_shift rather not vid_hdr_offset.
> Then, ubi still support (sub)page aligined VID header offset.
>
> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>  v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
>  drivers/mtd/ubi/build.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)

[...]

Hello,

Having had the problem, and found this patch as a fix, feel free to
add my:

Tested-by: Nicolas Schichan <nschichan@freebox.fr>

-- 
Nicolas Schichan

[PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Nicolas Schichan]

> Following process will make ubi attaching failed since commit
> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
>
> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
> modprobe nandsim id_bytes=$ID
> flash_eraseall /dev/mtd0
> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
> (dmesg):
>   ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
>   UBI error: cannot attach mtd0
>   UBI error: cannot initialize UBI, error -22
>
> Rework original solution, the key point is making sure
> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
> so we should check vid_hdr_shift rather not vid_hdr_offset.
> Then, ubi still support (sub)page aligined VID header offset.
>
> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>  v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
>  drivers/mtd/ubi/build.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)

[...]

Hello,

Having had the problem, and found this patch as a fix, feel free to
add my:

Tested-by: Nicolas Schichan <nschichan@freebox.fr>

-- 
Nicolas Schichan


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Zhihao Cheng]

Hi,
> 
>> Following process will make ubi attaching failed since commit
>> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
>>
>> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
>> modprobe nandsim id_bytes=$ID
>> flash_eraseall /dev/mtd0
>> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
>> (dmesg):
>>    ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
>>    UBI error: cannot attach mtd0
>>    UBI error: cannot initialize UBI, error -22
>>
>> Rework original solution, the key point is making sure
>> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
>> so we should check vid_hdr_shift rather not vid_hdr_offset.
>> Then, ubi still support (sub)page aligined VID header offset.
>>
>> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> ---
>>   v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
>>   drivers/mtd/ubi/build.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
> 
> [...]
> 
> Hello,
> 
> Having had the problem, and found this patch as a fix, feel free to
> add my:
> 

Thanks for testing.

> Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Zhihao Cheng]

Hi,
> 
>> Following process will make ubi attaching failed since commit
>> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
>>
>> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
>> modprobe nandsim id_bytes=$ID
>> flash_eraseall /dev/mtd0
>> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
>> (dmesg):
>>    ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
>>    UBI error: cannot attach mtd0
>>    UBI error: cannot initialize UBI, error -22
>>
>> Rework original solution, the key point is making sure
>> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
>> so we should check vid_hdr_shift rather not vid_hdr_offset.
>> Then, ubi still support (sub)page aligined VID header offset.
>>
>> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> ---
>>   v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
>>   drivers/mtd/ubi/build.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
> 
> [...]
> 
> Hello,
> 
> Having had the problem, and found this patch as a fix, feel free to
> add my:
> 

Thanks for testing.

> Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> 

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Miquel Raynal]

Hello,

chengzhihao1@huawei.com wrote on Sat, 25 Mar 2023 09:02:40 +0800:

> Hi,
> >   
> >> Following process will make ubi attaching failed since commit
> >> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
> >>
> >> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
> >> modprobe nandsim id_bytes=$ID
> >> flash_eraseall /dev/mtd0
> >> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
> >> (dmesg):
> >>    ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
> >>    UBI error: cannot attach mtd0
> >>    UBI error: cannot initialize UBI, error -22
> >>
> >> Rework original solution, the key point is making sure
> >> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
> >> so we should check vid_hdr_shift rather not vid_hdr_offset.
> >> Then, ubi still support (sub)page aligined VID header offset.

					aligned

> >>
> >> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")

This commit has been backported to stable, so it is important this fix
also gets there quickly:

Cc: stable@vger.kernel.org

> >> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> >> ---
> >>   v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
> >>   drivers/mtd/ubi/build.c | 21 +++++++++++++++------
> >>   1 file changed, 15 insertions(+), 6 deletions(-)  
> > 
> > [...]
> > 
> > Hello,
> > 
> > Having had the problem, and found this patch as a fix, feel free to
> > add my:
> >   
> 
> Thanks for testing.
> 
> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>

Same here.

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19

Thanks,
Miquèl

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Miquel Raynal]

Hello,

chengzhihao1@huawei.com wrote on Sat, 25 Mar 2023 09:02:40 +0800:

> Hi,
> >   
> >> Following process will make ubi attaching failed since commit
> >> 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size"):
> >>
> >> ID="0xec,0xa1,0x00,0x15" # 128M 128KB 2KB
> >> modprobe nandsim id_bytes=$ID
> >> flash_eraseall /dev/mtd0
> >> modprobe ubi mtd="0,2048"  # set vid_hdr offset as 2048 (one page)
> >> (dmesg):
> >>    ubi0 error: ubi_attach_mtd_dev [ubi]: VID header offset 2048 too large.
> >>    UBI error: cannot attach mtd0
> >>    UBI error: cannot initialize UBI, error -22
> >>
> >> Rework original solution, the key point is making sure
> >> 'vid_hdr_shift + UBI_VID_HDR_SIZE < ubi->vid_hdr_alsize',
> >> so we should check vid_hdr_shift rather not vid_hdr_offset.
> >> Then, ubi still support (sub)page aligined VID header offset.

					aligned

> >>
> >> Fixes: 1b42b1a36fc946 ("ubi: ensure that VID header offset ... size")

This commit has been backported to stable, so it is important this fix
also gets there quickly:

Cc: stable@vger.kernel.org

> >> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> >> ---
> >>   v2->v3: Use printing format '%zu' for UBI_VID_HDR_SIZE.
> >>   drivers/mtd/ubi/build.c | 21 +++++++++++++++------
> >>   1 file changed, 15 insertions(+), 6 deletions(-)  
> > 
> > [...]
> > 
> > Hello,
> > 
> > Having had the problem, and found this patch as a fix, feel free to
> > add my:
> >   
> 
> Thanks for testing.
> 
> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>

Same here.

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
>> Thanks for testing.
>> 
>> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> 
> Same here.
> 
> Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19

Applied to next, PR will follow soon.

Thanks everyone!
//richard

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
>> Thanks for testing.
>> 
>> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> 
> Same here.
> 
> Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19

Applied to next, PR will follow soon.

Thanks everyone!
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Daniel Golle]

Hi Richard,

On Wed, Mar 29, 2023 at 11:33:40PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
> >> Thanks for testing.
> >> 
> >> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> > 
> > Same here.
> > 
> > Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19
> 
> Applied to next, PR will follow soon.

As stable linux trees are affected I wonder when this will hit
linux-stable, ie. will it be part of 5.15.108, for example?


Cheers


Daniel

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Daniel Golle]

Hi Richard,

On Wed, Mar 29, 2023 at 11:33:40PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
> >> Thanks for testing.
> >> 
> >> > Tested-by: Nicolas Schichan <nschichan@freebox.fr>
> > 
> > Same here.
> > 
> > Tested-by: Miquel Raynal <miquel.raynal@bootlin.com> # v5.10, v4.19
> 
> Applied to next, PR will follow soon.

As stable linux trees are affected I wonder when this will hit
linux-stable, ie. will it be part of 5.15.108, for example?


Cheers


Daniel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Daniel Golle" <daniel@makrotopia.org>
> As stable linux trees are affected I wonder when this will hit
> linux-stable, ie. will it be part of 5.15.108, for example?

Just sent out the PR to Linus.
As soon it is part of the next -rc, stable maintains can pick up the fix.

Thanks,
//richard

Re: [PATCH -next v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Daniel Golle" <daniel@makrotopia.org>
> As stable linux trees are affected I wonder when this will hit
> linux-stable, ie. will it be part of 5.15.108, for example?

Just sent out the PR to Linus.
As soon it is part of the next -rc, stable maintains can pick up the fix.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/