Date: Mon, 17 Jun 2024 11:48:25 -0600
From: Gagan Sidhu <broly@....com>
To: Richard Weinberger <richard@....at>
Cc: ZhaoLong Wang <wangzhaolong1@...wei.com>,
 chengzhihao1 <chengzhihao1@...wei.com>,
 dpervushin <dpervushin@...eddedalley.com>,
 linux-kernel <linux-kernel@...r.kernel.org>,
 linux-mtd <linux-mtd@...ts.infradead.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Vignesh Raghavendra <vigneshr@...com>,
 yangerkun <yangerkun@...wei.com>,
 yi zhang <yi.zhang@...wei.com>
Subject: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by
 ftl notifier

https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/gluebi.c#L297

it seems the GLUEBI is setting the mtd to MTD_UBIVOLUME

https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c

where this doesn’t even have the text “mtd” anywhere.

but the boot partition is always the ubiblock device.

so is gluebi taking the same volume and adding the MTD_UBIVOLUME label or something?

that seems a little unusual.

Thanks,
Gagan

> On Jun 17, 2024, at 11:33 AM, Gagan Sidhu <broly@....com> wrote:
> 
> just to highlight this, let’s look at the failed boot with the changes discussed in this patch
> 
> [    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)
> [    6.500892] Rebooting in 1 seconds..
> 
> 
> 
> here, i assume ubiblock0_0 is the device created from CONFIG_MTD_UBI_BLOCK, correct?
> 
> then, i don’t think it’s GLUEBI that is the reason my boot works. i think gluebi is useless now that you mention it, and isn’t the reason everything works.
> 
> as you can see, UBI_BLOCK is the reasno ubiblock0_0 is created.
> 
> this patch prevents this device from being registered/announced. so when ubi tries to set it (correctly) as the root partition (#12), it fails.
> 
> so doesn’t this change affect more than just GLUEBI? it seems to affect UBI_BLOCK as well.
> 
> Thanks,
> Gagan
> 
>> On Jun 17, 2024, at 11:23 AM, Gagan Sidhu <broly@....com> wrote:
>> 
>> 
>>> On Jun 17, 2024, at 10:52 AM, Richard Weinberger <richard@....at> wrote:
>>> 
>>> ----- Ursprüngliche Mail -----
>>>> Von: "Gagan Sidhu" <broly@....com>
>>>> i don’t think my articulation is correct if you interpreted it as that.
>>>> 
>>>> as i understand it, gluebi simply makes it handy when you have a filesystem
>>>> packed within a ubi file, and it will take that file and mount itas a block
>>>> device.
>>> 
>>> There is no such thing as an UBI file. UBI hosts volumes.
>>> You can install into these volumes whatever you want.
>>> Also a file system such as UBIFS, but this seems not to be the case here.
>> that’s correct. the UBI sits underneath so it’s not ubifs. 
>> 
>>> 
>>>> so i would say it’s not MTD->UBI->GLUEBI->MTD->MTDBLOCK
>>>> 
>>>> it’d say it’s more MTD->GLUEBI->MTDBLOCK
>>> 
>>> No. GLUBI emulates a MTD on top of an UBI volume.
>>> So every read/write operation of the filesystem will first to through:
>>> 
>>> 1. block layer
>>> 2. MTDBLOCK (and mtd)
>>> 3. GLUBI
>>> 4. UBI
>>> 5. MTD (this time the real one)
>>> 
>>> Is this really a setup OpenWRT is using?
>>> I'm not saying it's impossible, but far from ideal.
>>> We have UBIBlock for reasons.
>>> 
>> i don’t understand what you mean. i didn’t think this was unusual haha.
>> 
>> all ubiblock does is give me the right to use a read-only filesystem. it doesn’t map the UBI to a block device.
>> 
>> are you saying there is an easy automated solution that allows me to remove gluebi, and maintain functionality? it doesn’t seem so easy.
>> 
>> for example, here is an openwrt setup: https://forum.openwrt.org/t/ubifs-mount-twice-at-booting/126198
>> 
>> so instead of using gluebi, they use an UBIFS. or they use an overlay. but up until that point, it’s similar.
>> 
>> i didn’t think gluebi was the reason this check was problematic.
>> 	- are you saying MTD_UBIVOLUME is only a property of GLUEBI?
>> 
>> these lines seemed more general than that.
>> 
>> my position is this:
>> 
>> 1. ubi seems to take care of everything as long as i name the partition accordingly (here, i pack the ubi file with two volumes, one for the kernel, the other with the rootfs).
>> 2. the change being discussed broke that. 
>> 3. i don’t see how gluebi is the root of the problem though, because i have MTD_UBI_BLOCK enabled as well, so shouldn’t in spite of the change? it does not.
>> 
>> 
>>> Anyway, since the kernel has to be user space friendly and
>>> users seems to use such "odd" stackings I consider reverting this patch.
>>> ZhaoLong Wang, what do you think?
>>> 
>>> Thanks,
>>> //richard
>> 
> 

Powered by blists - more mailing lists