Re: [PATCHv3] btrfs: Introduce new BTRFS_IOC_SNAP_DESTROY_V2 ioctl

[kbuild test robot]

CC: kbuil...@lists.01.org
In-Reply-To: <20200207130546.6771...@gmail.com>
References: <20200207130546.6771...@gmail.com>
TO: Marcos Paulo de Souza <marcos.s...@gmail.com>
CC: dst...@suse.com, w...@suse.com, linux...@vger.kernel.org, h...@infradead.org, jo...@toxicpanda.com, Marcos Paulo de Souza <mpde...@suse.com>, Marcos Paulo de Souza <mpde...@suse.com>
CC: Marcos Paulo de Souza <mpde...@suse.com>

Hi Marcos,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.6-rc1]
[also build test WARNING on next-20200210]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Marcos-Paulo-de-Souza/btrfs-Introduce-new-BTRFS_IOC_SNAP_DESTROY_V2-ioctl/20200210-210646
base: bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
config: arm64-defconfig (attached as .config)
compiler: clang version 11.0.0 (git://gitmirror/llvm_project 4dcc029edbe4bd5e30d4f0cdcf123ea4ed2b6418)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:2867:4: warning: array index 4087 is past the end of the array (which contains 4040 elements) [-Warray-bounds]
vol_args2->name[BTRFS_PATH_NAME_MAX] = '\0';
^ ~~~~~~~~~~~~~~~~~~~
include/uapi/linux/btrfs.h:125:3: note: array 'name' declared here
char name[BTRFS_SUBVOL_NAME_MAX + 1];
^
1 warning generated.

vim +2867 fs/btrfs/ioctl.c

2838
2839 static noinline int btrfs_ioctl_snap_destroy(struct file *file,
2840 void __user *arg,
2841 bool destroy_v2)
2842 {
2843 struct dentry *parent = file->f_path.dentry;
2844 struct btrfs_fs_info *fs_info = btrfs_sb(parent->d_sb);
2845 struct dentry *dentry;
2846 struct inode *dir = d_inode(parent);
2847 struct inode *inode;
2848 struct btrfs_root *root = BTRFS_I(dir)->root;
2849 struct btrfs_root *dest = NULL;
2850 struct btrfs_ioctl_vol_args *vol_args = NULL;
2851 struct btrfs_ioctl_vol_args_v2 *vol_args2 = NULL;
2852 char *subvol_name, *subvol_name_ptr = NULL;
2853 int subvol_namelen;
2854 int err = 0;
2855 bool destroy_parent = false;
2856
2857 if (destroy_v2) {
2858 vol_args2 = memdup_user(arg, sizeof(*vol_args2));
2859 if (IS_ERR(vol_args2))
2860 return PTR_ERR(vol_args2);
2861
2862 /*
2863 * If SPEC_BY_ID is not set, we are looking for the subvolume by
2864 * name, same as v1 currently does.
2865 */
2866 if (!(vol_args2->flags & BTRFS_SUBVOL_SPEC_BY_ID)) {
> 2867 vol_args2->name[BTRFS_PATH_NAME_MAX] = '\0';
2868 subvol_name = vol_args2->name;
2869
2870 err = mnt_want_write_file(file);
2871 if (err)
2872 goto out;
2873 } else {
2874 if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID) {
2875 err = -EINVAL;
2876 goto out;
2877 }
2878
2879 err = mnt_want_write_file(file);
2880 if (err)
2881 goto out;
2882
2883 dentry = btrfs_get_dentry(fs_info->sb,
2884 BTRFS_FIRST_FREE_OBJECTID,
2885 vol_args2->subvolid, 0, 0);
2886 if (IS_ERR(dentry)) {
2887 err = PTR_ERR(dentry);
2888 goto out_drop_write;
2889 }
2890
2891 /*
2892 * Change the default parent since the subvolume being
2893 * deleted can be outside of the current mount point.
2894 */
2895 parent = btrfs_get_parent(dentry);
2896
2897 /*
2898 * At this point dentry->d_name can point to '/' if the
2899 * subvolume we want to destroy is outsite of the
2900 * current mount point, so we need to released the
2901 * current dentry and execute the lookup to return a new
2902 * one with ->d_name pointing to the
2903 * <mount point>/subvol_name.
2904 */
2905 dput(dentry);
2906 if (IS_ERR(parent)) {
2907 err = PTR_ERR(parent);
2908 goto out_drop_write;
2909 }
2910 dir = d_inode(parent);
2911
2912 /* If v2 was used with SPEC_BY_ID, a new parent was
2913 * allocated since the subvolume can be outside of the
2914 * current moutn point. Later on we need to release this
2915 * new parent dentry.
2916 */
2917 destroy_parent = true;
2918
2919 subvol_name_ptr = btrfs_get_subvol_name_from_objectid(fs_info,
2920 vol_args2->subvolid);
2921 if (IS_ERR(subvol_name_ptr)) {
2922 err = PTR_ERR(subvol_name_ptr);
2923 goto free_parent;
2924 }
2925 /* subvol_name_ptr is already NULL termined */
2926 subvol_name = (char *)kbasename(subvol_name_ptr);
2927 }
2928 } else {
2929 vol_args = memdup_user(arg, sizeof(*vol_args));
2930 if (IS_ERR(vol_args))
2931 return PTR_ERR(vol_args);
2932
2933 vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
2934 subvol_name = vol_args->name;
2935
2936 err = mnt_want_write_file(file);
2937 if (err)
2938 goto out;
2939 }
2940
2941 subvol_namelen = strlen(subvol_name);
2942
2943 if (strchr(subvol_name, '/') ||
2944 strncmp(subvol_name, "..", subvol_namelen) == 0) {
2945 err = -EINVAL;
2946 goto free_subvol_name;
2947 }
2948
2949 if (!S_ISDIR(dir->i_mode)) {
2950 err = -ENOTDIR;
2951 goto free_subvol_name;
2952 }
2953
2954 err = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
2955 if (err == -EINTR)
2956 goto out_drop_write;
2957 dentry = lookup_one_len(subvol_name, parent, subvol_namelen);
2958 if (IS_ERR(dentry)) {
2959 err = PTR_ERR(dentry);
2960 goto out_unlock_dir;
2961 }
2962
2963 if (d_really_is_negative(dentry)) {
2964 err = -ENOENT;
2965 goto out_dput;
2966 }
2967
2968 inode = d_inode(dentry);
2969 dest = BTRFS_I(inode)->root;
2970 if (!capable(CAP_SYS_ADMIN)) {
2971 /*
2972 * Regular user. Only allow this with a special mount
2973 * option, when the user has write+exec access to the
2974 * subvol root, and when rmdir(2) would have been
2975 * allowed.
2976 *
2977 * Note that this is _not_ check that the subvol is
2978 * empty or doesn't contain data that we wouldn't
2979 * otherwise be able to delete.
2980 *
2981 * Users who want to delete empty subvols should try
2982 * rmdir(2).
2983 */
2984 err = -EPERM;
2985 if (!btrfs_test_opt(fs_info, USER_SUBVOL_RM_ALLOWED))
2986 goto out_dput;
2987
2988 /*
2989 * Do not allow deletion if the parent dir is the same
2990 * as the dir to be deleted. That means the ioctl
2991 * must be called on the dentry referencing the root
2992 * of the subvol, not a random directory contained
2993 * within it.
2994 */
2995 err = -EINVAL;
2996 if (root == dest)
2997 goto out_dput;
2998
2999 err = inode_permission(inode, MAY_WRITE | MAY_EXEC);
3000 if (err)
3001 goto out_dput;
3002 }
3003
3004 /* check if subvolume may be deleted by a user */
3005 err = btrfs_may_delete(dir, dentry, 1);
3006 if (err)
3007 goto out_dput;
3008
3009 if (btrfs_ino(BTRFS_I(inode)) != BTRFS_FIRST_FREE_OBJECTID) {
3010 err = -EINVAL;
3011 goto out_dput;
3012 }
3013
3014 inode_lock(inode);
3015 err = btrfs_delete_subvolume(dir, dentry);
3016 inode_unlock(inode);
3017 if (!err) {
3018 fsnotify_rmdir(dir, dentry);
3019 d_delete(dentry);
3020 }
3021
3022 out_dput:
3023 dput(dentry);
3024 out_unlock_dir:
3025 inode_unlock(dir);
3026 free_subvol_name:
3027 kfree(subvol_name_ptr);
3028 free_parent:
3029 if (destroy_parent)
3030 dput(parent);
3031 out_drop_write:
3032 mnt_drop_write_file(file);
3033 out:
3034 kfree(vol_args2);
3035 kfree(vol_args);
3036 return err;
3037 }
3038

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuil...@lists.01.org

[Nathan Chancellor]

On Fri, Feb 07, 2020 at 10:05:46AM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpde...@suse.com>
>
> This ioctl will be responsible for deleting a subvolume using it's id.
> This can be used when a system has a file system mounted from a
> subvolume, rather than the root file system, like below:
>
> /
> |- @subvol1
> |- @subvol2
> \- @subvol_default
>
> If only @subvol_default is mounted, we have no path to reach
> @subvol1 and @subvol2, thus no way to delete them. Current subvolume
> delete ioctl takes a file handler point as argument, and if
> @subvol_default is mounted, we can't reach @subvol1 and @subvol2 from
> the same mount point.
>
> This patch introduces a new flag to allow BTRFS_IOC_SNAP_DESTROY_V2
> to delete subvolume using subvolid.
>
> Now, we can use this new ioctl specifying the subvolume id and refer to
> the same mount point. It doesn't matter which subvolume was mounted,
> since we can reach to the desired one using the subvolume id, and then
> delete it.
>
> Also in this patch:
> * export get_subvol_name_from_objectid, adding btrfs suffix
> * add BTRFS_SUBVOL_SPEC_BY_ID flag
> * add subvolid as a union member in struct btrfs_ioctl_vol_args_v2.
>
> Signed-off-by: Marcos Paulo de Souza <mpde...@suse.com>
> ---
>
> Changes from v2:
> * Commit message improved, explaining how to use the new ioctl (David)
> * Moved subvolid member to the union which already contained devid and name
> (David)
> * Changed name_ptr to subvol_name_ptr, since it'll point to the "full"
> subvolume name, but we need the basename of this char, which was also renamed
> to subvol_name (David).
> * Change the check for a valid subvolid to be >= BTRFS_FIRST_FREE_OBJECTID
> (David)
> * Now BTRFS_IOC_SNAP_DESTROY_V2 can handle both cases where the user uses the
> subvolid and just the subvolume name (David)
> * Changed BTRFS_SUBVOL_DELETE_BY_ID to BTRFS_SUBVOL_SPEC_BY_ID, since this flag
> can be used for other actions rather than deleting a subvolume (David, Christoph)
> * Rewritten comment about the getting/releasing the dentry before doing the
> lookup, explaining why this dentry can be released in order to get a new one
> from lookup (David)
> * Moved mnt_want_write_file call sites right after the flag validation (David)
>
> Changes from v1:
> * make btrfs_ioctl_snap_destroy handle both SNAP_DESTROY and SNAP_DESTROY_V2
> (suggested by Josef)
> * Change BTRFS_SUBVOL_DELETE_BY_ID to BTRFS_SUBVOL_BY_ID (David)
> * Send patches for btrfs-progs and xfstests along this change
>
> fs/btrfs/ctree.h | 2 +
> fs/btrfs/export.c | 4 +-
> fs/btrfs/export.h | 5 ++
> fs/btrfs/ioctl.c | 128 +++++++++++++++++++++++++++++++------
> fs/btrfs/super.c | 4 +-
> include/uapi/linux/btrfs.h | 8 ++-
> 6 files changed, 127 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 54efb21c2727..2d56517c4bca 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2963,6 +2963,8 @@ int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
> int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> unsigned long new_flags);
> int btrfs_sync_fs(struct super_block *sb, int wait);
> +char *btrfs_get_subvol_name_from_objectid(struct btrfs_fs_info *fs_info,
> + u64 subvol_objectid);
>
> static inline __printf(2, 3) __cold
> void btrfs_no_printk(const struct btrfs_fs_info *fs_info, const char *fmt, ...)
> diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
> index 72e312cae69d..027411cdbae7 100644
> --- a/fs/btrfs/export.c
> +++ b/fs/btrfs/export.c
> @@ -57,7 +57,7 @@ static int btrfs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> return type;
> }
>
> -static struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
> +struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
> u64 root_objectid, u32 generation,
> int check_generation)
> {
> @@ -152,7 +152,7 @@ static struct dentry *btrfs_fh_to_dentry(struct super_block *sb, struct fid *fh,
> return btrfs_get_dentry(sb, objectid, root_objectid, generation, 1);
> }
>
> -static struct dentry *btrfs_get_parent(struct dentry *child)
> +struct dentry *btrfs_get_parent(struct dentry *child)
> {
> struct inode *dir = d_inode(child);
> struct btrfs_fs_info *fs_info = btrfs_sb(dir->i_sb);
> diff --git a/fs/btrfs/export.h b/fs/btrfs/export.h
> index 57488ecd7d4e..f981e8103d64 100644
> --- a/fs/btrfs/export.h
> +++ b/fs/btrfs/export.h
> @@ -18,4 +18,9 @@ struct btrfs_fid {
> u64 parent_root_objectid;
> } __attribute__ ((packed));
>
> +struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
> + u64 root_objectid, u32 generation,
> + int check_generation);
> +struct dentry *btrfs_get_parent(struct dentry *child);
> +
> #endif
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 12ae31e1813e..be5350582955 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -28,6 +28,7 @@
> #include <linux/iversion.h>
> #include "ctree.h"
> #include "disk-io.h"
> +#include "export.h"
> #include "transaction.h"
> #include "btrfs_inode.h"
> #include "print-tree.h"
> @@ -2836,7 +2837,8 @@ static int btrfs_ioctl_get_subvol_rootref(struct file *file, void __user *argp)

> }
>
> static noinline int btrfs_ioctl_snap_destroy(struct file *file,

> - void __user *arg)
> + void __user *arg,
> + bool destroy_v2)
> {

> struct dentry *parent = file->f_path.dentry;

> struct btrfs_fs_info *fs_info = btrfs_sb(parent->d_sb);

> @@ -2845,34 +2847,114 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> struct inode *inode;

> struct btrfs_root *root = BTRFS_I(dir)->root;

> struct btrfs_root *dest = NULL;

> - struct btrfs_ioctl_vol_args *vol_args;
> - int namelen;
> + struct btrfs_ioctl_vol_args *vol_args = NULL;
> + struct btrfs_ioctl_vol_args_v2 *vol_args2 = NULL;
> + char *subvol_name, *subvol_name_ptr = NULL;
> + int subvol_namelen;
> int err = 0;
> + bool destroy_parent = false;
>
> - if (!S_ISDIR(dir->i_mode))
> - return -ENOTDIR;
> + if (destroy_v2) {
> + vol_args2 = memdup_user(arg, sizeof(*vol_args2));
> + if (IS_ERR(vol_args2))
> + return PTR_ERR(vol_args2);
>
> - vol_args = memdup_user(arg, sizeof(*vol_args));
> - if (IS_ERR(vol_args))
> - return PTR_ERR(vol_args);
> + /*
> + * If SPEC_BY_ID is not set, we are looking for the subvolume by
> + * name, same as v1 currently does.
> + */
> + if (!(vol_args2->flags & BTRFS_SUBVOL_SPEC_BY_ID)) {
> + vol_args2->name[BTRFS_PATH_NAME_MAX] = '\0';
> + subvol_name = vol_args2->name;
>
> - vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
> - namelen = strlen(vol_args->name);
> - if (strchr(vol_args->name, '/') ||
> - strncmp(vol_args->name, "..", namelen) == 0) {
> - err = -EINVAL;
> - goto out;
> + err = mnt_want_write_file(file);
> + if (err)
> + goto out;
> + } else {
> + if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + err = mnt_want_write_file(file);
> + if (err)
> + goto out;
> +
> + dentry = btrfs_get_dentry(fs_info->sb,
> + BTRFS_FIRST_FREE_OBJECTID,
> + vol_args2->subvolid, 0, 0);
> + if (IS_ERR(dentry)) {
> + err = PTR_ERR(dentry);
> + goto out_drop_write;
> + }
> +
> + /*
> + * Change the default parent since the subvolume being
> + * deleted can be outside of the current mount point.
> + */
> + parent = btrfs_get_parent(dentry);
> +
> + /*
> + * At this point dentry->d_name can point to '/' if the
> + * subvolume we want to destroy is outsite of the
> + * current mount point, so we need to released the
> + * current dentry and execute the lookup to return a new
> + * one with ->d_name pointing to the
> + * <mount point>/subvol_name.
> + */
> + dput(dentry);
> + if (IS_ERR(parent)) {
> + err = PTR_ERR(parent);
> + goto out_drop_write;
> + }
> + dir = d_inode(parent);
> +
> + /* If v2 was used with SPEC_BY_ID, a new parent was
> + * allocated since the subvolume can be outside of the
> + * current moutn point. Later on we need to release this
> + * new parent dentry.
> + */
> + destroy_parent = true;
> +
> + subvol_name_ptr = btrfs_get_subvol_name_from_objectid(fs_info,
> + vol_args2->subvolid);
> + if (IS_ERR(subvol_name_ptr)) {
> + err = PTR_ERR(subvol_name_ptr);
> + goto free_parent;
> + }
> + /* subvol_name_ptr is already NULL termined */
> + subvol_name = (char *)kbasename(subvol_name_ptr);
> + }
> + } else {
> + vol_args = memdup_user(arg, sizeof(*vol_args));
> + if (IS_ERR(vol_args))
> + return PTR_ERR(vol_args);
> +
> + vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
> + subvol_name = vol_args->name;
> +
> + err = mnt_want_write_file(file);
> + if (err)
> + goto out;
> }
>
> - err = mnt_want_write_file(file);
> - if (err)
> - goto out;
> + subvol_namelen = strlen(subvol_name);
>
> + if (strchr(subvol_name, '/') ||
> + strncmp(subvol_name, "..", subvol_namelen) == 0) {
> + err = -EINVAL;
> + goto free_subvol_name;
> + }
> +
> + if (!S_ISDIR(dir->i_mode)) {
> + err = -ENOTDIR;
> + goto free_subvol_name;
> + }

>
> err = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);

> if (err == -EINTR)
> goto out_drop_write;
> - dentry = lookup_one_len(vol_args->name, parent, namelen);
> + dentry = lookup_one_len(subvol_name, parent, subvol_namelen);
> if (IS_ERR(dentry)) {
> err = PTR_ERR(dentry);
> goto out_unlock_dir;
> @@ -2941,9 +3023,15 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> dput(dentry);
> out_unlock_dir:
> inode_unlock(dir);
> +free_subvol_name:
> + kfree(subvol_name_ptr);
> +free_parent:
> + if (destroy_parent)
> + dput(parent);
> out_drop_write:
> mnt_drop_write_file(file);
> out:
> + kfree(vol_args2);
> kfree(vol_args);
> return err;
> }
> @@ -5464,7 +5552,9 @@ long btrfs_ioctl(struct file *file, unsigned int
> case BTRFS_IOC_SUBVOL_CREATE_V2:
> return btrfs_ioctl_snap_create_v2(file, argp, 1);
> case BTRFS_IOC_SNAP_DESTROY:
> - return btrfs_ioctl_snap_destroy(file, argp);
> + return btrfs_ioctl_snap_destroy(file, argp, false);
> + case BTRFS_IOC_SNAP_DESTROY_V2:
> + return btrfs_ioctl_snap_destroy(file, argp, true);
> case BTRFS_IOC_SUBVOL_GETFLAGS:
> return btrfs_ioctl_subvol_getflags(file, argp);
> case BTRFS_IOC_SUBVOL_SETFLAGS:
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index f452a94abdc3..649531e92a1d 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1005,7 +1005,7 @@ static int btrfs_parse_subvol_options(const char *options, char **subvol_name,
> return error;
> }
>
> -static char *get_subvol_name_from_objectid(struct btrfs_fs_info *fs_info,
> +char *btrfs_get_subvol_name_from_objectid(struct btrfs_fs_info *fs_info,
> u64 subvol_objectid)
> {
> struct btrfs_root *root = fs_info->tree_root;
> @@ -1417,7 +1417,7 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid,
> goto out;
> }
> }
> - subvol_name = get_subvol_name_from_objectid(btrfs_sb(mnt->mnt_sb),
> + subvol_name = btrfs_get_subvol_name_from_objectid(btrfs_sb(mnt->mnt_sb),
> subvol_objectid);
> if (IS_ERR(subvol_name)) {
> root = ERR_CAST(subvol_name);
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 7a8bc8b920f5..280f6ded2104 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -42,11 +42,14 @@ struct btrfs_ioctl_vol_args {
>
> #define BTRFS_DEVICE_SPEC_BY_ID (1ULL << 3)
>
> +#define BTRFS_SUBVOL_SPEC_BY_ID (1ULL << 4)
> +
> #define BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED \
> (BTRFS_SUBVOL_CREATE_ASYNC | \
> BTRFS_SUBVOL_RDONLY | \
> BTRFS_SUBVOL_QGROUP_INHERIT | \
> - BTRFS_DEVICE_SPEC_BY_ID)
> + BTRFS_DEVICE_SPEC_BY_ID | \
> + BTRFS_SUBVOL_SPEC_BY_ID)
>
> #define BTRFS_FSID_SIZE 16
> #define BTRFS_UUID_SIZE 16
> @@ -120,6 +123,7 @@ struct btrfs_ioctl_vol_args_v2 {
> };
> union {

> char name[BTRFS_SUBVOL_NAME_MAX + 1];

> + __u64 subvolid;
> __u64 devid;
> };
> };
> @@ -949,5 +953,7 @@ enum btrfs_err_code {
> struct btrfs_ioctl_get_subvol_rootref_args)
> #define BTRFS_IOC_INO_LOOKUP_USER _IOWR(BTRFS_IOCTL_MAGIC, 62, \
> struct btrfs_ioctl_ino_lookup_user_args)
> +#define BTRFS_IOC_SNAP_DESTROY_V2 _IOW(BTRFS_IOCTL_MAGIC, 63, \
> + struct btrfs_ioctl_vol_args_v2)
>
> #endif /* _UAPI_LINUX_BTRFS_H */
> --
> 2.24.0
>

Hi Marcos,

We received a build report from the 0day bot when building with clang
that appears legitimate if I am reading everything correctly.

../fs/btrfs/ioctl.c:2867:4: warning: array index 4087 is past the end of the array (which contains 4040 elements) [-Warray-bounds]

vol_args2->name[BTRFS_PATH_NAME_MAX] = '\0';
^ ~~~~~~~~~~~~~~~~~~~

../include/uapi/linux/btrfs.h:125:3: note: array 'name' declared here

char name[BTRFS_SUBVOL_NAME_MAX + 1];
^
1 warning generated.

The full report can be viewed here:

https://groups.google.com/d/msg/clang-built-linux/YFcXVkPdkTY/EhB6grZ2BQAJ

Mind taking a look at it?

Cheers,
Nathan

[Nathan Chancellor]

Reported to the mailing list:

https://lore.kernel.org/linux-btrfs/20200210234158.GA37636@ubuntu-x2-xlarge-x86/

Cheers,
Nathan

[Marcos Paulo de Souza]

Sure, I will send a new patch to address this warning after this one
gets merged, since this problem existed before this change. Thanks for
the report!

[David Sterba]

On Tue, Feb 11, 2020 at 03:57:21PM -0300, Marcos Paulo de Souza wrote:
> > We received a build report from the 0day bot when building with clang
> > that appears legitimate if I am reading everything correctly.
> >
> > ../fs/btrfs/ioctl.c:2867:4: warning: array index 4087 is past the end
> > of the array (which contains 4040 elements) [-Warray-bounds]
> > vol_args2->name[BTRFS_PATH_NAME_MAX] = '\0';
> > ^ ~~~~~~~~~~~~~~~~~~~
> > ../include/uapi/linux/btrfs.h:125:3: note: array 'name' declared here
> > char name[BTRFS_SUBVOL_NAME_MAX + 1];
> > ^
> > 1 warning generated.
>
> Sure, I will send a new patch to address this warning after this one
> gets merged, since this problem existed before this change. Thanks for
> the report!

Actually the warning is correct because you used a different macro:
BTRFS_PATH_NAME_MAX (4087) instead of BTRFS_SUBVOL_NAME_MAX (4039).