From 0e9e246d3730b412da140e8526868a17f81ae7f1 Mon Sep 17 00:00:00 2001 From: Abhishek Mishra Date: Tue, 19 May 2026 12:50:29 +0000 Subject: [PATCH] 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 --- fs/inode/fs_inode.c | 184 +++++++++++++++++++++++++++++--------------- fs/inode/inode.h | 37 +++++---- fs/vfs/fs_mkdir.c | 13 ++++ fs/vfs/fs_rename.c | 43 ++++++++++- fs/vfs/fs_unlink.c | 8 ++ 5 files changed, 208 insertions(+), 77 deletions(-) diff --git a/fs/inode/fs_inode.c b/fs/inode/fs_inode.c index 32dee8ff43291..37baef28d0b4b 100644 --- a/fs/inode/fs_inode.c +++ b/fs/inode/fs_inode.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -48,6 +49,70 @@ static rw_semaphore_t g_inode_lock = RWSEM_INITIALIZER; +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: _inode_checkmode + * + * Description: + * Test effective credentials against 'inode' for 'amode' access. + * Kernel threads always pass. + * + * Returned Value: + * Zero (OK) or -EACCES. + * + ****************************************************************************/ + +#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 */ + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -124,49 +189,48 @@ void inode_runlock(void) * 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, 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. - * - * 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. + * 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)) { +#if defined(CONFIG_PSEUDOFS_ATTRIBUTES) && defined(CONFIG_SCHED_USER_IDENTITY) + if (INODE_IS_MOUNTPT(inode)) + { + return OK; + } + + if ((oflags & O_RDOK) != 0) + { + amode |= R_OK; + } + + if ((oflags & O_WROK) != 0) + { + amode |= W_OK; + } + + return _inode_checkmode(inode, amode); +#else return OK; +#endif } ops = inode->u.i_ops; @@ -185,61 +249,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 */ } diff --git a/fs/inode/inode.h b/fs/inode/inode.h index 8eccad2d367cc..3735d9a566cb7 100644 --- a/fs/inode/inode.h +++ b/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 * diff --git a/fs/vfs/fs_mkdir.c b/fs/vfs/fs_mkdir.c index c9be09d10495f..bfd66a2583431 100644 --- a/fs/vfs/fs_mkdir.c +++ b/fs/vfs/fs_mkdir.c @@ -31,6 +31,7 @@ #include #include #include +#include #include @@ -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. diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c index 15442a0a18c1a..11ea28a3b3f61 100644 --- a/fs/vfs/fs_rename.c +++ b/fs/vfs/fs_rename.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -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,30 @@ 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); + + /* inode_find() holds a reference on parnode; RELEASE_SEARCH() only + * frees pardesc.buffer. + */ + + 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 +544,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 { diff --git a/fs/vfs/fs_unlink.c b/fs/vfs/fs_unlink.c index 8726610a26035..a8c70f66b86cc 100644 --- a/fs/vfs/fs_unlink.c +++ b/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.