On Sat, 2025-06-14 at 07:22 +0100, Al Viro wrote:
> ... and parse_longname() is not guaranteed that. That's the reason
> why it uses kmemdup_nul() to build the argument for kstrtou64();
> the problem is, kstrtou64() is not the only thing that need it.
>
> Just get a NUL-terminated copy of the entire thing and be done
> with that...
>
> Fixes: dd66df0053ef "ceph: add support for encrypted snapshot names"
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> fs/ceph/crypto.c | 31 ++++++++++++-------------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
I did run xfstests with the patch set. I don't see any issues.
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
Thanks,
Slava.
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 3b3c4d8d401e..9c7062245880 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -215,35 +215,31 @@ static struct inode *parse_longname(const struct inode *parent,
> struct ceph_client *cl = ceph_inode_to_client(parent);
> struct inode *dir = NULL;
> struct ceph_vino vino = { .snap = CEPH_NOSNAP };
> - char *inode_number;
> - char *name_end;
> - int orig_len = *name_len;
> + char *name_end, *inode_number;
> int ret = -EIO;
> -
> + /* NUL-terminate */
> + char *str __free(kfree) = kmemdup_nul(name, *name_len, GFP_KERNEL);
> + if (!str)
> + return ERR_PTR(-ENOMEM);
> /* Skip initial '_' */
> - name++;
> - name_end = strrchr(name, '_');
> + str++;
> + name_end = strrchr(str, '_');
> if (!name_end) {
> - doutc(cl, "failed to parse long snapshot name: %s\n", name);
> + doutc(cl, "failed to parse long snapshot name: %s\n", str);
> return ERR_PTR(-EIO);
> }
> - *name_len = (name_end - name);
> + *name_len = (name_end - str);
> if (*name_len <= 0) {
> pr_err_client(cl, "failed to parse long snapshot name\n");
> return ERR_PTR(-EIO);
> }
>
> /* Get the inode number */
> - inode_number = kmemdup_nul(name_end + 1,
> - orig_len - *name_len - 2,
> - GFP_KERNEL);
> - if (!inode_number)
> - return ERR_PTR(-ENOMEM);
> + inode_number = name_end + 1;
> ret = kstrtou64(inode_number, 10, &vino.ino);
> if (ret) {
> - doutc(cl, "failed to parse inode number: %s\n", name);
> - dir = ERR_PTR(ret);
> - goto out;
> + doutc(cl, "failed to parse inode number: %s\n", str);
> + return ERR_PTR(ret);
> }
>
> /* And finally the inode */
> @@ -254,9 +250,6 @@ static struct inode *parse_longname(const struct inode *parent,
> if (IS_ERR(dir))
> doutc(cl, "can't find inode %s (%s)\n", inode_number, name);
> }
> -
> -out:
> - kfree(inode_number);
> return dir;
> }
>
