From 33265fe88ba59148609dc0706c84e4fb7fde43aa Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Wed, 15 Jul 2020 16:16:35 +1000 Subject: [PATCH] LU-6142 lustre: change obd_ioctl_getdata() args Instead of passing a pointer to 'char *' to obd_ioctl_getdata(), pass a pointer to 'struct obd_ioctl_data *'. This seems more natural, and avoid needing extra 'char *data' variables wherever the function is called. Test-Parameters: trivial Signed-off-by: Mr NeilBrown Change-Id: I0287cd2fafd071a2580d48a52498469ceded3e79 Reviewed-on: https://review.whamcloud.com/39381 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Shaun Tancheff Reviewed-by: James Simmons --- lustre/include/obd_class.h | 3 +- lustre/llite/dir.c | 26 +++++++--------- lustre/llite/llite_lib.c | 68 ++++++++++++++++++++--------------------- lustre/lov/lov_obd.c | 73 ++++++++++++++++++++++----------------------- lustre/obdclass/class_obd.c | 21 +++++++------ 5 files changed, 91 insertions(+), 100 deletions(-) diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index 45ff5ef..5c6e25e 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -1862,7 +1862,8 @@ struct root_squash_info { int server_name2index(const char *svname, __u32 *idx, const char **endptr); /* linux-module.c */ -int obd_ioctl_getdata(char **buf, int *len, void __user *arg); +struct obd_ioctl_data; +int obd_ioctl_getdata(struct obd_ioctl_data **data, int *len, void __user *arg); int class_procfs_init(void); int class_procfs_clean(void); diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c index 56d70d9..5dc4e95 100644 --- a/lustre/llite/dir.c +++ b/lustre/llite/dir.c @@ -1350,7 +1350,7 @@ static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long arg) struct dentry *dentry = file_dentry(file); struct inode *inode = file_inode(file); struct ll_sb_info *sbi = ll_i2sbi(inode); - struct obd_ioctl_data *data; + struct obd_ioctl_data *data = NULL; int rc = 0; ENTRY; @@ -1388,14 +1388,12 @@ static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return 0; } case IOC_MDC_LOOKUP: { - int namelen, len = 0; - char *buf = NULL; + int namelen, len = 0; char *filename; - rc = obd_ioctl_getdata(&buf, &len, (void __user *)arg); + rc = obd_ioctl_getdata(&data, &len, (void __user *)arg); if (rc != 0) RETURN(rc); - data = (void *)buf; filename = data->ioc_inlbuf1; namelen = strlen(filename); @@ -1411,12 +1409,11 @@ static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long arg) GOTO(out_free, rc); } out_free: - OBD_FREE_LARGE(buf, len); - return rc; - } + OBD_FREE_LARGE(data, len); + return rc; + } case LL_IOC_LMV_SETSTRIPE: { struct lmv_user_md *lum; - char *buf = NULL; char *filename; int namelen = 0; int lumlen = 0; @@ -1424,11 +1421,10 @@ out_free: int len; int rc; - rc = obd_ioctl_getdata(&buf, &len, (void __user *)arg); + rc = obd_ioctl_getdata(&data, &len, (void __user *)arg); if (rc) RETURN(rc); - data = (void *)buf; if (data->ioc_inlbuf1 == NULL || data->ioc_inlbuf2 == NULL || data->ioc_inllen1 == 0 || data->ioc_inllen2 == 0) GOTO(lmv_out_free, rc = -EINVAL); @@ -1467,7 +1463,7 @@ out_free: mode = data->ioc_type; rc = ll_dir_setdirstripe(dentry, lum, lumlen, filename, mode); lmv_out_free: - OBD_FREE_LARGE(buf, len); + OBD_FREE_LARGE(data, len); RETURN(rc); } @@ -2095,17 +2091,15 @@ out_hur: } case LL_IOC_MIGRATE: { struct lmv_user_md *lum; - char *buf = NULL; int len; char *filename; int namelen = 0; int rc; - rc = obd_ioctl_getdata(&buf, &len, (void __user *)arg); + rc = obd_ioctl_getdata(&data, &len, (void __user *)arg); if (rc) RETURN(rc); - data = (struct obd_ioctl_data *)buf; if (data->ioc_inlbuf1 == NULL || data->ioc_inlbuf2 == NULL || data->ioc_inllen1 == 0 || data->ioc_inllen2 == 0) GOTO(migrate_free, rc = -EINVAL); @@ -2129,7 +2123,7 @@ out_hur: rc = ll_migrate(inode, file, lum, filename); migrate_free: - OBD_FREE_LARGE(buf, len); + OBD_FREE_LARGE(data, len); RETURN(rc); } diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index 7c78279..335f0d8 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -2996,44 +2996,44 @@ out: int ll_obd_statfs(struct inode *inode, void __user *arg) { - struct ll_sb_info *sbi = NULL; - struct obd_export *exp; - char *buf = NULL; - struct obd_ioctl_data *data = NULL; - __u32 type; - int len = 0, rc; - - if (!inode || !(sbi = ll_i2sbi(inode))) - GOTO(out_statfs, rc = -EINVAL); - - rc = obd_ioctl_getdata(&buf, &len, arg); - if (rc) - GOTO(out_statfs, rc); - - data = (void*)buf; - if (!data->ioc_inlbuf1 || !data->ioc_inlbuf2 || - !data->ioc_pbuf1 || !data->ioc_pbuf2) - GOTO(out_statfs, rc = -EINVAL); - - if (data->ioc_inllen1 != sizeof(__u32) || - data->ioc_inllen2 != sizeof(__u32) || - data->ioc_plen1 != sizeof(struct obd_statfs) || - data->ioc_plen2 != sizeof(struct obd_uuid)) - GOTO(out_statfs, rc = -EINVAL); - - memcpy(&type, data->ioc_inlbuf1, sizeof(__u32)); + struct ll_sb_info *sbi = NULL; + struct obd_export *exp; + struct obd_ioctl_data *data = NULL; + __u32 type; + int len = 0, rc; + + if (inode) + sbi = ll_i2sbi(inode); + if (!sbi) + GOTO(out_statfs, rc = -EINVAL); + + rc = obd_ioctl_getdata(&data, &len, arg); + if (rc) + GOTO(out_statfs, rc); + + if (!data->ioc_inlbuf1 || !data->ioc_inlbuf2 || + !data->ioc_pbuf1 || !data->ioc_pbuf2) + GOTO(out_statfs, rc = -EINVAL); + + if (data->ioc_inllen1 != sizeof(__u32) || + data->ioc_inllen2 != sizeof(__u32) || + data->ioc_plen1 != sizeof(struct obd_statfs) || + data->ioc_plen2 != sizeof(struct obd_uuid)) + GOTO(out_statfs, rc = -EINVAL); + + memcpy(&type, data->ioc_inlbuf1, sizeof(__u32)); if (type & LL_STATFS_LMV) - exp = sbi->ll_md_exp; + exp = sbi->ll_md_exp; else if (type & LL_STATFS_LOV) - exp = sbi->ll_dt_exp; - else - GOTO(out_statfs, rc = -ENODEV); + exp = sbi->ll_dt_exp; + else + GOTO(out_statfs, rc = -ENODEV); - rc = obd_iocontrol(IOC_OBD_STATFS, exp, len, buf, NULL); - if (rc) - GOTO(out_statfs, rc); + rc = obd_iocontrol(IOC_OBD_STATFS, exp, len, data, NULL); + if (rc) + GOTO(out_statfs, rc); out_statfs: - OBD_FREE_LARGE(buf, len); + OBD_FREE_LARGE(data, len); return rc; } diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 7972d62..85a9fa0 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -1008,51 +1008,48 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len, RETURN(-EFAULT); break; } - case OBD_IOC_LOV_GET_CONFIG: { - struct obd_ioctl_data *data; - struct lov_desc *desc; - char *buf = NULL; - __u32 *genp; - - len = 0; - if (obd_ioctl_getdata(&buf, &len, uarg)) - RETURN(-EINVAL); + case OBD_IOC_LOV_GET_CONFIG: { + struct obd_ioctl_data *data = NULL; + struct lov_desc *desc; + __u32 *genp; - data = (struct obd_ioctl_data *)buf; + len = 0; + if (obd_ioctl_getdata(&data, &len, uarg)) + RETURN(-EINVAL); - if (sizeof(*desc) > data->ioc_inllen1) { - OBD_FREE_LARGE(buf, len); - RETURN(-EINVAL); - } + if (sizeof(*desc) > data->ioc_inllen1) { + OBD_FREE_LARGE(data, len); + RETURN(-EINVAL); + } - if (sizeof(uuidp->uuid) * count > data->ioc_inllen2) { - OBD_FREE_LARGE(buf, len); - RETURN(-EINVAL); - } + if (sizeof(uuidp->uuid) * count > data->ioc_inllen2) { + OBD_FREE_LARGE(data, len); + RETURN(-EINVAL); + } - if (sizeof(__u32) * count > data->ioc_inllen3) { - OBD_FREE_LARGE(buf, len); - RETURN(-EINVAL); - } + if (sizeof(__u32) * count > data->ioc_inllen3) { + OBD_FREE_LARGE(data, len); + RETURN(-EINVAL); + } - desc = (struct lov_desc *)data->ioc_inlbuf1; - memcpy(desc, &(lov->desc), sizeof(*desc)); + desc = (struct lov_desc *)data->ioc_inlbuf1; + memcpy(desc, &lov->desc, sizeof(*desc)); - uuidp = (struct obd_uuid *)data->ioc_inlbuf2; - genp = (__u32 *)data->ioc_inlbuf3; - /* the uuid will be empty for deleted OSTs */ - for (i = 0; i < count; i++, uuidp++, genp++) { - if (!lov->lov_tgts[i]) - continue; - *uuidp = lov->lov_tgts[i]->ltd_uuid; - *genp = lov->lov_tgts[i]->ltd_gen; - } + uuidp = (struct obd_uuid *)data->ioc_inlbuf2; + genp = (__u32 *)data->ioc_inlbuf3; + /* the uuid will be empty for deleted OSTs */ + for (i = 0; i < count; i++, uuidp++, genp++) { + if (!lov->lov_tgts[i]) + continue; + *uuidp = lov->lov_tgts[i]->ltd_uuid; + *genp = lov->lov_tgts[i]->ltd_gen; + } - if (copy_to_user(uarg, buf, len)) - rc = -EFAULT; - OBD_FREE_LARGE(buf, len); - break; - } + if (copy_to_user(uarg, data, len)) + rc = -EFAULT; + OBD_FREE_LARGE(data, len); + break; + } case OBD_IOC_QUOTACTL: { struct if_quotactl *qctl = karg; struct lov_tgt_desc *tgt = NULL; diff --git a/lustre/obdclass/class_obd.c b/lustre/obdclass/class_obd.c index cf1dbe2..436f04c 100644 --- a/lustre/obdclass/class_obd.c +++ b/lustre/obdclass/class_obd.c @@ -209,7 +209,7 @@ static int obd_ioctl_is_invalid(struct obd_ioctl_data *data) } /* buffer MUST be at least the size of obd_ioctl_hdr */ -int obd_ioctl_getdata(char **buf, int *len, void __user *arg) +int obd_ioctl_getdata(struct obd_ioctl_data **datap, int *len, void __user *arg) { struct obd_ioctl_hdr hdr; struct obd_ioctl_data *data; @@ -241,23 +241,22 @@ int obd_ioctl_getdata(char **buf, int *len, void __user *arg) * obdfilter-survey is an example, which relies on ioctl. So we'd * better avoid vmalloc on ioctl path. LU-66 */ - OBD_ALLOC_LARGE(*buf, hdr.ioc_len); - if (!*buf) { + OBD_ALLOC_LARGE(data, hdr.ioc_len); + if (!data) { CERROR("Cannot allocate control buffer of len %d\n", hdr.ioc_len); RETURN(-EINVAL); } *len = hdr.ioc_len; - data = (struct obd_ioctl_data *)*buf; - if (copy_from_user(*buf, arg, hdr.ioc_len)) { - OBD_FREE_LARGE(*buf, hdr.ioc_len); + if (copy_from_user(data, arg, hdr.ioc_len)) { + OBD_FREE_LARGE(data, hdr.ioc_len); RETURN(-EFAULT); } if (obd_ioctl_is_invalid(data)) { CERROR("ioctl not correctly formatted\n"); - OBD_FREE_LARGE(*buf, hdr.ioc_len); + OBD_FREE_LARGE(data, hdr.ioc_len); RETURN(-EINVAL); } @@ -279,24 +278,24 @@ int obd_ioctl_getdata(char **buf, int *len, void __user *arg) if (data->ioc_inllen4) data->ioc_inlbuf4 = &data->ioc_bulk[0] + offset; + *datap = data; + RETURN(0); } EXPORT_SYMBOL(obd_ioctl_getdata); int class_handle_ioctl(unsigned int cmd, unsigned long arg) { - char *buf = NULL; struct obd_ioctl_data *data; struct obd_device *obd = NULL; int err = 0, len = 0; ENTRY; CDEBUG(D_IOCTL, "cmd = %x\n", cmd); - if (obd_ioctl_getdata(&buf, &len, (void __user *)arg)) { + if (obd_ioctl_getdata(&data, &len, (void __user *)arg)) { CERROR("OBD ioctl: data error\n"); RETURN(-EINVAL); } - data = (struct obd_ioctl_data *)buf; switch (cmd) { case OBD_IOC_PROCESS_CFG: { @@ -471,7 +470,7 @@ int class_handle_ioctl(unsigned int cmd, unsigned long arg) if (copy_to_user((void __user *)arg, data, len)) err = -EFAULT; out: - OBD_FREE_LARGE(buf, len); + OBD_FREE_LARGE(data, len); RETURN(err); } /* class_handle_ioctl */ -- 1.8.3.1