diff mbox series

[v3,19/20] orangefs_d_revalidate(): use stable parent inode and name passed by caller

Message ID 20250123014643.1964371-19-viro@zeniv.linux.org.uk (mailing list archive)
State Handled Elsewhere
Headers show
Series [v3,01/20] make sure that DNAME_INLINE_LEN is a multiple of word size | expand

Commit Message

Al Viro Jan. 23, 2025, 1:46 a.m. UTC 
->d_name use is a UAF if the userland side of things can be slowed down
by attacker.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/orangefs/dcache.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Comments

Mike Marshall Jan. 25, 2025, 4:25 p.m. UTC | #1 
Tested-by: Mike Marshall <hubcap@omnibond.com>

On Wed, Jan 22, 2025 at 8:46 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> ->d_name use is a UAF if the userland side of things can be slowed down
> by attacker.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/orangefs/dcache.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/fs/orangefs/dcache.c b/fs/orangefs/dcache.c
> index c32c9a86e8d0..a19d1ad705db 100644
> --- a/fs/orangefs/dcache.c
> +++ b/fs/orangefs/dcache.c
> @@ -13,10 +13,9 @@
>  #include "orangefs-kernel.h"
>
>  /* Returns 1 if dentry can still be trusted, else 0. */
> -static int orangefs_revalidate_lookup(struct dentry *dentry)
> +static int orangefs_revalidate_lookup(struct inode *parent_inode, const struct qstr *name,
> +                                     struct dentry *dentry)
>  {
> -       struct dentry *parent_dentry = dget_parent(dentry);
> -       struct inode *parent_inode = parent_dentry->d_inode;
>         struct orangefs_inode_s *parent = ORANGEFS_I(parent_inode);
>         struct inode *inode = dentry->d_inode;
>         struct orangefs_kernel_op_s *new_op;
> @@ -26,14 +25,14 @@ static int orangefs_revalidate_lookup(struct dentry *dentry)
>         gossip_debug(GOSSIP_DCACHE_DEBUG, "%s: attempting lookup.\n", __func__);
>
>         new_op = op_alloc(ORANGEFS_VFS_OP_LOOKUP);
> -       if (!new_op) {
> -               ret = -ENOMEM;
> -               goto out_put_parent;
> -       }
> +       if (!new_op)
> +               return -ENOMEM;
>
>         new_op->upcall.req.lookup.sym_follow = ORANGEFS_LOOKUP_LINK_NO_FOLLOW;
>         new_op->upcall.req.lookup.parent_refn = parent->refn;
> -       strscpy(new_op->upcall.req.lookup.d_name, dentry->d_name.name);
> +       /* op_alloc() leaves ->upcall zeroed */
> +       memcpy(new_op->upcall.req.lookup.d_name, name->name,
> +                       min(name->len, ORANGEFS_NAME_MAX - 1));
>
>         gossip_debug(GOSSIP_DCACHE_DEBUG,
>                      "%s:%s:%d interrupt flag [%d]\n",
> @@ -78,8 +77,6 @@ static int orangefs_revalidate_lookup(struct dentry *dentry)
>         ret = 1;
>  out_release_op:
>         op_release(new_op);
> -out_put_parent:
> -       dput(parent_dentry);
>         return ret;
>  out_drop:
>         gossip_debug(GOSSIP_DCACHE_DEBUG, "%s:%s:%d revalidate failed\n",
> @@ -115,7 +112,7 @@ static int orangefs_d_revalidate(struct inode *dir, const struct qstr *name,
>          * If this passes, the positive dentry still exists or the negative
>          * dentry still does not exist.
>          */
> -       if (!orangefs_revalidate_lookup(dentry))
> +       if (!orangefs_revalidate_lookup(dir, name, dentry))
>                 return 0;
>
>         /* We do not need to continue with negative dentries. */
> --
> 2.39.5
>
diff mbox series

Patch

diff --git a/fs/orangefs/dcache.c b/fs/orangefs/dcache.c
index c32c9a86e8d0..a19d1ad705db 100644
--- a/fs/orangefs/dcache.c
+++ b/fs/orangefs/dcache.c
@@ -13,10 +13,9 @@ 
 #include "orangefs-kernel.h"
 
 /* Returns 1 if dentry can still be trusted, else 0. */
-static int orangefs_revalidate_lookup(struct dentry *dentry)
+static int orangefs_revalidate_lookup(struct inode *parent_inode, const struct qstr *name,
+				      struct dentry *dentry)
 {
-	struct dentry *parent_dentry = dget_parent(dentry);
-	struct inode *parent_inode = parent_dentry->d_inode;
 	struct orangefs_inode_s *parent = ORANGEFS_I(parent_inode);
 	struct inode *inode = dentry->d_inode;
 	struct orangefs_kernel_op_s *new_op;
@@ -26,14 +25,14 @@  static int orangefs_revalidate_lookup(struct dentry *dentry)
 	gossip_debug(GOSSIP_DCACHE_DEBUG, "%s: attempting lookup.\n", __func__);
 
 	new_op = op_alloc(ORANGEFS_VFS_OP_LOOKUP);
-	if (!new_op) {
-		ret = -ENOMEM;
-		goto out_put_parent;
-	}
+	if (!new_op)
+		return -ENOMEM;
 
 	new_op->upcall.req.lookup.sym_follow = ORANGEFS_LOOKUP_LINK_NO_FOLLOW;
 	new_op->upcall.req.lookup.parent_refn = parent->refn;
-	strscpy(new_op->upcall.req.lookup.d_name, dentry->d_name.name);
+	/* op_alloc() leaves ->upcall zeroed */
+	memcpy(new_op->upcall.req.lookup.d_name, name->name,
+			min(name->len, ORANGEFS_NAME_MAX - 1));
 
 	gossip_debug(GOSSIP_DCACHE_DEBUG,
 		     "%s:%s:%d interrupt flag [%d]\n",
@@ -78,8 +77,6 @@  static int orangefs_revalidate_lookup(struct dentry *dentry)
 	ret = 1;
 out_release_op:
 	op_release(new_op);
-out_put_parent:
-	dput(parent_dentry);
 	return ret;
 out_drop:
 	gossip_debug(GOSSIP_DCACHE_DEBUG, "%s:%s:%d revalidate failed\n",
@@ -115,7 +112,7 @@  static int orangefs_d_revalidate(struct inode *dir, const struct qstr *name,
 	 * If this passes, the positive dentry still exists or the negative
 	 * dentry still does not exist.
 	 */
-	if (!orangefs_revalidate_lookup(dentry))
+	if (!orangefs_revalidate_lookup(dir, name, dentry))
 		return 0;
 
 	/* We do not need to continue with negative dentries. */