Re: [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size]

[Richard Weinberger <richard@nod.at>]

Uwe,

----- Ursprüngliche Mail -----
> Von: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
> This patch is in mainline as 1b42b1a36fc946f0d7088425b90d491b4257ca3e,
> and backported to various stable releases.
> 
> For me this breaks
> 
>	ubiattach -m 0 -O 2048
> 
> I think the check
> 
>	ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->vid_hdr_alsize
> 
> is wrong. Without -O passed to ubiattach (and dynamic debug enabled) I
> get:
> 
> [ 5294.936762] UBI DBG gen (pid 9619): sizeof(struct ubi_ainf_peb) 56
> [ 5294.936769] UBI DBG gen (pid 9619): sizeof(struct ubi_wl_entry) 32
> [ 5294.936774] UBI DBG gen (pid 9619): min_io_size      2048
> [ 5294.936779] UBI DBG gen (pid 9619): max_write_size   2048
> [ 5294.936783] UBI DBG gen (pid 9619): hdrs_min_io_size 512
> [ 5294.936787] UBI DBG gen (pid 9619): ec_hdr_alsize    512
> [ 5294.936791] UBI DBG gen (pid 9619): vid_hdr_alsize   512
> [ 5294.936796] UBI DBG gen (pid 9619): vid_hdr_offset   512
> [ 5294.936800] UBI DBG gen (pid 9619): vid_hdr_aloffset 512
> [ 5294.936804] UBI DBG gen (pid 9619): vid_hdr_shift    0
> [ 5294.936808] UBI DBG gen (pid 9619): leb_start        2048
> [ 5294.936812] UBI DBG gen (pid 9619): max_erroneous    409
> 
> So the check would only pass for vid_hdr_offset <= 512 -
> UBI_VID_HDR_SIZE; note that even specifying the default value 512 (i.e.
> 
>	ubiattach -m 0 -O 512
> 
> ) fails the check.
> 
> A less strong check would be:
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 0904eb40c95f..69c28a862430 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -666,8 +666,8 @@ 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)) {
> +	if (ubi->vid_hdr_offset &&
> +	    ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->peb_size) {
> 		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
> 		return -EINVAL;
> 	}
> 
> But I'm unsure if this would be too lax?!

As written on IRC, 1e020e1b96af ("ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size") is supposed to fix that
and on it's way into stable.

Thanks,
//richard