linux-kernel - Re: [PATCH 1/1] mtd: ubi: skip programming unused bits in ubi headers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAyq3Sa=7d6We2xz6bEb_x_zBv+fJwEZuSsfti+S46t3JVo7sw@mail.gmail.com>
Date: Thu, 6 Nov 2025 17:45:27 +0800
From: Cheng Ming Lin <linchengming884@...il.com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: richard@....at, chengzhihao1@...wei.com, vigneshr@...com, 
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	alvinzhou@...c.com.tw, leoyu@...c.com.tw, 
	Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH 1/1] mtd: ubi: skip programming unused bits in ubi headers

Hi Miquel,

Miquel Raynal <miquel.raynal@...tlin.com> 於 2025年11月6日 週四 下午5:10寫道：
>
> Hello,
>
> On 06/11/2025 at 13:49:40 +08, Cheng Ming Lin <linchengming884@...il.com> wrote:
>
> > From: Cheng Ming Lin <chengminglin@...c.com.tw>
> >
> > This patch prevents unnecessary programming of bits in ec_hdr and
> > vid_hdr that are not used or read during normal UBI operation. These
> > unused bits are typcially already set to 1 in erased flash and do not
> > need to be explicitly programmed to 0 if they are not used.
> >
> > Programming such unused areas offers no functional benefit and may
> > result in unnecessary flash wear, reducing the overall lifetime of the
> > device. By skipping these writes, we preserve the flash state as much as
> > possible and minimize wear caused by redundant operations.
> >
> > This change ensures that only necessary fields are written when preparing
> > UBI headers, improving flash efficiency without affecting functionality.
> >
> > Additionally, the Kioxia TC58NVG1S3HTA00 datasheet (page 63) also notes
> > that continuous program/erase cycling with a high percentage of '0' bits
> > in the data pattern can accelerate block endurance degradation.
> > This further supports avoiding large 0x00 patterns.
> >
> > Link: https://europe.kioxia.com/content/dam/kioxia/newidr/productinfo/datasheet/201910/DST_TC58NVG1S3HTA00-TDE_EN_31442.pdf
> >
> > Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
>
> Thanks for this very clear and detailed commit log, as well as for the
> well written cover letter. I am personally fine with the overall idea of
> clearing these unused bits to 1. Yet, I have one concern regarding the
> implementation, please see below.
>
> > ---
> >  drivers/mtd/ubi/io.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
> > index a4999bce4..c21242a14 100644
> > --- a/drivers/mtd/ubi/io.c
> > +++ b/drivers/mtd/ubi/io.c
> > @@ -868,6 +868,8 @@ int ubi_io_write_ec_hdr(struct ubi_device *ubi, int pnum,
> >               return -EROFS;
> >       }
> >
> > +     memset((char *)ec_hdr + UBI_EC_HDR_SIZE, 0xFF, ubi->ec_hdr_alsize - UBI_EC_HDR_SIZE);
> > +
> >       err = ubi_io_write(ubi, ec_hdr, pnum, 0, ubi->ec_hdr_alsize);
> >       return err;
> >  }
> > @@ -1150,6 +1152,11 @@ int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,
> >               return -EROFS;
> >       }
> >
> > +     if (ubi->vid_hdr_shift)
> > +             memset((char *)p, 0xFF, ubi->vid_hdr_alsize - UBI_VID_HDR_SIZE);
> > +     else
> > +             memset((char *)p + UBI_VID_HDR_SIZE, 0xFF, ubi->vid_hdr_alsize - UBI_VID_HDR_SIZE);
>
> Here I am reaching the limits of my UBI knowledge, so I would prefer
> Richard to (in)validate what I am saying, but AFAIU, the VID header can
> be literally anywhere in the page, not just at the start or end of a
> subpage, so in the vid_hdr_shift I would expect some extra maths to
> happen, no?
>
> Here is an excerpt of the main comment at the top of the io.c file:
>
>     * As it was noted above, the VID header may start at a non-aligned
>     * offset. For example, in case of a 2KiB page NAND flash with a 512
>     * bytes sub-page, the VID header may reside at offset 1984 which is
>     * the last 64 bytes of the * last sub-page (EC header is always at
>     * offset zero).
>

Here I was considering the comment in io.c, and also the definitions in
ubi.h, which say:

 * @vid_hdr_offset: starting offset of the volume identifier header (might
 *                  be unaligned)
 * @vid_hdr_aloffset: starting offset of the VID header aligned to
 *                    @hdrs_min_io_size
 * @vid_hdr_shift: contains @vid_hdr_offset - @vid_hdr_aloffset

So, if ubi->vid_hdr_shift is non-zero, then the VID header is not located
at the beginning of the subpage. In this case, we fill 0xFF from the start
of the buffer until the actual vid_hdr_start.

If ubi->vid_hdr_shift is zero, then the VID header is already placed at
the beginning of the subpage. Then we fill 0xFF only after the header
region in the buffer.

> I am not sure this is super common today though.
>
> Thanks,
> Miquèl

Thanks,
Cheng Ming Lin