This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git

commit 95ca3e7ca5f31d63d3572c266b2bcbd3eee6729a
Author: Theodore Karatapanis <tkaratapa...@census-labs.com>
AuthorDate: Wed Jul 16 12:56:42 2025 +0300

    drivers/misc/optee: Add mmap shm op + remove memfd
    
    The previous approach with memfd has 3 problems:
    1) The close operation on the memfd isn't tied with optee_shm_close,
       therefore close(fd) doesn't free the optee_shm struct allocated
       by the kernel.
    
    2) The kernel unnecessarily maps the file descriptor to its memory,
       however only userspace should need to do that.
    
    3) Since the kernel doesn't need to map the file descriptor we
       don't need to unmap it.
    
    To use anonymous mapping, the prototype of map_anonymous() was
    moved from fs/mmap/fs_anonmap.h to include/nuttx/fs/fs.h. Since
    fs_anonmap.h didn't contain any other information it is deleted.
    
    A type from fs/mmap/fs_rammap.h was moved to the public :
    include/nuttx/fs/fs.h as well.
    
    Signed-off-by: Theodore Karatapanis <tkaratapa...@census-labs.com>
---
 drivers/misc/Kconfig  |   6 +--
 drivers/misc/optee.c  | 121 +++++++++++++++++++++++++++++++-------------------
 fs/mmap/fs_anonmap.c  |   2 +-
 fs/mmap/fs_anonmap.h  |  63 --------------------------
 fs/mmap/fs_mmap.c     |   2 +-
 include/nuttx/fs/fs.h |  25 +++++++++++
 include/nuttx/tee.h   |   1 +
 7 files changed, 106 insertions(+), 114 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1892f84e317..a9fb7092598 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -53,19 +53,19 @@ choice
 config DEV_OPTEE_LOCAL
        bool "OPTEE Local Socket Support"
        depends on NET_LOCAL
-       depends on LIBC_MEMFD_SHMFS
+       depends on FS_ANONMAP
        depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
        bool "OP-TEE RPMSG Socket Support"
        depends on NET_RPMSG
-       depends on LIBC_MEMFD_SHMFS
+       depends on FS_ANONMAP
        depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_SMC
        bool "OP-TEE SMC CC Support"
        depends on ARCH_ARM || ARCH_ARM64
-       depends on LIBC_MEMFD_SHMFS
+       depends on FS_ANONMAP
        depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_NONE
diff --git a/drivers/misc/optee.c b/drivers/misc/optee.c
index d9c04283292..ef883abc421 100644
--- a/drivers/misc/optee.c
+++ b/drivers/misc/optee.c
@@ -138,14 +138,18 @@ struct optee_page_list_entry
  * Private Function Prototypes
  ****************************************************************************/
 
-/* The file operation functions */
+/* File operation functions for /dev/tee* */
 
 static int optee_open(FAR struct file *filep);
 static int optee_close(FAR struct file *filep);
 static int optee_ioctl(FAR struct file *filep, int cmd,
                        unsigned long arg);
 
+/* File operation functions for shm fds. */
+
 static int optee_shm_close(FAR struct file *filep);
+static int optee_shm_mmap(FAR struct file *filep,
+                          FAR struct mm_map_entry_s *map);
 
 /****************************************************************************
  * Private Data
@@ -168,15 +172,8 @@ static const struct file_operations g_optee_ops =
 
 static const struct file_operations g_optee_shm_ops =
 {
-  NULL,            /* open */
-  optee_shm_close, /* close */
-  NULL,            /* read */
-  NULL,            /* write */
-  NULL,            /* seek */
-  NULL,            /* ioctl */
-  NULL,            /* mmap */
-  NULL,            /* truncate */
-  NULL             /* poll */
+  .close = optee_shm_close,
+  .mmap = optee_shm_mmap,
 };
 
 static struct inode g_optee_shm_inode =
@@ -491,6 +488,60 @@ static int optee_shm_close(FAR struct file *filep)
   return 0;
 }
 
+/****************************************************************************
+ * Name: optee_shm_mmap
+ *
+ * Description:
+ *   shm mmap operation
+ *
+ * Parameters:
+ *   filep  - the file instance
+ *   map    - Filled by the userspace, with the mapping parameters.
+ *
+ * Returned Values:
+ *   OK on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static int optee_shm_mmap(FAR struct file *filep,
+                          FAR struct mm_map_entry_s *map)
+{
+  FAR struct optee_shm *shm = filep->f_priv;
+  int32_t ret = OK;
+
+  if ((map->flags & MAP_PRIVATE) && (map->flags & MAP_SHARED))
+    {
+      return -EINVAL;
+    }
+
+  /* Forbid multiple mmaps of the same fd. */
+
+  if (shm->vaddr != 0)
+    {
+      return -EBUSY;
+    }
+
+  /* Forbid allocations with bigger size than what registered with
+   * with optee_ioctl_shm_alloc().
+   */
+
+  if (shm->length < map->length)
+    {
+      return -EINVAL;
+    }
+
+  ret = map_anonymous(map, false);
+
+  if (ret == OK)
+    {
+      DEBUGASSERT(map->vaddr != NULL);
+      shm->vaddr = (uint64_t)map->vaddr;
+      shm->paddr = optee_va_to_pa(map->vaddr);
+    }
+
+  return ret;
+}
+
 /****************************************************************************
  * Name: optee_open
  *
@@ -980,8 +1031,6 @@ optee_ioctl_shm_alloc(FAR struct optee_priv_data *priv,
                       FAR struct tee_ioctl_shm_alloc_data *data)
 {
   FAR struct optee_shm *shm;
-  FAR void *addr;
-  int memfd;
   int ret;
 
   if (!optee_is_valid_range(data, sizeof(*data)))
@@ -989,40 +1038,27 @@ optee_ioctl_shm_alloc(FAR struct optee_priv_data *priv,
       return -EFAULT;
     }
 
-  memfd = memfd_create(OPTEE_SERVER_PATH, O_CREAT | O_CLOEXEC);
-  if (memfd < 0)
+  ret = optee_shm_alloc(priv, NULL, data->size, TEE_SHM_USER_MAP, &shm);
+  if (ret < 0)
     {
-      return get_errno();
+      return ret;
     }
 
-  if (ftruncate(memfd, data->size) < 0)
-    {
-      ret = get_errno();
-      goto err;
-    }
+  ret = file_allocate_from_inode(&g_optee_shm_inode,
+                                 O_CLOEXEC | O_RDOK, 0, shm, 0);
 
-  addr = mmap(NULL, data->size, PROT_READ | PROT_WRITE, MAP_SHARED,
-              memfd, 0);
-  if (addr == MAP_FAILED)
-    {
-      ret = get_errno();
-      goto err;
-    }
+  /* Will free automatically the shm once the descriptor is closed. */
 
-  ret = optee_shm_alloc(priv, addr, data->size, 0, &shm);
   if (ret < 0)
     {
-      goto err_with_mmap;
+      optee_shm_free(shm);
+      return ret;
     }
 
-  data->id = shm->id;
-  return memfd;
+  shm->fd = ret;
 
-err_with_mmap:
-  munmap(addr, data->size);
-err:
-  close(memfd);
-  return ret;
+  data->id = shm->id;
+  return shm->fd;
 }
 
 static int
@@ -1218,15 +1254,15 @@ int optee_shm_alloc(FAR struct optee_priv_data *priv, 
FAR void *addr,
       ptr = addr;
     }
 
-  if (ptr == NULL)
+  if (!(flags & TEE_SHM_USER_MAP) && ptr == NULL)
     {
-      goto err;
+      return -EINVAL;
     }
 
   shm->fd = -1;
   shm->priv = priv;
   shm->vaddr = (uintptr_t)ptr;
-  shm->paddr = optee_va_to_pa((FAR void *)shm->vaddr);
+  shm->paddr = shm->vaddr ? optee_va_to_pa((FAR void *)shm->vaddr) : 0;
   shm->length = size;
   shm->flags = flags;
   shm->id = idr_alloc(priv->shms, shm, 0, 0);
@@ -1292,13 +1328,6 @@ void optee_shm_free(FAR struct optee_shm *shm)
       kmm_free((FAR void *)(uintptr_t)shm->vaddr);
     }
 
-  if (!(shm->flags & (TEE_SHM_ALLOC | TEE_SHM_REGISTER)))
-    {
-      /* allocated by optee_ioctl_shm_alloc(), need to unmap */
-
-      munmap((FAR void *)(uintptr_t)shm->vaddr, shm->length);
-    }
-
   idr_remove(shm->priv->shms, shm->id);
   kmm_free(shm);
 }
diff --git a/fs/mmap/fs_anonmap.c b/fs/mmap/fs_anonmap.c
index 2eacc93c03a..a631696ee9f 100644
--- a/fs/mmap/fs_anonmap.c
+++ b/fs/mmap/fs_anonmap.c
@@ -25,12 +25,12 @@
  ****************************************************************************/
 
 #include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
 #include <nuttx/kmalloc.h>
 #include <nuttx/sched.h>
 #include <assert.h>
 #include <debug.h>
 
-#include "fs_anonmap.h"
 #include "sched/sched.h"
 #include "fs_heap.h"
 
diff --git a/fs/mmap/fs_anonmap.h b/fs/mmap/fs_anonmap.h
deleted file mode 100644
index b00ff8bd804..00000000000
--- a/fs/mmap/fs_anonmap.h
+++ /dev/null
@@ -1,63 +0,0 @@
-/****************************************************************************
- * fs/mmap/fs_anonmap.h
- *
- * SPDX-License-Identifier: Apache-2.0
- *
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.  The
- * ASF licenses this file to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the
- * License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
- * License for the specific language governing permissions and limitations
- * under the License.
- *
- ****************************************************************************/
-
-#ifndef __FS_MMAP_FS_ANONMAP_H
-#define __FS_MMAP_FS_ANONMAP_H
-
-/****************************************************************************
- * Included Files
- ****************************************************************************/
-
-#include <nuttx/config.h>
-#include <sys/types.h>
-#include <nuttx/mm/map.h>
-
-/****************************************************************************
- * Public Function Prototypes
- ****************************************************************************/
-
-/****************************************************************************
- * Name: map_anonymous
- *
- * Description:
- *   Support simulation of private anonymous mapping by allocating memory
- *   from heap
- *
- * Input Parameters:
- *   map     Input struct containing user request
- *   kernel  fs_heap_zalloc or kumm_zalloc
- *
- * Returned Value:
- *   On success returns 0. Otherwise negated errno is returned appropriately.
- *
- *     ENOMEM
- *       Insufficient memory is available to simulate mapping
- *
- ****************************************************************************/
-
-#ifdef CONFIG_FS_ANONMAP
-int map_anonymous(FAR struct mm_map_entry_s *entry, bool kernel);
-#else
-#  define map_anonymous(entry, kernel) (-ENOSYS)
-#endif /* CONFIG_FS_ANONMAP */
-
-#endif /* __FS_MMAP_FS_ANONMAP_H */
diff --git a/fs/mmap/fs_mmap.c b/fs/mmap/fs_mmap.c
index ed0c847acfe..1739b03d36c 100644
--- a/fs/mmap/fs_mmap.c
+++ b/fs/mmap/fs_mmap.c
@@ -35,10 +35,10 @@
 #include <debug.h>
 
 #include <nuttx/kmalloc.h>
+#include <nuttx/fs/fs.h>
 
 #include "inode/inode.h"
 #include "fs_rammap.h"
-#include "fs_anonmap.h"
 
 /****************************************************************************
  * Private Functions
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index 992c4c69ff8..f54d0bd324a 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -1938,6 +1938,31 @@ int file_pipe(FAR struct file *filep[2], size_t bufsize, 
int flags);
 int nx_mkfifo(FAR const char *pathname, mode_t mode, size_t bufsize);
 #endif
 
+/****************************************************************************
+ * Name: map_anonymous
+ *
+ * Description:
+ *   Support simulation of private anonymous mapping by allocating memory
+ *   from heap
+ *
+ * Input Parameters:
+ *   map     Input struct containing user request
+ *   kernel  fs_heap_zalloc or kumm_zalloc
+ *
+ * Returned Value:
+ *   On success returns 0. Otherwise negated errno is returned appropriately.
+ *
+ *     ENOMEM
+ *       Insufficient memory is available to simulate mapping
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_FS_ANONMAP
+int map_anonymous(FAR struct mm_map_entry_s *entry, bool kernel);
+#else
+#  define map_anonymous(entry, kernel) (-ENOSYS)
+#endif /* CONFIG_FS_ANONMAP */
+
 #undef EXTERN
 #if defined(__cplusplus)
 }
diff --git a/include/nuttx/tee.h b/include/nuttx/tee.h
index 860021e3fb3..8534d7e5b12 100644
--- a/include/nuttx/tee.h
+++ b/include/nuttx/tee.h
@@ -382,6 +382,7 @@ struct tee_iocl_supp_send_arg
 
 #define TEE_SHM_ALLOC    (1 << 0) /* Kernel-malloced and must freed */
 #define TEE_SHM_REGISTER (1 << 1) /* Registered with TEE OS */
+#define TEE_SHM_USER_MAP (1 << 2) /* Will be used by userspace after mmap */
 
 /* struct tee_ioctl_shm_register_data - Shared memory register argument
  * addr:      [in] Start address of shared memory to register

Reply via email to