On Mon, 2025-09-01 at 15:14 +0000, Alex Markuze wrote:
> ceph_get_reply_dir() may return a different, referenced inode when r_parent is stale and the parent directory lock is not held.
> ceph_fill_trace() used that inode but failed to drop the reference when it differed from req->r_parent, leaking an inode reference.
>
> Keep the directory inode in a local and iput() it at function end if it does not match req->r_parent.
>
> Signed-off-by: Alex Markuze <amarkuze@xxxxxxxxxx>
> ---
> fs/ceph/inode.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 470ee595ecf2..439c08ece283 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1585,6 +1585,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);
> struct ceph_client *cl = fsc->client;
> int err = 0;
> + struct inode *dir = NULL;
Probably, we need to declare the dir pointer before err declaration. What do you
think?
>
> doutc(cl, "%p is_dentry %d is_target %d\n", req,
> rinfo->head->is_dentry, rinfo->head->is_target);
> @@ -1601,7 +1602,11 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> * r_parent may be stale, in cases when R_PARENT_LOCKED is not set,
> * so we need to get the correct inode
> */
> - struct inode *dir = ceph_get_reply_dir(sb, req->r_parent, rinfo);
> + dir = ceph_get_reply_dir(sb, req->r_parent, rinfo);
> + if (IS_ERR(dir)) {
> + err = PTR_ERR(dir);
> + goto done;
> + }
> if (dir) {
> err = ceph_fill_inode(dir, NULL, &rinfo->diri,
> rinfo->dirfrag, session, -1,
> @@ -1869,6 +1874,9 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> &dvino, ptvino);
> }
> done:
> + /* Drop extra ref from ceph_get_reply_dir() if it returned a new inode */
> + if (!IS_ERR(dir) && dir && dir != req->r_parent)
I think it makes sense to check the dir on NULL at first, then on error.
Maybe, we need to name dir variable as parent_dir or simply parent?
Thanks,
Slava.
> + iput(dir);
> doutc(cl, "done err=%d\n", err);
> return err;
> }
