Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

Zhihao Cheng
2024-06-22T02:38:28Z

在 2024/6/21 19:36, Gagan Sidhu 写道:
>
>> On Jun 20, 2024, at 10:55 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> 在 2024/6/21 12:27, Gagan Sidhu 写道:
>>>> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>
>>>> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>>>>> Thanks,
>>>>> Gagan
>>>>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>>>
>>>>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>>>>> hi zhihao,
>>>>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>>>>>
>>>>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>>>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>>>>> From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>>>>> Q1. According to previous talking, the booting configuration is
>>>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>>>>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>>>>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
>>>>
>>>> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
>>>> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?
>>> as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.
>>
>> Yes, openwrt knows nothing under the mtd layer(Whatever the mtd is generated by a gluebi device or the mtd is a real physical nand flash). I mean, the layers' inforamtion on your device could be(The squashfs image is stored in UBI volume ubi0_0.):
>>
>> ↗ ubiblock0_0
>> mtdX(nand) -> UBI(holds volume ubi0_0)
>> ↘ mtd12(gluebi) -> mtdblock12
>>
>> The openwrt only sees ubiblock0_0, mtd12 and mtdblock12(The ubi0_0 and gluebi are not awared by openwrt). The openwrt detects that mtd12 has label 'linux,rootfs' in device tree, so it tries mouting mtdblock12 without checking the existence of mtdblock12.
>> Could that be possible?
>
> just to share the relevant part of the log again:
> ```
> [ 3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
> [ 3.202005] Creating 9 MTD partitions on "MT7621-NAND":
> [ 3.212430] 0x000000000000-0x000000080000 : "Bootloader"
> [ 3.224024] 0x0000000c0000-0x000000100000 : "Config"
> [ 3.234684] 0x000000100000-0x000000140000 : "Factory"
> [ 3.245518] 0x000000140000-0x000000180000 : "Config2"
> [ 3.256379] 0x000000180000-0x000002d80000 : "sysv"
> [ 3.895176] 1 squashfs-split partitions found on MTD device sysv
> [ 3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
> [ 3.920925] 2 uimage-fw partitions found on MTD device sysv
> [ 3.932031] Creating 2 MTD partitions on "sysv":
> [ 3.941232] 0x000000000000-0x000000400000 : "kernel"
> [ 3.951995] 0x000000400000-0x000002c00000 : "ubi"
> [ 3.962325] 0x000002d80000-0x000004d80000 : "private"
> [ 3.973322] 0x000004d80000-0x000007580000 : "firmware2"
> [ 3.984759] 0x000007580000-0x000007b80000 : "mydlink"
> [ 3.995699] 0x000007b80000-0x000008000000 : "reserved"
> [ 4.006687] [mtk_nand] probe successfully!
> ```
> and
>
> ```
> [ 5.462504] auto-attach mtd7
> [ 5.462525] ubi0: default fastmap pool size: 15
> [ 5.477309] ubi0: default fastmap WL pool size: 7
> [ 5.486683] ubi0: attaching mtd7
> [ 5.811240] UBI: EOF marker found, PEBs from 273 will be erased
> [ 5.811299] ubi0: scanning is finished
> [ 5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
> [ 5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
> [ 5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
> [ 5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
> [ 5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
> [ 5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
> [ 5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
> [ 5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
> [ 5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
> [ 6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
> [ 6.021426] rootfs: parsing partitions cmdlinepart
> [ 6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
> [ 6.043694] rootfs: got parser (null)
> [ 6.051426] mtd: device 12 (rootfs) set to be root filesystem
> [ 6.062891] rootfs_data: parsing partitions cmdlinepart
> [ 6.073669] rootfs_data: got parser (null)
> [ 6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
> [ 6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
> [ 6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
> [ 6.297406] Please append a correct "root=" boot option; here are the available partitions:
> [ 6.314054] 1f00 512 mtdblock0
> [ 6.314060] (driver?)
> [ 6.327077] 1f01 256 mtdblock1
> [ 6.327081] (driver?)
> [ 6.340101] 1f02 256 mtdblock2
> [ 6.340105] (driver?)
> [ 6.353124] 1f03 256 mtdblock3
> [ 6.353129] (driver?)
> [ 6.366153] 1f04 45056 mtdblock4
> [ 6.366158] (driver?)
> [ 6.379175] 1f05 40572 mtdblock5
> [ 6.379179] (driver?)
> [ 6.392217] 1f06 4096 mtdblock6
> [ 6.392222] (driver?)
> [ 6.405240] 1f07 40960 mtdblock7
> [ 6.405244] (driver?)
> [ 6.418272] 1f08 32768 mtdblock8
> [ 6.418277] (driver?)
> [ 6.431296] 1f09 40960 mtdblock9
> [ 6.431300] (driver?)
> [ 6.444324] 1f0a 6144 mtdblock10
> [ 6.444328] (driver?)
> [ 6.457518] 1f0b 4608 mtdblock11
> [ 6.457523] (driver?)
> [ 6.470720] fe00 33604 ubiblock0_0
> [ 6.470724] (driver?)
> [ 6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
> ```
> openwrt does see ubi0_0 because it has to, in order to create ubiblock0_0.
>
> regarding openwrt not checking the existence of mtdblock12: i think what richard said earlier was that ubiblock0_0 _is_ mtdblock12. as i understand it, the ubiblock0_0 is merely a convenient label for /dev/mtdblock12

I think they are two different devices, according above log:
1. 'block ubiblock0_0: created from ubi0:0(rootfs)', which comes from
https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c#L430,
and it means that the ubiblock0_0 is generated from volume ubi0_0(which
has label rootfs)
2. 'VFS: Unable to mount root fs on unknown-block(31,12)', it means that
the major device number is 31, it should be a mtdblock according to
https://github.com/torvalds/linux/blob/master/include/uapi/linux/major.h#L56.
The mtdblock device is generated from a mtd char device, according to
https://github.com/torvalds/linux/blob/master/drivers/mtd/mtd_blkdevs.c#L350.
>
> that’s why i was complaining and posted the error log, because VFS seemed to be mounting the right partition but was failing.
>
> i could be wrong on this, though.
>
>>
>>> the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
>>> -however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.
>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD
>>> openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
>>> openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.
>>> split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
>>> -i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.
>
> .