royzah commented on code in PR #20415:
URL: https://github.com/apache/nuttx/pull/20415#discussion_r4154606037
##########
include/nuttx/addrenv.h:
##########
@@ -530,6 +536,16 @@ int addrenv_give(FAR struct addrenv_s *addrenv);
void addrenv_drop(FAR struct addrenv_s *addrenv, bool deferred);
+#ifdef CONFIG_BUILD_KERNEL
Review Comment:
Yeah, protected trusts user ptrs too. There the check is vs the userspace
regions, not an addrenv. Follow up PR ok?
##########
include/nuttx/addrenv.h:
##########
@@ -530,6 +536,16 @@ int addrenv_give(FAR struct addrenv_s *addrenv);
void addrenv_drop(FAR struct addrenv_s *addrenv, bool deferred);
+#ifdef CONFIG_BUILD_KERNEL
+bool uaccess_ok(FAR const void *ptr, size_t len);
+bool uaccess_nested(FAR const void *parent, FAR const void *ptr);
+void uaccess_check(FAR const void *ptr, size_t len);
Review Comment:
Will do, access_ok(ptr, len). uaccess_check kills the caller, linux gives
-EFAULT. Want -EFAULT here too?
##########
arch/arm64/src/common/arm64_mmu.c:
##########
@@ -475,6 +500,13 @@ static void init_xlat_tables(const struct arm_mmu_region
*region)
sinfo("mmap: virt %lux phys %lux size %lux\n", virt, phys, size);
#endif
+#ifdef CONFIG_BUILD_KERNEL
+ if (size > 0 && arm64_overlaps_user(virt, size))
+ {
+ PANIC();
Review Comment:
No process behind it, only boot tables and arm64_mmu_set_memregion(). Will
make set_memregion() return -EINVAL like the misaligned case, assert stays at
boot.
##########
drivers/misc/addrenv.c:
##########
@@ -89,6 +89,7 @@ FAR void *up_addrenv_pa_to_va(uintptr_t pa)
return (FAR void *)pa;
}
+#ifndef CONFIG_BUILD_KERNEL
Review Comment:
Cant drop the whole file: up_addrenv_pa_to_va() only lives here for arm64,
riscv and x86_64 (rpmsg, xhci use it), and x86_64 calls
simple_addrenv_initialize(). Only va_to_pa has an MMU walk, so only that steps
aside.
##########
libs/libc/syslog/lib_syslog.c:
##########
@@ -63,7 +65,15 @@ void vsyslog(int priority, FAR const IPTR char *fmt, va_list
ap)
* of structures in the NuttX syscalls does not work.
*/
-#ifdef va_copy
+#if defined(CONFIG_BUILD_KERNEL) && !defined(__KERNEL__)
+ FAR char *msg;
+
+ if (vasprintf(&msg, fmt, ap) >= 0)
Review Comment:
Else the kernel formats it, and a %s pointing at kernel memory gets read
with kernel rights into the log. In the caller those reads stay at user rights.
##########
syscall/syscall_uaccess.c:
##########
@@ -274,6 +279,20 @@ int uaccess_ioctl(int fd, int req, ...)
arg = va_arg(ap, unsigned long);
va_end(ap);
+ switch (req)
+ {
+ case BIOC_XIPBASE:
Review Comment:
Read only yes, but in kernel build the process cant map it. All it gets is
the kernel layout, any use faults.
--
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]
