nor flash bug in UBIFS

[Qi Wang (qiwang)]

Dears:
I am qiwang come from Micron. When I check UBIFS source code, I find there is a place may has potential risk. 

There is a “nor_erase_prepare” function, it will be called before erase a NOR block.
This “nor_erase_prepare” function will write ‘0’ into this block first, and if program failed, it will read and check the headers. as below:

                /*
                * It is important to first invalidate the EC header, and then the VID
                * header. Otherwise a power cut may lead to valid EC header and
                * invalid VID header, in which case UBI will treat this PEB as
                * corrupted and will try to preserve it, and print scary warnings.
                */
                addr = (loff_t)pnum * ubi->peb_size;
                err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
                if (!err) {
                                addr += ubi->vid_hdr_aloffset;
                                err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
                                if (!err)
                                                return 0;
                }

                /*
                * We failed to write to the media. This was observed with Spansion
                * S29GL512N NOR flash. Most probably the previously eraseblock erasure
                * was interrupted at a very inappropriate moment, so it became
                * unwritable. In this case we probably anyway have garbage in this
                * PEB.
                */
                err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
                if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
                    err1 == UBI_IO_FF) {
                                struct ubi_ec_hdr ec_hdr;

                                err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
                                if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
                                    err1 == UBI_IO_FF)
                                                /*
                                                * Both VID and EC headers are corrupted, so we can
                                                * safely erase this PEB and not afraid that it will be
                                                * treated as a valid PEB in case of an unclean reboot.
                                                */
                                                return 0;
                }


As the comment in source code mentioned, “* We failed to write to the media. This was observed with SpansionS29GL512N NOR flash. Most probably the previously erase block erasure was interrupted at a very inappropriate moment, so it became unwritable.” 
Also for Micron NOR flash, the block is unwritable when eraseblock erasure is interrupted. 

But I want to say the potential risk is if low level driver program data to this block, it will get “timeout error”. And the timeout period could be very long(almost several minutes), during this period, any operation on NOR flash cannot be accept. so program data to a erasure interrupted block isn’t a sensible action. 
in order to avoid program a erasure interrupted block, I suggest UBIFS can read this block before program data. the code changing as below:

                /*
                * We failed to write to the media. This was observed with Spansion
                * S29GL512N NOR flash. Most probably the previously eraseblock erasure
                * was interrupted at a very inappropriate moment, so it became
                * unwritable. In this case we probably anyway have garbage in this
                * PEB.
                */
                err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
                if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
                    err1 == UBI_IO_FF) {
                                struct ubi_ec_hdr ec_hdr;

                                err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
                                if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
                                    err1 == UBI_IO_FF)
                                                /*
                                                * Both VID and EC headers are corrupted, so we can
                                                * safely erase this PEB and not afraid that it will be
                                                * treated as a valid PEB in case of an unclean reboot.
                                                */
                                                return 0;
                }


/*
                * It is important to first invalidate the EC header, and then the VID
                * header. Otherwise a power cut may lead to valid EC header and
                * invalid VID header, in which case UBI will treat this PEB as
                * corrupted and will try to preserve it, and print scary warnings.
                */
                addr = (loff_t)pnum * ubi->peb_size;
                err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
                if (!err) {
                                addr += ubi->vid_hdr_aloffset;
                                err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
                                if (!err)
                                                return 0;
                }



Looking forward to received your reply. I am glad to talk with you if you have any different idea. 

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang at micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131