Tested-by: Mike Marshall <hubcap@xxxxxxxxxxxx>
On Wed, Jan 22, 2025 at 8:46 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> ---
> 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
>
