Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libs/libc/modlib/modlib_bind.c: Fix system crash if modlib_bind fails #15830

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

jlaitine
Copy link
Contributor

@jlaitine jlaitine commented Feb 13, 2025

Summary

Addrenv is changed to the newly created process' one in the beginning of modlib_bind, and needs to be changed back always when returning from the function; also in error cases.

This fixes a system crash in case where linking of a new process fails in modlib_bind. In the error case the modlib_bind returned with error code, but with wrong address environment.

Impact

Fixes a crashing bug in platforms using CONFIG_ARCH_ADDRENV.

Testing

Tested on imx93 and mpfs platforms.

@github-actions github-actions bot added Area: OS Components OS Components issues Size: XS The size of the change in this PR is very small labels Feb 13, 2025
@@ -1076,6 +1076,8 @@ int modlib_bind(FAR struct module_s *modp,
}
#endif

errout_with_addrenv:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove indent

Addrenv is changed to the newly created process' one in the beginning of
modlib_bind, and needs to be changed always when returning from the function;
also in error cases.

Signed-off-by: Jukka Laitinen <[email protected]>
@jlaitine jlaitine force-pushed the fix_system_crash_in_modlib_bind branch from 28154c1 to feb82cc Compare February 13, 2025 08:33
@lupyuen
Copy link
Member

lupyuen commented Feb 13, 2025

@nuttxpr test milkv_duos:nsh

@nuttxpr
Copy link

nuttxpr commented Feb 13, 2025

[Experimental Bot, please feedback here]

Build and Test Successful (milkv_duos:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4806168

$ git clone https://github.com/tiiuae/nuttx nuttx --branch fix_system_crash_in_modlib_bind
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at feb82cc1bd libs/libc/modlib/modlib_bind.c: Fix system crash if modlib_bind fails
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at 7f424c3e8 nshlib/builtin: check background task before restore the signal
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/feb82cc1bd7ac2feab4ea0389a823f30673845c7
NuttX Apps: https://github.com/apache/nuttx-apps/tree/7f424c3e8ddaa0a6a828175b8e4205c1cc0a2a8a
$ cd nuttx
$ tools/configure.sh milkv_duos:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image tftpserver:/tftpboot/Image-sg2000
$ ssh tftpserver ls -l /tftpboot/Image-sg2000
$ cd /home/luppy/nuttx-build-farm
$ ssh tftpserver
OpenSBI v0.9
nsh> uname -a
NuttX 10.4.0 feb82cc1bd Feb 13 2025 17:24:22 risc-v milkv_duos
nsh> ostest
arena       81000    81000
ordblks         2        3
mxordblk    7cff8    78ff8
uordblks     2660     4570
fordblks    7e9a0    7ca90
user_main: Exiting
ostest_main: Exiting with status 0
nsh> Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/oz64-power.sh off
----- Power off Oz64
[]

Copy link
Member

@lupyuen lupyuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested OK on Real Hardware: Oz64 SG2000 RISC-V SBC. Thanks :-)

@anchao anchao merged commit 18c3e76 into apache:master Feb 14, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues Size: XS The size of the change in this PR is very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants