xiaoxiang781216 commented on code in PR #19599:
URL: https://github.com/apache/nuttx/pull/19599#discussion_r3699177911
##########
fs/inode/fs_inoderemove.c:
##########
@@ -82,7 +82,9 @@ static FAR struct inode *inode_unlink(FAR const char *path)
#ifdef CONFIG_FS_PERMISSION
Review Comment:
remove the check
##########
fs/inode/fs_inode.c:
##########
@@ -217,64 +217,138 @@ void inode_runlock(void)
* 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)
+{
+#ifdef CONFIG_FS_PERMISSION
+ if (inode == NULL)
+ {
+ return OK;
+ }
+
+ return fs_checkmode(inode->i_owner, inode->i_group, inode->i_mode, amode);
+#else
+ UNUSED(inode);
+ UNUSED(amode);
+ return OK;
+#endif
+}
+
/****************************************************************************
- * 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;
+ }
+ }
+ 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;
#else
+ UNUSED(inode);
+ UNUSED(amode);
+ UNUSED(flags);
return OK;
Review Comment:
let's change `#else` of inode_checkpathperm/inode_checkperm to empty macro
for optimization
##########
fs/inode/fs_inode.c:
##########
@@ -308,10 +382,8 @@ int inode_checkopenperm(FAR struct inode *inode, int
oflags)
}
#ifdef CONFIG_FS_PERMISSION
Review Comment:
let's remove the check of CONFIG_FS_PERMISSION before ALL
inode_checkperm/inode_checkopenperm/inode_checkpathperm
##########
fs/inode/fs_inode.c:
##########
@@ -217,64 +217,138 @@ void inode_runlock(void)
* 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)
+{
+#ifdef CONFIG_FS_PERMISSION
+ if (inode == NULL)
+ {
+ return OK;
+ }
+
+ return fs_checkmode(inode->i_owner, inode->i_group, inode->i_mode, amode);
+#else
+ UNUSED(inode);
+ UNUSED(amode);
+ return OK;
+#endif
+}
+
/****************************************************************************
- * 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;
+ }
+ }
+ 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;
#else
+ UNUSED(inode);
+ UNUSED(amode);
+ UNUSED(flags);
return OK;
-#endif /* CONFIG_FS_PERMISSION */
+#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.
- *
- * 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.
*
****************************************************************************/
int inode_checkopenperm(FAR struct inode *inode, int oflags)
{
FAR const struct file_operations *ops;
+#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))
+ {
+ if (inode->u.i_mops == NULL || inode->u.i_mops->open == NULL)
+ {
+ return -ENXIO;
+ }
+
+ return OK;
+ }
+#endif
+
if (INODE_IS_NAMEDSEM(inode))
{
#ifdef CONFIG_FS_PERMISSION
Review Comment:
remove too
##########
fs/inode/fs_inodereserve.c:
##########
@@ -229,7 +229,11 @@ int inode_reserve(FAR const char *path,
#ifdef CONFIG_FS_PERMISSION
Review Comment:
remove too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
