This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 08afc5f2cf7c37d74c65347dc1aa281942722330 Author: Abhishek Mishra <[email protected]> AuthorDate: Sun Aug 2 17:54:38 2026 +0000 fs: enforce path search and unify inode permission helpers Require X_OK on path ancestors and mountpoint gates via inode_checkpathperm(), and use inode_checkperm() for inode mode checks. inode_checkpathperm() takes the inode tree read lock (or INODE_CHECK_LOCKED when the caller already holds it) and can fold a non-zero amode check onto the target inode in the same call. When CONFIG_FS_PERMISSION is disabled, inode_checkperm/inode_checkpathperm are empty macros returning 0 so there is no stub .text cost. Skip mountpoint R/W open-mode checks in inode_checkopenperm(); keep traversal separate from open. Add an optional mountpt_operations.permission hook (tmpfs when CONFIG_FS_PERMISSION) for in-volume DAC without making the VFS mount-crossing path depend on it. Signed-off-by: Abhishek Mishra <[email protected]> --- fs/inode/fs_inode.c | 131 +++++++++++++++++++++++++++++++-------------- fs/inode/fs_inoderemove.c | 6 +-- fs/inode/fs_inodereserve.c | 8 +-- fs/inode/inode.h | 39 ++++++++++---- fs/mount/fs_mount.c | 9 ++++ fs/mount/fs_umount2.c | 11 +++- fs/tmpfs/fs_tmpfs.c | 41 +++++++++++++- fs/vfs/fs_chstat.c | 7 +++ fs/vfs/fs_mkdir.c | 7 +++ fs/vfs/fs_open.c | 12 ++++- fs/vfs/fs_readlink.c | 7 +++ fs/vfs/fs_rename.c | 14 ++++- fs/vfs/fs_rmdir.c | 7 +++ fs/vfs/fs_stat.c | 7 +++ fs/vfs/fs_statfs.c | 7 +++ fs/vfs/fs_unlink.c | 6 +++ include/nuttx/fs/fs.h | 12 +++++ 17 files changed, 271 insertions(+), 60 deletions(-) diff --git a/fs/inode/fs_inode.c b/fs/inode/fs_inode.c index e89eb337489..cc9b4115356 100644 --- a/fs/inode/fs_inode.c +++ b/fs/inode/fs_inode.c @@ -213,61 +213,107 @@ void inode_runlock(void) up_read(&g_inode_lock); } +#ifdef CONFIG_FS_PERMISSION /**************************************************************************** * Name: inode_checkperm * * Description: - * Check 'inode' for 'amode' access on pseudo-filesystem inodes. - * NULL 'inode' (root) and mountpoints are exempt. - * - * Input Parameters: - * inode - Inode to check, 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. + * Check 'inode' for 'amode' access against stored owner/group/mode. + * Applies to pseudoFS nodes and to mountpoint inodes (whose i_mode gates + * traversal into the mounted filesystem). Optional mountpt_operations + * .permission may expose the same policy for in-volume paths; the VFS + * does not call that hook for mount-crossing. * ****************************************************************************/ +int inode_checkperm(FAR struct inode *inode, int amode) +{ + if (inode == NULL) + { + return OK; + } + + return fs_checkmode(inode->i_owner, inode->i_group, inode->i_mode, amode); +} + /**************************************************************************** - * Name: inode_checkperm + * Name: inode_checkpathperm + * + * Description: + * Require X_OK on every ancestor of 'inode', and on 'inode' itself when + * it is a directory or mountpoint that must be traversed. If 'amode' is + * non-zero, also require that access on 'inode'. Takes the inode tree + * read lock unless INODE_CHECK_LOCKED is set in 'flags'. + * ****************************************************************************/ -int inode_checkperm(FAR struct inode *inode, int amode) +int inode_checkpathperm(FAR struct inode *inode, int amode, int flags) { -#ifdef CONFIG_FS_PERMISSION + FAR struct inode *node; + int locked = (flags & INODE_CHECK_LOCKED) != 0; + int ret; if (inode == NULL) { return OK; } - if (INODE_IS_MOUNTPT(inode)) + if (!locked) { - return OK; + inode_rlock(); } - return fs_checkmode(inode->i_owner, inode->i_group, inode->i_mode, amode); + if (INODE_IS_PSEUDODIR(inode) +#ifndef CONFIG_DISABLE_MOUNTPOINT + || INODE_IS_MOUNTPT(inode) +#endif + ) + { + ret = inode_checkperm(inode, X_OK); + if (ret < 0) + { + goto errout; + } + } -#else - return OK; -#endif /* CONFIG_FS_PERMISSION */ + for (node = inode->i_parent; node != NULL; node = node->i_parent) + { + ret = inode_checkperm(node, X_OK); + if (ret < 0) + { + goto errout; + } + } + + if (amode != 0) + { + ret = inode_checkperm(inode, amode); + if (ret < 0) + { + goto errout; + } + } + + ret = OK; + +errout: + if (!locked) + { + inode_runlock(); + } + + return ret; } +#endif /* CONFIG_FS_PERMISSION */ /**************************************************************************** * Name: inode_checkopenperm * * 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, or a negated errno on failure. + * support, then mode bits for non-mountpoint inodes. Mountpoints are not + * mode-checked here for R/W (that would confuse directory bits with file + * open modes); callers use inode_checkpathperm() for traversal. * ****************************************************************************/ @@ -275,22 +321,31 @@ int inode_checkopenperm(FAR struct inode *inode, int oflags) { FAR const struct file_operations *ops; - if (INODE_IS_NAMEDSEM(inode)) +#ifndef CONFIG_DISABLE_MOUNTPOINT + /* Mountpoints: only verify that open exists. Path search / DAC for the + * mount directory is handled by inode_checkpathperm(); per-file DAC is + * the filesystem's responsibility. + */ + + if (INODE_IS_MOUNTPT(inode)) { -#ifdef CONFIG_FS_PERMISSION - return inode_checkperm(inode, R_OK | W_OK); -#else + if (inode->u.i_mops == NULL || inode->u.i_mops->open == NULL) + { + return -ENXIO; + } + return OK; + } #endif + + if (INODE_IS_NAMEDSEM(inode)) + { + return inode_checkperm(inode, R_OK | W_OK); } if (INODE_IS_MQUEUE(inode) || INODE_IS_PSEUDODIR(inode)) { -#ifdef CONFIG_FS_PERMISSION return inode_checkperm(inode, fs_open_amode(oflags)); -#else - return OK; -#endif } ops = inode->u.i_ops; @@ -307,11 +362,5 @@ int inode_checkopenperm(FAR struct inode *inode, int oflags) return -EACCES; } -#ifdef CONFIG_FS_PERMISSION - return inode_checkperm(inode, fs_open_amode(oflags)); - -#else - return OK; -#endif /* CONFIG_FS_PERMISSION */ } diff --git a/fs/inode/fs_inoderemove.c b/fs/inode/fs_inoderemove.c index 5c41948e896..7696bf46184 100644 --- a/fs/inode/fs_inoderemove.c +++ b/fs/inode/fs_inoderemove.c @@ -79,17 +79,17 @@ static FAR struct inode *inode_unlink(FAR const char *path) inode = desc.node; DEBUGASSERT(inode != NULL); -#ifdef CONFIG_FS_PERMISSION if (desc.parent != NULL) { - ret = inode_checkperm(desc.parent, W_OK); + /* Caller holds the inode tree lock. */ + + ret = inode_checkpathperm(desc.parent, W_OK, INODE_CHECK_LOCKED); if (ret < 0) { inode = NULL; goto errout; } } -#endif /* If peer is non-null, then remove the node from the right of * of that peer node. diff --git a/fs/inode/fs_inodereserve.c b/fs/inode/fs_inodereserve.c index f3b3e3b7f12..3082a523b97 100644 --- a/fs/inode/fs_inodereserve.c +++ b/fs/inode/fs_inodereserve.c @@ -226,16 +226,18 @@ int inode_reserve(FAR const char *path, left = desc.peer; parent = desc.parent; -#ifdef CONFIG_FS_PERMISSION if (parent != NULL) { - ret = inode_checkperm(parent, W_OK | X_OK); + /* Traverse ancestors (X_OK), then require write on the parent. + * Caller holds the inode tree lock. + */ + + ret = inode_checkpathperm(parent, W_OK, INODE_CHECK_LOCKED); if (ret < 0) { goto errout_with_search; } } -#endif for (; ; ) { diff --git a/fs/inode/inode.h b/fs/inode/inode.h index 74d9ef89c00..2f938ac81a3 100644 --- a/fs/inode/inode.h +++ b/fs/inode/inode.h @@ -423,31 +423,52 @@ void inode_addref(FAR struct inode *inode); void inode_release(FAR struct inode *inode); +/* Caller already holds the inode tree lock (inode_lock/inode_rlock). */ + +#define INODE_CHECK_LOCKED (1 << 0) + /**************************************************************************** * Name: inode_checkperm * * Description: - * Check 'inode' for 'amode' access on pseudo-filesystem inodes. - * NULL 'inode' (root) and mountpoints are exempt. + * Check 'inode' for 'amode' access against stored owner/group/mode + * (pseudoFS nodes and mountpoint inodes used as traverse gates). + * Empty macros when CONFIG_FS_PERMISSION is disabled (zero cost). * - * Input Parameters: - * inode - Inode to check, or NULL for a root-level path - * amode - Access mode bitmask (R_OK / W_OK / X_OK) + ****************************************************************************/ + +/**************************************************************************** + * Name: inode_checkpathperm * - * Returned Value: - * Zero (OK) on success, or -EACCES if permission is denied. + * Description: + * Enforce directory search (X_OK) on the path leading to 'inode', and + * optionally check 'amode' on 'inode' itself. Requires X_OK on every + * ancestor, and on 'inode' when it is a pseudo directory or mountpoint. + * If 'amode' is non-zero, also requires that access on 'inode'. + * + * Takes the inode tree read lock unless INODE_CHECK_LOCKED is set in + * 'flags' (caller already holds inode_lock/inode_rlock). + * Empty macros when CONFIG_FS_PERMISSION is disabled (zero cost). * ****************************************************************************/ +#ifdef CONFIG_FS_PERMISSION int inode_checkperm(FAR struct inode *inode, int amode); +int inode_checkpathperm(FAR struct inode *inode, int amode, int flags); +#else +# define inode_checkperm(inode, amode) 0 +# define inode_checkpathperm(inode, amode, flags) 0 +# define fs_open_amode(oflags) 0 +#endif /**************************************************************************** * Name: inode_checkopenperm * * 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. + * support, then mode bits for non-mountpoint inodes. Mountpoints skip + * open-mode checks here; callers must use inode_checkpathperm() so + * parent / mountgates still require X_OK to traverse. * * Input Parameters: * inode - The inode to check diff --git a/fs/mount/fs_mount.c b/fs/mount/fs_mount.c index 0f4758c43a9..d46d6ee8c20 100644 --- a/fs/mount/fs_mount.c +++ b/fs/mount/fs_mount.c @@ -403,6 +403,15 @@ int nx_mount(FAR const char *source, FAR const char *target, inode_release(mountpt_inode); goto errout_with_lock; } + + /* Require search on ancestors and write on the mount target. */ + + ret = inode_checkpathperm(mountpt_inode, W_OK, INODE_CHECK_LOCKED); + if (ret < 0) + { + inode_release(mountpt_inode); + goto errout_with_lock; + } } #endif diff --git a/fs/mount/fs_umount2.c b/fs/mount/fs_umount2.c index b030a316e0f..0de3dfb16d8 100644 --- a/fs/mount/fs_umount2.c +++ b/fs/mount/fs_umount2.c @@ -110,9 +110,18 @@ int nx_umount2(FAR const char *target, unsigned int flags) * performed, or a negated error code on a failure. */ - /* Hold the semaphore through the unbind logic */ + /* Hold the inode tree lock across permission checks and unbind, + * matching mount()'s check-under-lock pattern. + */ inode_lock(); + + ret = inode_checkpathperm(mountpt_inode, W_OK, INODE_CHECK_LOCKED); + if (ret < 0) + { + goto errout_with_lock; + } + ret = mountpt_inode->u.i_mops->unbind(mountpt_inode->i_private, &blkdrvr_inode, flags); if (ret < 0) diff --git a/fs/tmpfs/fs_tmpfs.c b/fs/tmpfs/fs_tmpfs.c index ed25f2a50f0..28c76b17d47 100644 --- a/fs/tmpfs/fs_tmpfs.c +++ b/fs/tmpfs/fs_tmpfs.c @@ -180,6 +180,10 @@ static int tmpfs_stat(FAR struct inode *mountpt, FAR const char *relpath, FAR struct stat *buf); static int tmpfs_chstat(FAR struct inode *mountpt, FAR const char *relpath, FAR const struct stat *buf, int flags); +#ifdef CONFIG_FS_PERMISSION +static int tmpfs_permission(FAR struct inode *mountpt, + FAR const char *relpath, int amode); +#endif /**************************************************************************** * Public Data @@ -218,7 +222,14 @@ const struct mountpt_operations g_tmpfs_operations = tmpfs_rmdir, /* rmdir */ tmpfs_rename, /* rename */ tmpfs_stat, /* stat */ - tmpfs_chstat /* chstat */ + tmpfs_chstat, /* chstat */ + NULL, /* syncfs */ + NULL, /* ioctldir */ +#ifdef CONFIG_FS_PERMISSION + tmpfs_permission /* permission */ +#else + NULL /* permission */ +#endif }; /**************************************************************************** @@ -3132,6 +3143,34 @@ errout_with_fslock: #endif } +/**************************************************************************** + * Name: tmpfs_permission + ****************************************************************************/ + +#ifdef CONFIG_FS_PERMISSION +static int tmpfs_permission(FAR struct inode *mountpt, + FAR const char *relpath, int amode) +{ + FAR struct tmpfs_s *fs; + int ret; + + DEBUGASSERT(mountpt != NULL && relpath != NULL); + + fs = mountpt->i_private; + DEBUGASSERT(fs != NULL); + + ret = tmpfs_lock(fs); + if (ret < 0) + { + return ret; + } + + ret = tmpfs_check_pathperm(fs, relpath, strlen(relpath), amode); + tmpfs_unlock(fs); + return ret; +} +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/fs/vfs/fs_chstat.c b/fs/vfs/fs_chstat.c index 1d1828992f5..16b61b257e9 100644 --- a/fs/vfs/fs_chstat.c +++ b/fs/vfs/fs_chstat.c @@ -71,6 +71,13 @@ static int chstat_recursive(FAR const char *path, inode = desc.node; DEBUGASSERT(inode != NULL); + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + inode_release(inode); + goto errout_with_search; + } + /* The way we handle the chstat depends on the type of inode that we * are dealing with. */ diff --git a/fs/vfs/fs_mkdir.c b/fs/vfs/fs_mkdir.c index 253be08f27a..6efc790ed11 100644 --- a/fs/vfs/fs_mkdir.c +++ b/fs/vfs/fs_mkdir.c @@ -100,6 +100,13 @@ int mkdir(const char *pathname, mode_t mode) goto errout_with_inode; } + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + errcode = -ret; + goto errout_with_inode; + } + /* Perform the mkdir operation using the relative path * at the mountpoint. */ diff --git a/fs/vfs/fs_open.c b/fs/vfs/fs_open.c index 0959527defa..bc2024b8a23 100644 --- a/fs/vfs/fs_open.c +++ b/fs/vfs/fs_open.c @@ -170,7 +170,17 @@ static int file_vopen(FAR struct file *filep, FAR const char *path, } #endif - /* Validate operation support and pseudo-filesystem permissions */ + /* Enforce directory search (X_OK) on ancestors / mount gates, then + * validate open modes. inode_checkpathperm() takes the tree read lock + * for the path walk; for non-mountpoints, hold it again around openperm + * so i_mode cannot race with concurrent chmod. + */ + + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + goto errout_with_inode; + } #ifndef CONFIG_DISABLE_MOUNTPOINT if (INODE_IS_MOUNTPT(inode)) diff --git a/fs/vfs/fs_readlink.c b/fs/vfs/fs_readlink.c index 6cdf067d434..3a057ad895d 100644 --- a/fs/vfs/fs_readlink.c +++ b/fs/vfs/fs_readlink.c @@ -94,6 +94,13 @@ ssize_t readlink(FAR const char *path, FAR char *buf, size_t bufsize) node = desc.node; DEBUGASSERT(node != NULL); + ret = inode_checkpathperm(node, 0, 0); + if (ret < 0) + { + errcode = -ret; + goto errout_with_inode; + } + /* An inode was found that includes this path and possibly refers to a * symbolic link. * diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c index 99c66279e77..28afc700089 100644 --- a/fs/vfs/fs_rename.c +++ b/fs/vfs/fs_rename.c @@ -88,6 +88,11 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode, SETUP_SEARCH(&newdesc, newpath, true); + /* Ancestor X_OK was already checked by rename() via + * inode_checkpathperm(oldinode, ...). Still require parent W_OK here + * under the tree lock before mutating. + */ + ret = inode_checkperm(oldparent, W_OK); if (ret < 0) { @@ -305,7 +310,7 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode, } /* Get an inode for the new relpath -- it should lie on the same - * mountpoint + * mountpoint. Path search on oldinode was already enforced by rename(). */ SETUP_SEARCH(&newdesc, newpath, true); @@ -516,6 +521,13 @@ int rename(FAR const char *oldpath, FAR const char *newpath) oldinode = olddesc.node; DEBUGASSERT(oldinode != NULL); + ret = inode_checkpathperm(oldinode, 0, 0); + if (ret < 0) + { + inode_release(oldinode); + goto errout_with_oldsearch; + } + #ifndef CONFIG_DISABLE_MOUNTPOINT /* Verify that the old inode is a valid mountpoint. */ diff --git a/fs/vfs/fs_rmdir.c b/fs/vfs/fs_rmdir.c index f2b53284ddb..f2911085d53 100644 --- a/fs/vfs/fs_rmdir.c +++ b/fs/vfs/fs_rmdir.c @@ -91,6 +91,13 @@ int rmdir(FAR const char *pathname) if (INODE_IS_MOUNTPT(inode) && inode->u.i_mops) { + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + errcode = -ret; + goto errout_with_inode; + } + /* Perform the rmdir operation using the relative path * from the mountpoint. */ diff --git a/fs/vfs/fs_stat.c b/fs/vfs/fs_stat.c index 7e4a29ac79c..27f1b040f08 100644 --- a/fs/vfs/fs_stat.c +++ b/fs/vfs/fs_stat.c @@ -105,6 +105,13 @@ static int stat_recursive(FAR const char *path, inode = desc.node; DEBUGASSERT(inode != NULL); + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + inode_release(inode); + goto errout_with_search; + } + /* The way we handle the stat depends on the type of inode that we * are dealing with. */ diff --git a/fs/vfs/fs_statfs.c b/fs/vfs/fs_statfs.c index 842780d35ad..70d796c3b5f 100644 --- a/fs/vfs/fs_statfs.c +++ b/fs/vfs/fs_statfs.c @@ -110,6 +110,13 @@ int statfs(FAR const char *path, FAR struct statfs *buf) inode = desc.node; DEBUGASSERT(inode != NULL); + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + inode_release(inode); + goto errout_with_search; + } + /* The way we handle the statfs depends on the type of inode that we * are dealing with. */ diff --git a/fs/vfs/fs_unlink.c b/fs/vfs/fs_unlink.c index 8726610a260..e92a9ec99ec 100644 --- a/fs/vfs/fs_unlink.c +++ b/fs/vfs/fs_unlink.c @@ -88,6 +88,12 @@ int nx_unlink(FAR const char *pathname) if (INODE_IS_MOUNTPT(inode) && inode->u.i_mops) { + ret = inode_checkpathperm(inode, 0, 0); + if (ret < 0) + { + goto errout_with_inode; + } + /* Perform the unlink operation using the relative path at the * mountpoint. */ diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index 818f734e5a8..a13e56b04ab 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -387,6 +387,18 @@ struct mountpt_operations CODE int (*ioctldir)(FAR struct inode *mountpt, FAR struct fs_dirent_s *dir, int cmd, unsigned long arg); + + /* Optional DAC check for a path relative to this mountpoint. + * Filesystems may implement this for a common in-volume permission + * entry point. The VFS mount-crossing gate does not call it; entry + * into a volume uses inode_checkpathperm() on the mountpoint inode. + * Filesystems without Unix permissions leave it NULL. + * + * Placed at the end so existing positional initialisers stay valid. + */ + + CODE int (*permission)(FAR struct inode *mountpt, + FAR const char *relpath, int amode); }; #endif /* CONFIG_DISABLE_MOUNTPOINT */
