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 <[email protected]>
---
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
> 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 <[email protected]>
> ---
> 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 <[email protected]>
--
Nicolas Schichan
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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
Hello,
[email protected] 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: [email protected]
> >> Signed-off-by: Zhihao Cheng <[email protected]>
> >> ---
> >> 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 <[email protected]>
Same here.
Tested-by: Miquel Raynal <[email protected]> # v5.10, v4.19
Thanks,
Miquèl
----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <[email protected]>
>> Thanks for testing.
>>
>> > Tested-by: Nicolas Schichan <[email protected]>
>
> Same here.
>
> Tested-by: Miquel Raynal <[email protected]> # v5.10, v4.19
Applied to next, PR will follow soon.
Thanks everyone!
//richard
Hi Richard,
On Wed, Mar 29, 2023 at 11:33:40PM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > Von: "Miquel Raynal" <[email protected]>
> >> Thanks for testing.
> >>
> >> > Tested-by: Nicolas Schichan <[email protected]>
> >
> > Same here.
> >
> > Tested-by: Miquel Raynal <[email protected]> # 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
----- Ursprüngliche Mail -----
> Von: "Daniel Golle" <[email protected]>
> 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