Re: [PATCH v3 08/17] btrfs: handle nokey names.

[Josef Bacik <josef@toxicpanda.com>]

On Tue, Aug 08, 2023 at 01:12:10PM -0400, Sweet Tea Dorminy wrote:
> For encrypted or unencrypted names, we calculate the offset for the dir
> item by hashing the name for the dir item. However, this doesn't work
> for a long nokey name, where we do not have the complete ciphertext.
> Instead, fscrypt stores the filesystem-provided hash in the nokey name,
> and we can extract it from the fscrypt_name structure in such a case.
> 
> Additionally, for nokey names, if we find the nokey name on disk we can
> update the fscrypt_name with the disk name, so add that to searching for
> diritems.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/btrfs/dir-item.c | 37 +++++++++++++++++++++++++++++++++++--
>  fs/btrfs/fscrypt.c  | 27 +++++++++++++++++++++++++++
>  fs/btrfs/fscrypt.h  | 11 +++++++++++
>  3 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
> index da95ae411d72..ee7dad888f53 100644
> --- a/fs/btrfs/dir-item.c
> +++ b/fs/btrfs/dir-item.c
> @@ -231,6 +231,28 @@ struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
>  	return di;
>  }
>  
> +/*
> + * If appropriate, populate the disk name for a fscrypt_name looked up without
> + * a key.
> + *
> + * @path:	The path to the extent buffer in which the name was found.
> + * @di:		The dir item corresponding.
> + * @fname:	The fscrypt_name to perhaps populate.
> + *
> + * Returns: 0 if the name is already populated or the dir item doesn't exist
> + * or the name was successfully populated, else an error code.
> + */
> +static int ensure_disk_name_from_dir_item(struct btrfs_path *path,
> +					  struct btrfs_dir_item *di,
> +					  struct fscrypt_name *name)
> +{
> +	if (name->disk_name.name || !di)
> +		return 0;
> +
> +	return btrfs_fscrypt_get_disk_name(path->nodes[0], di,
> +					   &name->disk_name);
> +}
> +
>  /*
>   * Lookup for a directory item by fscrypt_name.
>   *
> @@ -257,8 +279,12 @@ struct btrfs_dir_item *btrfs_lookup_dir_item_fname(struct btrfs_trans_handle *tr
>  
>  	key.objectid = dir;
>  	key.type = BTRFS_DIR_ITEM_KEY;
> -	key.offset = btrfs_name_hash(name->disk_name.name, name->disk_name.len);
> -	/* XXX get the right hash for no-key names */
> +
> +	if (!name->disk_name.name)
> +		key.offset = name->hash | ((u64)name->minor_hash << 32);
> +	else
> +		key.offset = btrfs_name_hash(name->disk_name.name,
> +					     name->disk_name.len);
>  
>  	ret = btrfs_search_slot(trans, root, &key, path, mod, -mod);
>  	if (ret == 0)
> @@ -266,6 +292,8 @@ struct btrfs_dir_item *btrfs_lookup_dir_item_fname(struct btrfs_trans_handle *tr
>  
>  	if (ret == -ENOENT || (di && IS_ERR(di) && PTR_ERR(di) == -ENOENT))
>  		return NULL;
> +	if (ret == 0)
> +		ret = ensure_disk_name_from_dir_item(path, di, name);
>  	if (ret < 0)
>  		di = ERR_PTR(ret);
>  
> @@ -382,7 +410,12 @@ btrfs_search_dir_index_item(struct btrfs_root *root, struct btrfs_path *path,
>  	btrfs_for_each_slot(root, &key, &key, path, ret) {
>  		if (key.objectid != dirid || key.type != BTRFS_DIR_INDEX_KEY)
>  			break;
> +
>  		di = btrfs_match_dir_item_fname(root->fs_info, path, name);
> +		if (di)
> +			ret = ensure_disk_name_from_dir_item(path, di, name);
> +		if (ret)
> +			break;

This is a little wonky, I'd rather just

if (!di)
	continue;
ret = ensure_disk_name_from_dir_item(path, di, name);
if (ret)
	break
return di;

Thanks,

Josef