[PATCH 0/4] fs: ubifs: Fix crash and add safeguards
Heiko Schocher
hs at denx.de
Thu Jul 4 06:28:31 CEST 2024
Hello Alexander,
On 03.07.24 12:12, Alexander Dahl wrote:
> Hei hei,
>
> filesystem handling is different in U-Boot and beyond that UBI/UBIFS is
> different from other filesystems in U-Boot. There's UBI and UBIFS code
> ported from Linux (quite old already now, maybe someone wants to update
> that?), and there's "glue code" or "wrapper code" to interface with
> U-Boot scripts, commands, and filesystem handling. The fixes and
> improvements in this patch series are for this U-Boot specific glue
> code.
Yes, the linux base is very old ... patches are welcome!
And for me it is not that easy, as I do not have a hardware with
current mainline U-Boot running on it... I want to update a hardware
I have to current mainline, but I had no time yet for it...
> I'm no filesystem expert, but after days of debugging I'm quite sure the
> bug is in U-Boot since UBIFS support was added in 2009, and it was
> repeated in 2015 when generic filesystem support for UBIFS was added.
> So please review carefully!
Which bug?
> The crashes were not easily reproducible, only with boards using the old
> distroboot _and_ a boot script inspired by (but not equal to) the one
> proposed by RAUC [1], which basically boils down to:
>
> ubifsmount ubi0:boot (from distroboot)
> test -e (from distroboot)
> ubifsmount ubi0:rootfs1 (this time from the boot script,
> triggering a ubifs_umount)
So, you have a special sequence you execute to trigger the bug, good!
In special 2 ubifsmount in a row... may not often needed for booting!
(I ask me, why that is needed? Boottime is not good than...)
BTW: Is this really a good bootcmd in [1] as on *every* boot your
Environment is saved? This is not good for lifetime of your
storage device ... why not using bootcounter?
> Crashes can be triggered more easily, if patch order is changed and
> patch 2 (resetting pointers to NULL after free) comes first, or if patch
> 2 is applied on its own only.
Hmm...
> The fix is in the first patch, and on my boards I see no crashes
> anymore. I also tested all kinds of combinations of calling `ubi part`,
> `ubi detach`, `ubifsmount`, `ubifsumount`, `ubifsls`, `ubifsload`, `ls`,
> `load`, `size`, and `test -e` and got no crashes anymore after the fix.
That sounds good! Hmm.. test -e has nothing to do with ubi/ubifs I think.
On what hardware do you test? Is it in mainline?
> The three additional patches (2 to 4) are more or less safeguards and
> improvements for the future, and come from me trying and my debugging
> code done on the way, more or less optional, but I think nice to have.
I will look at them .. but give me some time, as I am in holidays the
next 2 weeks ... Hmm.. and it would be good to get some Tested-by
from people with hardware...
bye,
Heiko
>
> Greets
> Alex
>
> [1] https://github.com/rauc/rauc/blob/master/contrib/uboot.sh
>
> Alexander Dahl (4):
> fs: ubifs: Fix memleak and double free in u-boot wrapper functions
> fs: ubifs: Set pointers to NULL after free
> fs: ubifs: Make k(z)alloc/kfree symmetric
> fs: ubifs: Add volume mounted check
>
> fs/ubifs/super.c | 8 ++++++--
> fs/ubifs/ubifs.c | 31 +++++++++++++++++++------------
> 2 files changed, 25 insertions(+), 14 deletions(-)
>
>
> base-commit: 65fbdab27224ee3943a89496b21862db83c34da2
>
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
More information about the U-Boot
mailing list