Re: [PATCH] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size
From: kernel test robot
Date: Sat Mar 04 2023 - 02:01:51 EST
Hi Zhihao,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rw-ubifs/next]
[also build test WARNING on linus/master rw-ubifs/fixes v6.2 next-20230303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zhihao-Cheng/ubi-Fix-failure-attaching-when-vid_hdr-offset-equals-to-sub-page-size/20230304-130928
base: https://git.kernel.org/pub/scm/linux/kernel/git/rw/ubifs.git next
patch link: https://lore.kernel.org/r/20230304053056.403325-1-chengzhihao1%40huawei.com
patch subject: [PATCH] ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230304/202303041438.JdnS3veD-lkp@xxxxxxxxx/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c9c8101e019bacf5c9a4e933c22888f07cc2f2c3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zhihao-Cheng/ubi-Fix-failure-attaching-when-vid_hdr-offset-equals-to-sub-page-size/20230304-130928
git checkout c9c8101e019bacf5c9a4e933c22888f07cc2f2c3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/mtd/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Link: https://lore.kernel.org/oe-kbuild-all/202303041438.JdnS3veD-lkp@xxxxxxxxx/
All warnings (new ones prefixed by >>):
drivers/mtd/ubi/build.c: In function 'io_init':
>> drivers/mtd/ubi/build.c:694:30: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
694 | ubi_err(ubi, "Invalid VID header offset %d, VID header shift(%d)"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mtd/ubi/build.c:695:47: note: format string is defined here
695 | " + VID header size(%lu) > VID header aligned size(%d).",
| ~~^
| |
| long unsigned int
| %u
vim +694 drivers/mtd/ubi/build.c
578
579 /**
580 * io_init - initialize I/O sub-system for a given UBI device.
581 * @ubi: UBI device description object
582 * @max_beb_per1024: maximum expected number of bad PEB per 1024 PEBs
583 *
584 * If @ubi->vid_hdr_offset or @ubi->leb_start is zero, default offsets are
585 * assumed:
586 * o EC header is always at offset zero - this cannot be changed;
587 * o VID header starts just after the EC header at the closest address
588 * aligned to @io->hdrs_min_io_size;
589 * o data starts just after the VID header at the closest address aligned to
590 * @io->min_io_size
591 *
592 * This function returns zero in case of success and a negative error code in
593 * case of failure.
594 */
595 static int io_init(struct ubi_device *ubi, int max_beb_per1024)
596 {
597 dbg_gen("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
598 dbg_gen("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
599
600 if (ubi->mtd->numeraseregions != 0) {
601 /*
602 * Some flashes have several erase regions. Different regions
603 * may have different eraseblock size and other
604 * characteristics. It looks like mostly multi-region flashes
605 * have one "main" region and one or more small regions to
606 * store boot loader code or boot parameters or whatever. I
607 * guess we should just pick the largest region. But this is
608 * not implemented.
609 */
610 ubi_err(ubi, "multiple regions, not implemented");
611 return -EINVAL;
612 }
613
614 if (ubi->vid_hdr_offset < 0)
615 return -EINVAL;
616
617 /*
618 * Note, in this implementation we support MTD devices with 0x7FFFFFFF
619 * physical eraseblocks maximum.
620 */
621
622 ubi->peb_size = ubi->mtd->erasesize;
623 ubi->peb_count = mtd_div_by_eb(ubi->mtd->size, ubi->mtd);
624 ubi->flash_size = ubi->mtd->size;
625
626 if (mtd_can_have_bb(ubi->mtd)) {
627 ubi->bad_allowed = 1;
628 ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
629 }
630
631 if (ubi->mtd->type == MTD_NORFLASH)
632 ubi->nor_flash = 1;
633
634 ubi->min_io_size = ubi->mtd->writesize;
635 ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft;
636
637 /*
638 * Make sure minimal I/O unit is power of 2. Note, there is no
639 * fundamental reason for this assumption. It is just an optimization
640 * which allows us to avoid costly division operations.
641 */
642 if (!is_power_of_2(ubi->min_io_size)) {
643 ubi_err(ubi, "min. I/O unit (%d) is not power of 2",
644 ubi->min_io_size);
645 return -EINVAL;
646 }
647
648 ubi_assert(ubi->hdrs_min_io_size > 0);
649 ubi_assert(ubi->hdrs_min_io_size <= ubi->min_io_size);
650 ubi_assert(ubi->min_io_size % ubi->hdrs_min_io_size == 0);
651
652 ubi->max_write_size = ubi->mtd->writebufsize;
653 /*
654 * Maximum write size has to be greater or equivalent to min. I/O
655 * size, and be multiple of min. I/O size.
656 */
657 if (ubi->max_write_size < ubi->min_io_size ||
658 ubi->max_write_size % ubi->min_io_size ||
659 !is_power_of_2(ubi->max_write_size)) {
660 ubi_err(ubi, "bad write buffer size %d for %d min. I/O unit",
661 ubi->max_write_size, ubi->min_io_size);
662 return -EINVAL;
663 }
664
665 /* Calculate default aligned sizes of EC and VID headers */
666 ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
667 ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
668
669 dbg_gen("min_io_size %d", ubi->min_io_size);
670 dbg_gen("max_write_size %d", ubi->max_write_size);
671 dbg_gen("hdrs_min_io_size %d", ubi->hdrs_min_io_size);
672 dbg_gen("ec_hdr_alsize %d", ubi->ec_hdr_alsize);
673 dbg_gen("vid_hdr_alsize %d", ubi->vid_hdr_alsize);
674
675 if (ubi->vid_hdr_offset == 0)
676 /* Default offset */
677 ubi->vid_hdr_offset = ubi->vid_hdr_aloffset =
678 ubi->ec_hdr_alsize;
679 else {
680 ubi->vid_hdr_aloffset = ubi->vid_hdr_offset &
681 ~(ubi->hdrs_min_io_size - 1);
682 ubi->vid_hdr_shift = ubi->vid_hdr_offset -
683 ubi->vid_hdr_aloffset;
684 }
685
686 /*
687 * Memory allocation for VID header is ubi->vid_hdr_alsize
688 * which is described in comments in io.c.
689 * Make sure VID header shift + UBI_VID_HDR_SIZE not exceeds
690 * ubi->vid_hdr_alsize, so that all vid header operations
691 * won't access memory out of bounds.
692 */
693 if ((ubi->vid_hdr_shift + UBI_VID_HDR_SIZE) > ubi->vid_hdr_alsize) {
> 694 ubi_err(ubi, "Invalid VID header offset %d, VID header shift(%d)"
695 " + VID header size(%lu) > VID header aligned size(%d).",
696 ubi->vid_hdr_offset, ubi->vid_hdr_shift,
697 UBI_VID_HDR_SIZE, ubi->vid_hdr_alsize);
698 return -EINVAL;
699 }
700
701 /* Similar for the data offset */
702 ubi->leb_start = ubi->vid_hdr_offset + UBI_VID_HDR_SIZE;
703 ubi->leb_start = ALIGN(ubi->leb_start, ubi->min_io_size);
704
705 dbg_gen("vid_hdr_offset %d", ubi->vid_hdr_offset);
706 dbg_gen("vid_hdr_aloffset %d", ubi->vid_hdr_aloffset);
707 dbg_gen("vid_hdr_shift %d", ubi->vid_hdr_shift);
708 dbg_gen("leb_start %d", ubi->leb_start);
709
710 /* The shift must be aligned to 32-bit boundary */
711 if (ubi->vid_hdr_shift % 4) {
712 ubi_err(ubi, "unaligned VID header shift %d",
713 ubi->vid_hdr_shift);
714 return -EINVAL;
715 }
716
717 /* Check sanity */
718 if (ubi->vid_hdr_offset < UBI_EC_HDR_SIZE ||
719 ubi->leb_start < ubi->vid_hdr_offset + UBI_VID_HDR_SIZE ||
720 ubi->leb_start > ubi->peb_size - UBI_VID_HDR_SIZE ||
721 ubi->leb_start & (ubi->min_io_size - 1)) {
722 ubi_err(ubi, "bad VID header (%d) or data offsets (%d)",
723 ubi->vid_hdr_offset, ubi->leb_start);
724 return -EINVAL;
725 }
726
727 /*
728 * Set maximum amount of physical erroneous eraseblocks to be 10%.
729 * Erroneous PEB are those which have read errors.
730 */
731 ubi->max_erroneous = ubi->peb_count / 10;
732 if (ubi->max_erroneous < 16)
733 ubi->max_erroneous = 16;
734 dbg_gen("max_erroneous %d", ubi->max_erroneous);
735
736 /*
737 * It may happen that EC and VID headers are situated in one minimal
738 * I/O unit. In this case we can only accept this UBI image in
739 * read-only mode.
740 */
741 if (ubi->vid_hdr_offset + UBI_VID_HDR_SIZE <= ubi->hdrs_min_io_size) {
742 ubi_warn(ubi, "EC and VID headers are in the same minimal I/O unit, switch to read-only mode");
743 ubi->ro_mode = 1;
744 }
745
746 ubi->leb_size = ubi->peb_size - ubi->leb_start;
747
748 if (!(ubi->mtd->flags & MTD_WRITEABLE)) {
749 ubi_msg(ubi, "MTD device %d is write-protected, attach in read-only mode",
750 ubi->mtd->index);
751 ubi->ro_mode = 1;
752 }
753
754 /*
755 * Note, ideally, we have to initialize @ubi->bad_peb_count here. But
756 * unfortunately, MTD does not provide this information. We should loop
757 * over all physical eraseblocks and invoke mtd->block_is_bad() for
758 * each physical eraseblock. So, we leave @ubi->bad_peb_count
759 * uninitialized so far.
760 */
761
762 return 0;
763 }
764
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests