fs/vfs: enforce pseudoFS permissions on mutation operations

Add pseudoFS permission enforcement for unlink(), mkdir(), and rename() VFS mutation operations.

This change validates parent-directory permissions before modifying pseudoFS inode topology and returns -EACCES for unauthorized operations.

The implementation preserves mountpoint filesystem behavior and fixes multiple inode lifetime/search-state issues in the rename path.

Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>

Author: Abhishekmishra2808

fs/inode/fs_inode.c

@@ -27,6 +27,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <unistd.h>
 
 #include <nuttx/fs/fs.h>
 #include <nuttx/rwsem.h>
@@ -121,49 +122,89 @@ void inode_runlock(void)
 }
 
 /****************************************************************************
- * Name: inode_checkperm
+ * Name: _inode_checkmode
  *
  * Description:
- *   Validate that 'inode' can be opened with the access described by
- *   'oflags'.  Two sequential checks are performed:
+ *   Test effective credentials against 'inode' for 'amode' access.
+ *   Kernel threads always pass.
  *
- *   1. Operation-support check (all inode types, unconditional):
- *      Verifies the driver exposes the read/write entry points required by
- *      'oflags'.  Returns -ENXIO when ops are NULL and -EACCES when the
- *      required entry point is absent.  Pseudo-directory inodes
- *      (INODE_IS_PSEUDODIR) are exempted from this step.
+ * Returned Value:
+ *   Zero (OK) or -EACCES.
  *
- *   2. UNIX permission check (pseudo-filesystem inodes only):
- *      Compares effective uid/gid against i_mode owner/group/other bits.
- *      Mountpoint inodes and kernel threads are unconditionally exempted.
- *      Requires CONFIG_PSEUDOFS_ATTRIBUTES and CONFIG_SCHED_USER_IDENTITY;
- *      when either option is disabled this step is a no-op.
+ ****************************************************************************/
+
+#if defined(CONFIG_PSEUDOFS_ATTRIBUTES) && defined(CONFIG_SCHED_USER_IDENTITY)
+static int _inode_checkmode(FAR struct inode *inode, int amode)
+{
+  FAR struct tcb_s *rtcb;
+  mode_t perm;
+  uid_t uid;
+  gid_t gid;
+
+  /* Kernel threads are always granted access */
+
+  rtcb = nxsched_self();
+  if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+    {
+      return OK;
+    }
+
+  /* Use effective credentials */
+
+  DEBUGASSERT(rtcb->group != NULL);
+  uid = rtcb->group->tg_euid;
+  gid = rtcb->group->tg_egid;
+
+  /* Select the applicable permission-bit triplet */
+
+  if (uid == inode->i_owner)
+    {
+      perm = (inode->i_mode >> 6) & 7;
+    }
+  else if (gid == inode->i_group)
+    {
+      perm = (inode->i_mode >> 3) & 7;
+    }
+  else
+    {
+      perm = inode->i_mode & 7;
+    }
+
+  /* Every requested bit must be present in the selected triplet */
+
+  if ((amode & perm) != amode)
+    {
+      return -EACCES;
+    }
+
+  return OK;
+}
+#endif /* CONFIG_PSEUDOFS_ATTRIBUTES && CONFIG_SCHED_USER_IDENTITY */
+
+/****************************************************************************
+ * Name: inode_checkperm
+ *
+ * Description:
+ *   Validate open access to 'inode' for 'oflags'.  Checks driver operation
+ *   support, then pseudo-filesystem mode bits when enabled.  Mountpoints
+ *   are exempt from mode checks.
  *
  * Input Parameters:
  *   inode  - The inode to check
  *   oflags - Open flags (O_RDONLY / O_WRONLY / O_RDWR)
  *
  * Returned Value:
- *   Zero (OK) on success.  Negated errno on failure:
- *     -ENXIO   ops pointer is NULL
- *     -EACCES  required operation not supported, or permission denied
+ *   Zero (OK) on success, or a negated errno on failure.
  *
  ****************************************************************************/
 
 int inode_checkperm(FAR struct inode *inode, int oflags)
 {
 #if defined(CONFIG_PSEUDOFS_ATTRIBUTES) && defined(CONFIG_SCHED_USER_IDENTITY)
-  FAR struct tcb_s *rtcb;
-  mode_t perm;
-  uid_t uid;
-  gid_t gid;
+  int amode = 0;
 #endif
   FAR const struct file_operations *ops;
 
-  /* === Step 1: operation-support check === */
-
-  /* Pseudo-directories carry no ops and are always accessible */
-
   if (INODE_IS_PSEUDODIR(inode))
     {
       return OK;
@@ -185,61 +226,61 @@ int inode_checkperm(FAR struct inode *inode, int oflags)
 
 #if defined(CONFIG_PSEUDOFS_ATTRIBUTES) && defined(CONFIG_SCHED_USER_IDENTITY)
 
-  /* === Step 2: UNIX permission check (pseudo-filesystem inodes only) === */
-
-  /* Mountpoints delegate permission enforcement to the underlying
-   * filesystem
-   */
-
   if (INODE_IS_MOUNTPT(inode))
     {
       return OK;
     }
 
-  /* Kernel threads are always granted access */
+  if ((oflags & O_RDOK) != 0)
+    {
+      amode |= R_OK;
+    }
 
-  rtcb = nxsched_self();
-  if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+  if ((oflags & O_WROK) != 0)
     {
-      return OK;
+      amode |= W_OK;
     }
 
-  /* Use effective credentials */
+  return _inode_checkmode(inode, amode);
 
-  DEBUGASSERT(rtcb->group != NULL);
-  uid = rtcb->group->tg_euid;
-  gid = rtcb->group->tg_egid;
+#endif /* CONFIG_PSEUDOFS_ATTRIBUTES && CONFIG_SCHED_USER_IDENTITY */
 
-  /* Select the applicable permission-bit triplet */
+  return OK;
+}
 
-  if (uid == inode->i_owner)
-    {
-      perm = (inode->i_mode >> 6) & 7;
-    }
-  else if (gid == inode->i_group)
-    {
-      perm = (inode->i_mode >> 3) & 7;
-    }
-  else
-    {
-      perm = inode->i_mode & 7;
-    }
+/****************************************************************************
+ * Name: inode_checkdirperm
+ *
+ * Description:
+ *   Check parent directory 'dir' for 'amode' access on pseudo-filesystem
+ *   inodes.  NULL 'dir' (root) and mountpoints are exempt.
+ *
+ * Input Parameters:
+ *   dir   - Parent directory inode, or NULL for a root-level path
+ *   amode - Access mode bitmask (R_OK / W_OK / X_OK)
+ *
+ * Returned Value:
+ *   Zero (OK) on success, or -EACCES if permission is denied.
+ *
+ ****************************************************************************/
 
-  /* Bit 2 (value 4) = read permission */
+int inode_checkdirperm(FAR struct inode *dir, int amode)
+{
+#if defined(CONFIG_PSEUDOFS_ATTRIBUTES) && defined(CONFIG_SCHED_USER_IDENTITY)
 
-  if (((oflags & O_RDOK) != 0) && ((perm & 4) == 0))
+  if (dir == NULL)
     {
-      return -EACCES;
+      return OK;
     }
 
-  /* Bit 1 (value 2) = write permission */
-
-  if (((oflags & O_WROK) != 0) && ((perm & 2) == 0))
+  if (INODE_IS_MOUNTPT(dir))
     {
-      return -EACCES;
+      return OK;
     }
 
-#endif /* CONFIG_PSEUDOFS_ATTRIBUTES && CONFIG_SCHED_USER_IDENTITY */
+  return _inode_checkmode(dir, amode);
 
+#else
   return OK;
+#endif /* CONFIG_PSEUDOFS_ATTRIBUTES && CONFIG_SCHED_USER_IDENTITY */
 }

fs/inode/inode.h

@@ -422,32 +422,39 @@ void inode_release(FAR struct inode *inode);
  * Name: inode_checkperm
  *
  * Description:
- *   Validate that 'inode' can be opened with the access described by
- *   'oflags'.  Two sequential checks are performed:
- *
- *   1. Operation-support check (all inode types):
- *      Ensures the driver exposes the read/write entry points required by
- *      'oflags'.  Pseudo-directory inodes are exempted.
- *
- *   2. UNIX permission check (pseudo-filesystem inodes only):
- *      Compares effective uid/gid against i_mode owner/group/other bits.
- *      Mountpoint inodes and kernel threads are unconditionally exempted.
- *      Active only when CONFIG_PSEUDOFS_ATTRIBUTES and
- *      CONFIG_SCHED_USER_IDENTITY are both enabled.
+ *   Validate open access to 'inode' for 'oflags'.  Checks driver operation
+ *   support, then pseudo-filesystem mode bits when enabled.  Mountpoints
+ *   are exempt from mode checks.
  *
  * Input Parameters:
  *   inode  - The inode to check
  *   oflags - Open flags (O_RDONLY / O_WRONLY / O_RDWR)
  *
  * Returned Value:
- *   Zero (OK) on success.  Negated errno on failure:
- *     -ENXIO   ops pointer is NULL
- *     -EACCES  required operation not supported, or permission denied
+ *   Zero (OK) on success, or a negated errno on failure.
  *
  ****************************************************************************/
 
 int inode_checkperm(FAR struct inode *inode, int oflags);
 
+/****************************************************************************
+ * Name: inode_checkdirperm
+ *
+ * Description:
+ *   Check parent directory 'dir' for 'amode' access on pseudo-filesystem
+ *   inodes.  NULL 'dir' (root) and mountpoints are exempt.
+ *
+ * Input Parameters:
+ *   dir   - Parent directory inode, or NULL for a root-level path
+ *   amode - Access mode bitmask (R_OK / W_OK / X_OK)
+ *
+ * Returned Value:
+ *   Zero (OK) on success, or -EACCES if permission is denied.
+ *
+ ****************************************************************************/
+
+int inode_checkdirperm(FAR struct inode *dir, int amode);
+
 /****************************************************************************
  * Name: foreach_inode
  *

fs/vfs/fs_mkdir.c

@@ -31,6 +31,7 @@
 #include <stdbool.h>
 #include <assert.h>
 #include <errno.h>
+#include <unistd.h>
 
 #include <nuttx/fs/fs.h>
 
@@ -136,6 +137,18 @@ int mkdir(const char *pathname, mode_t mode)
 
   else
     {
+      /* Verify write+search permission on the parent directory before
+       * adding a new name to the pseudo-filesystem tree.  POSIX requires
+       * both W_OK and X_OK to create a directory entry.
+       */
+
+      ret = inode_checkdirperm(desc.parent, W_OK | X_OK);
+      if (ret < 0)
+        {
+          errcode = -ret;
+          goto errout_with_search;
+        }
+
       /* Create an inode in the pseudo-filesystem at this path.
        * NOTE that the new inode will be created with a reference
        * count of zero.

fs/vfs/fs_rename.c

@@ -33,6 +33,7 @@
 #include <libgen.h>
 #include <assert.h>
 #include <errno.h>
+#include <unistd.h>
 
 #include <nuttx/fs/fs.h>
 #include <nuttx/lib/lib.h>
@@ -66,21 +67,35 @@
 
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
 static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
+                        FAR struct inode *oldparent,
                         FAR const char *newpath)
 {
   struct inode_search_s newdesc;
+  struct inode_search_s pardesc;
   FAR struct inode *newinode;
+  FAR struct inode *parnode;
   FAR char *subdir = NULL;
 #ifdef CONFIG_FS_NOTIFY
   bool isdir = INODE_IS_PSEUDODIR(oldinode);
 #endif
   int ret;
 
+  /* SETUP_SEARCH early so RELEASE_SEARCH at errout is safe. */
+
+  SETUP_SEARCH(&newdesc, newpath, true);
+
+  /* Verify source parent write permission. */
+
+  ret = inode_checkdirperm(oldparent, W_OK);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
   /* According to POSIX, any new inode at this path should be removed
    * first, provided that it is not a directory.
    */
 
-  SETUP_SEARCH(&newdesc, newpath, true);
   ret = inode_find(&newdesc);
   if (ret >= 0)
     {
@@ -156,6 +171,26 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
       inode_release(newinode);
     }
 
+  /* Re-resolve the final destination parent after path rewrite. */
+
+  SETUP_SEARCH(&pardesc, newpath, true);
+  inode_find(&pardesc);   /* pardesc.parent valid even if node not found */
+  parnode = pardesc.node;
+
+  ret = inode_checkdirperm(pardesc.parent, W_OK);
+
+  if (parnode != NULL)
+    {
+      inode_release(parnode);
+    }
+
+  RELEASE_SEARCH(&pardesc);
+
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
   /* Create a new, empty inode at the destination location.
    * NOTE that the new inode will be created with a reference count
    * of  zero.
@@ -505,7 +540,7 @@ int rename(FAR const char *oldpath, FAR const char *newpath)
 #endif /* CONFIG_DISABLE_MOUNTPOINT */
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
     {
-      ret = pseudorename(oldpath, oldinode, newpath);
+      ret = pseudorename(oldpath, oldinode, olddesc.parent, newpath);
     }
 #else
     {

fs/vfs/fs_unlink.c

@@ -121,6 +121,14 @@ int nx_unlink(FAR const char *pathname)
           goto errout_with_inode;
         }
 
+      /* Verify parent-directory write permission before unlink. */
+
+      ret = inode_checkdirperm(desc.parent, W_OK);
+      if (ret < 0)
+        {
+          goto errout_with_inode;
+        }
+
       /* Notify the driver that it has been unlinked.  If there are no
        * open references to the driver instance, then the driver should
        * release all resources because it is no longer accessible.