nuttx/nuttx-mirror
1
2
Fork 0
mirror of https://github.com/apache/nuttx.git synced 2025-01-13 01:38:36 +08:00
Code Issues Projects Releases Packages Wiki Activity Actions

Map user memory passed to accept() in kernel build

Browse source 
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 <gpoulios@census-labs.com>
This commit is contained in:
George Poulios 2024-12-18 19:29:49 +02:00 committed by Ville Juven
parent 97c0b43d86
commit 884b4604c2
1 changed files with 94 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

96
fs/socket/accept.c
View file

@ -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();