This is an automated email from the ASF dual-hosted git repository.
pussuw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 884b4604c2 Map user memory passed to accept() in kernel build
884b4604c2 is described below
commit 884b4604c248be6f14ef543535b6efe54f7f0c4e
Author: George Poulios <[email protected]>
AuthorDate: Wed Dec 18 19:29:49 2024 +0200
Map user memory passed to accept() in kernel build
Fixes an issue in kernel build where the user addresses passed
to accept() would be accessed when the wrong MMU mappings were
active. A crash would manifest when attempting to accept() on a
TCP server socket for instance under significant load. The accept
event handler would be called by the HP worker upon client
connection. At this point, accept_tcpsender() would attempt to
write to `addr` resulting in a page fault. Reproducibility would
depend on the current system load (num tasks or CPU stress) but
in loaded environments, it would crash almost 100% of the times.
It should be noted that Linux does this the other way around: it
operates on kernel stack allocated data and once done, it copies
them to user. This can also be a viable alternative, albeit with
one extra copy and a little extra memory.
Signed-off-by: George Poulios <[email protected]>
---
fs/socket/accept.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 94 insertions(+), 2 deletions(-)
diff --git a/fs/socket/accept.c b/fs/socket/accept.c
index 110de3957b..45f79bec72 100644
--- a/fs/socket/accept.c
+++ b/fs/socket/accept.c
@@ -38,10 +38,61 @@
#include <nuttx/cancelpt.h>
#include <nuttx/net/net.h>
#include <nuttx/fs/fs.h>
+#include <nuttx/mm/kmap.h>
#include <arch/irq.h>
+#include "sched/sched.h"
#include "fs_heap.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: map_user_mem
+ *
+ * Description:
+ * Map userspace memory to continuous kernel virtual memory using
+ * `kmm_map_user()`.
+ *
+ * Input Parameters:
+ * uaddr The user address to map. Can be NULL, in which case NULL will
+ * be returned through `p_kaddr`.
+ * size The size of the mem region to map
+ * p_kaddr Pointer to the kernel address to return. Cannot be NULL.
+ *
+ * Returned Value:
+ * Returns 0 (OK) on success, or:
+ * -ENOMEM
+ * If there is not enough free memory to create the mapping.
+ * -EFAULT
+ * If `uaddr` points to memory not belonging to the user address space.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_MM_KMAP
+static int map_user_mem(FAR void *uaddr, size_t size, void **p_kaddr)
+{
+ *p_kaddr = uaddr;
+
+ if (uaddr)
+ {
+ *p_kaddr = kmm_map_user(this_task(), uaddr, size);
+
+ if (*p_kaddr == NULL)
+ {
+ return -ENOMEM;
+ }
+ else if (*p_kaddr == uaddr)
+ {
+ return -EFAULT;
+ }
+ }
+
+ return OK;
+}
+#endif
+
/****************************************************************************
* Public Functions
****************************************************************************/
@@ -122,6 +173,8 @@ int accept4(int sockfd, FAR struct sockaddr *addr, FAR
socklen_t *addrlen,
FAR struct socket *psock = NULL;
FAR struct socket *newsock;
FAR struct file *filep;
+ struct sockaddr *kaddr;
+ socklen_t *kaddrlen;
int oflags = O_RDWR;
int errcode;
int newfd;
@@ -137,6 +190,33 @@ int accept4(int sockfd, FAR struct sockaddr *addr, FAR
socklen_t *addrlen,
goto errout;
}
+ /* Map user addresses to kernel virtual memory and check they actually
+ * belong to the user
+ *
+ * NOTE: also check if writeable?
+ * NOTE: possible TOCTOU with contents of addrlen?
+ */
+
+#ifdef CONFIG_MM_KMAP
+ ret = map_user_mem(addr, sizeof(*addr), (void **)&kaddr);
+ if (ret != OK)
+ {
+ errcode = -ret;
+ goto errout;
+ }
+
+ ret = map_user_mem(addrlen, sizeof(*addrlen), (void **)&kaddrlen);
+ if (ret != OK)
+ {
+ kmm_unmap(kaddr);
+ errcode = -ret;
+ goto errout;
+ }
+#else
+ kaddr = addr;
+ kaddrlen = addrlen;
+#endif
+
/* Get the underlying socket structure */
ret = sockfd_socket(sockfd, &filep, &psock);
@@ -146,7 +226,7 @@ int accept4(int sockfd, FAR struct sockaddr *addr, FAR
socklen_t *addrlen,
if (ret < 0)
{
errcode = -ret;
- goto errout;
+ goto errout_with_kmap;
}
newsock = fs_heap_zalloc(sizeof(*newsock));
@@ -156,7 +236,7 @@ int accept4(int sockfd, FAR struct sockaddr *addr, FAR
socklen_t *addrlen,
goto errout_with_filep;
}
- ret = psock_accept(psock, addr, addrlen, newsock, flags);
+ ret = psock_accept(psock, kaddr, kaddrlen, newsock, flags);
if (ret < 0)
{
errcode = -ret;
@@ -185,6 +265,12 @@ int accept4(int sockfd, FAR struct sockaddr *addr, FAR
socklen_t *addrlen,
}
fs_putfilep(filep);
+
+#ifdef CONFIG_MM_KMAP
+ kmm_unmap(kaddr);
+ kmm_unmap(kaddrlen);
+#endif
+
leave_cancellation_point();
return newfd;
@@ -197,6 +283,12 @@ errout_with_alloc:
errout_with_filep:
fs_putfilep(filep);
+errout_with_kmap:
+#ifdef CONFIG_MM_KMAP
+ kmm_unmap(kaddr);
+ kmm_unmap(kaddrlen);
+#endif
+
errout:
leave_cancellation_point();
