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


##########
fs/vfs/fs_rename.c:
##########
@@ -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);

Review Comment:
   RELEASE_SEARCH() only frees `desc.buffer` (please check 
SETUP_SEARCH/RELEASE_SEARCH in inode.h). inode_find() increments i_crefs only 
on desc->node when the lookup succeeds; it does not take a reference on 
desc->parent.
   So when pardesc.node is non-NULL we still need inode_release(parnode) before 
RELEASE_SEARCH(&pardesc).
   This matches the existing patterns used in `chstat_recursive()` and 
`mountptrename()` (release desc.node, then RELEASE_SEARCH()).



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

Review Comment:
   done !



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