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: h...@denx.de
