| Message ID | 20230306013308.3884777-1-chengzhihao1@huawei.com |
|---|---|
| State | Accepted |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | [-next,v3] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size | expand |
> 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>
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> >
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
----- 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
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
----- 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
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);
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(-)