casaroli commented on code in PR #20822:
URL: https://github.com/apache/nuttx/pull/20822#discussion_r4399673696
##########
arch/arm64/src/common/arm64_fatal.c:
##########
@@ -675,19 +831,31 @@ uint64_t *arm64_fatal_handler(uint64_t *regs)
* the register dump to the paths that are fatal.
*/
- ptcb = nxsched_get_tcb(tcb->group->tg_pid);
+ if (report)
+ {
+ ptcb = nxsched_get_tcb(tcb->group->tg_pid);
- _alert("Segmentation fault in %s (PID %d: %s)\n",
- ptcb != NULL ? get_task_name(ptcb) : "<gone>",
- tcb->pid, get_task_name(tcb));
- _alert("Reason: %s - %s\n", reason, desc);
+ _alert("Segmentation fault in %s (PID %d: %s)\n",
+ ptcb != NULL ? get_task_name(ptcb) : "<gone>",
+ tcb->pid, get_task_name(tcb));
+ _alert("Reason: %s - %s\n", reason, desc);
+ }
- tcb->flags |= TCB_FLAG_FORCED_CANCEL;
+ if (resume)
+ {
+ /* Return from memcpy() or memset() to its caller */
Review Comment:
An exit at the fault does not release what the system call holds. The first
version of this PR did that: over 231 faulting children the kernel heap grew by
33816 bytes in 110 allocations (the file reference and the 1 KiB pipe buffer of
a faulting pipe write, and a file reference for each faulting read). With the
return from `memcpy()` and `memset()` the system call finishes, and the growth
is 0 for these cases.
##########
arch/arm64/src/common/arm64_fatal.c:
##########
@@ -73,6 +83,21 @@ struct fatal_handle_info
static int default_debug_handler(uint64_t *regs, uint64_t far, uint64_t esr);
static int default_fatal_handler(uint64_t *regs, uint64_t far, uint64_t esr);
+/****************************************************************************
+ * External Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_BUILD_KERNEL
+/* The code of memcpy() and memset() (libs/libc/machine/arm64). They are
+ * not there if the C versions are in use.
+ */
+
+extern const char arm64_memcpy_start[] weak_data;
Review Comment:
A fault in `strlen()` or `strcpy()` ends the caller at once, as in any other
kernel code: the system does not panic. A return from them is not safe:
`memcpy()` and `memset()` return their first argument, which x0 still holds at
the fault, but `strlen()` returns a length that the faulting call did not
compute. The general solution is a fault fixup for each access to user memory
(copy-in and copy-out helpers that return -EFAULT); NuttX drivers and file
systems use user pointers directly, so that needs a change in each of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
