xiaoxiang781216 commented on code in PR #18902:
URL: https://github.com/apache/nuttx/pull/18902#discussion_r3278203692


##########
fs/inode/fs_inode.c:
##########
@@ -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))

Review Comment:
   call inode_checkdirperm directly



##########
fs/inode/fs_inode.c:
##########
@@ -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)

Review Comment:
   move out of if/else to avoid the dup with line 257-265



-- 
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]

Reply via email to