On Sun, Nov 10, 2024 at 02:41:38PM +0800, [email protected] wrote:
> Use safe string copying macros (strscpy & strscpy_pad) to make string
> copying simpler & safer.
>
> Signed-off-by: [email protected] <[email protected]>
No!
You're introducing nuls where there previously weren't any, and since
there are strict checks on how many padding nuls are allowed this is
broken.
I already told you this didn't need to be done; please stop.
> ---
> fs/bcachefs/dirent.c | 10 +++-------
> fs/bcachefs/disk_groups.c | 5 ++---
> fs/bcachefs/fs-ioctl.c | 17 ++++++++---------
> fs/bcachefs/fs.c | 6 +-----
> 4 files changed, 14 insertions(+), 24 deletions(-)
>
> diff --git a/fs/bcachefs/dirent.c b/fs/bcachefs/dirent.c
> index 4c22f78b0484..466fe5765739 100644
> --- a/fs/bcachefs/dirent.c
> +++ b/fs/bcachefs/dirent.c
> @@ -191,13 +191,9 @@ static struct bkey_i_dirent *dirent_create_key(struct
> btree_trans *trans,
>
> dirent->v.d_type = type;
>
> - memcpy(dirent->v.d_name, name->name, name->len);
> - memset(dirent->v.d_name + name->len, 0,
> - bkey_val_bytes(&dirent->k) -
> - offsetof(struct bch_dirent, d_name) -
> - name->len);
> -
> - EBUG_ON(bch2_dirent_name_bytes(dirent_i_to_s_c(dirent)) != name->len);
> + EBUG_ON(strscpy_pad(dirent->v.d_name, name->name,
> + bkey_val_bytes(&dirent->k) - offsetof(struct
> bch_dirent, d_name))
> + != name->len);
>
> return dirent;
> }
> diff --git a/fs/bcachefs/disk_groups.c b/fs/bcachefs/disk_groups.c
> index 5df8de0b8c02..c2fc575e38d1 100644
> --- a/fs/bcachefs/disk_groups.c
> +++ b/fs/bcachefs/disk_groups.c
> @@ -319,9 +319,8 @@ static int __bch2_disk_group_add(struct bch_sb_handle
> *sb, unsigned parent,
>
> g = &groups->entries[i];
>
> - memcpy(g->label, name, namelen);
> - if (namelen < sizeof(g->label))
> - g->label[namelen] = '\0';
> + strscpy(g->label, name, namelen);
> +
> SET_BCH_GROUP_DELETED(g, 0);
> SET_BCH_GROUP_PARENT(g, parent);
> SET_BCH_GROUP_DATA_ALLOWED(g, ~0);
> diff --git a/fs/bcachefs/fs-ioctl.c b/fs/bcachefs/fs-ioctl.c
> index 15725b4ce393..4233707bec9d 100644
> --- a/fs/bcachefs/fs-ioctl.c
> +++ b/fs/bcachefs/fs-ioctl.c
> @@ -279,26 +279,25 @@ static int bch2_ioc_getversion(struct bch_inode_info
> *inode, u32 __user *arg)
>
> static int bch2_ioc_getlabel(struct bch_fs *c, char __user *user_label)
> {
> - int ret;
> - size_t len;
> char label[BCH_SB_LABEL_SIZE];
> + size_t len;
>
> BUILD_BUG_ON(BCH_SB_LABEL_SIZE >= FSLABEL_MAX);
>
> mutex_lock(&c->sb_lock);
> - memcpy(label, c->disk_sb.sb->label, BCH_SB_LABEL_SIZE);
> + int ret = strscpy(label, c->disk_sb.sb->label, BCH_SB_LABEL_SIZE);
> mutex_unlock(&c->sb_lock);
>
> - len = strnlen(label, BCH_SB_LABEL_SIZE);
> - if (len == BCH_SB_LABEL_SIZE) {
> + if (ret == -E2BIG) {
> + len = BCH_SB_LABEL_SIZE - 1;
> bch_warn(c,
> "label is too long, return the first %zu bytes",
> - --len);
> + len);
> + } else {
> + len = ret;
> }
>
> - ret = copy_to_user(user_label, label, len);
> -
> - return ret ? -EFAULT : 0;
> + return copy_to_user(user_label, label, len) ? -EFAULT : 0;
> }
>
> static int bch2_ioc_setlabel(struct bch_fs *c,
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 91fce04272a1..be86a36e8cfe 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -1641,7 +1641,6 @@ static int bch2_get_name(struct dentry *parent, char
> *name, struct dentry *child
> subvol_inum target;
> u32 snapshot;
> struct qstr dirent_name;
> - unsigned name_len = 0;
> int ret;
>
> if (!S_ISDIR(dir->v.i_mode))
> @@ -1717,10 +1716,7 @@ static int bch2_get_name(struct dentry *parent, char
> *name, struct dentry *child
> goto err;
> found:
> dirent_name = bch2_dirent_get_name(d);
> -
> - name_len = min_t(unsigned, dirent_name.len, NAME_MAX);
> - memcpy(name, dirent_name.name, name_len);
> - name[name_len] = '\0';
> + strscpy(name, dirent_name.name, NAME_MAX);
> err:
> if (bch2_err_matches(ret, BCH_ERR_transaction_restart))
> goto retry;
> --
> 2.47.0
>
