From 5eae8514f5f1730fe93d55d348400f5ecf681078 Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Sun, 19 Mar 2023 20:22:10 -0600 Subject: [PATCH] LU-16634 misc: standardize iocontrol param handling Validate uarg and karg early in iocontrol processing where needed. This needs kernel interop for 4.20+ kernels for access_ok(), but this can be checked by #ifdef and does not need an autoconf test. Fix incorrect definition of OBD_IOC_BARRIER to match reality. Signed-off-by: Andreas Dilger Reported-by: Tao Lyu Change-Id: I1a0d2f839949debf346aa15c65b0f407e9ce7057 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/50314 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Arshad Hussain Reviewed-by: Vitaliy Kuznetsov Reviewed-by: Oleg Drokin --- libcfs/libcfs/crypto/policy.c | 6 +- lustre/include/lustre_compat.h | 6 ++ lustre/include/uapi/linux/lustre/lustre_ioctl.h | 3 +- lustre/ldlm/ldlm_lib.c | 2 +- lustre/llite/file.c | 73 ++++++++++++++----------- lustre/llite/llite_lib.c | 19 +++++-- lustre/lmv/lmv_obd.c | 31 ++++++++--- lustre/lod/lod_dev.c | 5 +- lustre/lov/lov_obd.c | 26 ++++++++- lustre/mdc/mdc_request.c | 21 ++++--- lustre/mdd/mdd_device.c | 5 +- lustre/mdt/mdt_handler.c | 42 +++++++------- lustre/mdt/mdt_internal.h | 2 +- lustre/mgs/mgs_handler.c | 23 ++++---- lustre/obdclass/class_obd.c | 21 ++++--- lustre/obdecho/echo_client.c | 5 +- lustre/ofd/ofd_obd.c | 34 +++++++----- lustre/osc/osc_request.c | 25 ++++++++- lustre/osp/osp_dev.c | 8 ++- lustre/tests/group_lock_test.c | 16 +++--- 20 files changed, 242 insertions(+), 131 deletions(-) diff --git a/libcfs/libcfs/crypto/policy.c b/libcfs/libcfs/crypto/policy.c index 5d094d5..38477bd 100644 --- a/libcfs/libcfs/crypto/policy.c +++ b/libcfs/libcfs/crypto/policy.c @@ -438,14 +438,14 @@ int llcrypt_ioctl_get_policy_ex(struct file *filp, void __user *uarg) offsetof(typeof(arg), policy)); BUILD_BUG_ON(sizeof(arg.policy) != sizeof(*policy)); + if (copy_from_user(&arg, uarg, sizeof(arg.policy_size))) + return -EFAULT; + err = llcrypt_get_policy(file_inode(filp), policy); if (err) return err; policy_size = llcrypt_policy_size(policy); - if (copy_from_user(&arg, uarg, sizeof(arg.policy_size))) - return -EFAULT; - if (policy_size > arg.policy_size) return -EOVERFLOW; arg.policy_size = policy_size; diff --git a/lustre/include/lustre_compat.h b/lustre/include/lustre_compat.h index ade2beb..90d8872 100644 --- a/lustre/include/lustre_compat.h +++ b/lustre/include/lustre_compat.h @@ -586,6 +586,12 @@ static inline bool is_root_inode(struct inode *inode) # endif #endif +#ifdef VERIFY_WRITE /* removed in kernel commit v4.20-10979-g96d4f267e40f */ +#define ll_access_ok(ptr, len) access_ok(VERIFY_WRITE, ptr, len) +#else +#define ll_access_ok(ptr, len) access_ok(ptr, len) +#endif + static inline void ll_security_release_secctx(char *secdata, u32 seclen) { #ifdef HAVE_SEC_RELEASE_SECCTX_1ARG diff --git a/lustre/include/uapi/linux/lustre/lustre_ioctl.h b/lustre/include/uapi/linux/lustre/lustre_ioctl.h index bfd91cf..2e9a9bb 100644 --- a/lustre/include/uapi/linux/lustre/lustre_ioctl.h +++ b/lustre/include/uapi/linux/lustre/lustre_ioctl.h @@ -224,7 +224,8 @@ enum obd_abort_recovery_flags { /* lustre/lustre_user.h 240-249 */ /* was LIBCFS_IOC_DEBUG_MASK _IOWR('f', 250, long) until 2.11 */ -#define OBD_IOC_BARRIER _IOWR('f', 261, OBD_IOC_DATA_TYPE) +/* OBD_IOC_BARRIER wrongly defined as _IOWR('f', 261, OBD_IOC_DATA_TYPE) */ +#define OBD_IOC_BARRIER _IOWR('g', 5, OBD_IOC_DATA_TYPE) #define IOC_OSC_SET_ACTIVE _IOWR('h', 21, void *) diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c index 71fd089..a4ddded 100644 --- a/lustre/ldlm/ldlm_lib.c +++ b/lustre/ldlm/ldlm_lib.c @@ -2835,7 +2835,7 @@ static int target_recovery_thread(void *arg) /* cancel update llogs upon recovery abort */ if (obd->obd_abort_recovery || obd->obd_abort_mdt_recovery) OBP(obd, iocontrol)(OBD_IOC_LLOG_CANCEL, obd->obd_self_export, - 0, NULL, NULL); + 0, trd, NULL); list_for_each_entry(req, &obd->obd_final_req_queue, rq_list) { /* diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 5d70521..1c80d3d 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -2639,11 +2639,15 @@ out_lump: static int ll_file_getstripe(struct inode *inode, void __user *lum, size_t size) { - struct lu_env *env; - __u16 refcheck; - int rc; + struct lu_env *env; + __u16 refcheck; + int rc; ENTRY; + /* exit before doing any work if pointer is bad */ + if (unlikely(!ll_access_ok(lum, sizeof(struct lov_user_md)))) + RETURN(-EFAULT); + env = cl_env_get(&refcheck); if (IS_ERR(env)) RETURN(PTR_ERR(env)); @@ -3948,7 +3952,7 @@ static long ll_file_unlock_lease(struct file *file, struct ll_ioc_lease *ioc, bool lease_broken = false; fmode_t fmode = 0; enum mds_op_bias bias = 0; - int fdv; + __u32 fdv; struct file *layout_file = NULL; void *data = NULL; size_t data_size = 0; @@ -3984,12 +3988,12 @@ static long ll_file_unlock_lease(struct file *file, struct ll_ioc_lease *ioc, bias = MDS_CLOSE_RESYNC_DONE; break; - case LL_LEASE_LAYOUT_MERGE: { + case LL_LEASE_LAYOUT_MERGE: if (ioc->lil_count != 1) GOTO(out_lease_close, rc = -EINVAL); uarg += sizeof(*ioc); - if (copy_from_user(&fdv, uarg, sizeof(__u32))) + if (copy_from_user(&fdv, uarg, sizeof(fdv))) GOTO(out_lease_close, rc = -EFAULT); layout_file = fget(fdv); @@ -4003,20 +4007,21 @@ static long ll_file_unlock_lease(struct file *file, struct ll_ioc_lease *ioc, data = file_inode(layout_file); bias = MDS_CLOSE_LAYOUT_MERGE; break; - } case LL_LEASE_LAYOUT_SPLIT: { - int mirror_id; + __u32 mirror_id; if (ioc->lil_count != 2) GOTO(out_lease_close, rc = -EINVAL); uarg += sizeof(*ioc); - if (copy_from_user(&fdv, uarg, sizeof(__u32))) + if (copy_from_user(&fdv, uarg, sizeof(fdv))) GOTO(out_lease_close, rc = -EFAULT); - uarg += sizeof(__u32); - if (copy_from_user(&mirror_id, uarg, sizeof(__u32))) + uarg += sizeof(fdv); + if (copy_from_user(&mirror_id, uarg, sizeof(mirror_id))) GOTO(out_lease_close, rc = -EFAULT); + if (mirror_id >= MIRROR_ID_NEG) + GOTO(out_lease_close, rc = -EINVAL); layout_file = fget(fdv); if (!layout_file) @@ -4214,6 +4219,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (_IOC_TYPE(cmd) == 'T' || _IOC_TYPE(cmd) == 't') /* tty ioctls */ RETURN(-ENOTTY); + /* can't do a generic karg == NULL check here, since it is too noisy and + * we need to return -ENOTTY for unsupported ioctls instead of -EINVAL. + */ switch (cmd) { case LL_IOC_GETFLAGS: /* Get the current value of the file flags */ @@ -4315,6 +4323,9 @@ out: struct hsm_user_state *hus; int rc; + if (!ll_access_ok(uarg, sizeof(*hus))) + RETURN(-EFAULT); + OBD_ALLOC_PTR(hus); if (hus == NULL) RETURN(-ENOMEM); @@ -4322,17 +4333,16 @@ out: op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0, LUSTRE_OPC_ANY, hus); if (IS_ERR(op_data)) { - OBD_FREE_PTR(hus); - RETURN(PTR_ERR(op_data)); - } - - rc = obd_iocontrol(cmd, ll_i2mdexp(inode), sizeof(*op_data), - op_data, NULL); + rc = PTR_ERR(op_data); + } else { + rc = obd_iocontrol(cmd, ll_i2mdexp(inode), + sizeof(*op_data), op_data, NULL); - if (copy_to_user(uarg, hus, sizeof(*hus))) - rc = -EFAULT; + if (copy_to_user(uarg, hus, sizeof(*hus))) + rc = -EFAULT; - ll_finish_md_op_data(op_data); + ll_finish_md_op_data(op_data); + } OBD_FREE_PTR(hus); RETURN(rc); } @@ -4344,12 +4354,10 @@ out: if (hss == NULL) RETURN(-ENOMEM); - if (copy_from_user(hss, uarg, sizeof(*hss))) { - OBD_FREE_PTR(hss); - RETURN(-EFAULT); - } - - rc = ll_hsm_state_set(inode, hss); + if (copy_from_user(hss, uarg, sizeof(*hss))) + rc = -EFAULT; + else + rc = ll_hsm_state_set(inode, hss); OBD_FREE_PTR(hss); RETURN(rc); @@ -4360,6 +4368,9 @@ out: const char *action; int rc; + if (!ll_access_ok(uarg, sizeof(*hca))) + RETURN(-EFAULT); + OBD_ALLOC_PTR(hca); if (hca == NULL) RETURN(-ENOMEM); @@ -4440,12 +4451,10 @@ skip_copy: if (hui == NULL) RETURN(-ENOMEM); - if (copy_from_user(hui, uarg, sizeof(*hui))) { - OBD_FREE_PTR(hui); - RETURN(-EFAULT); - } - - rc = ll_hsm_import(inode, file, hui); + if (copy_from_user(hui, uarg, sizeof(*hui))) + rc = -EFAULT; + else + rc = ll_hsm_import(inode, file, hui); OBD_FREE_PTR(hui); RETURN(rc); diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index 9b0c2ff..b3dd42e 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -36,19 +36,20 @@ #define DEBUG_SUBSYSTEM S_LLITE #include +#include +#include +#include +#include #include #include #include #include -#include #include +#include +#include +#include #include #include -#include -#include -#include -#include -#include #ifndef HAVE_CPUS_READ_LOCK #include @@ -3008,6 +3009,9 @@ int ll_iocontrol(struct inode *inode, struct file *file, struct mdt_body *body; struct md_op_data *op_data; + if (!ll_access_ok(uarg, sizeof(int))) + RETURN(-EFAULT); + op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0, LUSTRE_OPC_ANY, NULL); if (IS_ERR(op_data)) @@ -3091,6 +3095,9 @@ int ll_iocontrol(struct inode *inode, struct file *file, case IOC_OBD_STATFS: RETURN(ll_obd_statfs(inode, uarg)); case LL_IOC_GET_MDTIDX: { + if (!ll_access_ok(uarg, sizeof(rc))) + RETURN(-EFAULT); + rc = ll_get_mdt_idx(inode); if (rc < 0) RETURN(rc); diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c index dbbfd89..6d75167 100644 --- a/lustre/lmv/lmv_obd.c +++ b/lustre/lmv/lmv_obd.c @@ -853,10 +853,26 @@ static int lmv_iocontrol(unsigned int cmd, struct obd_export *exp, ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", exp->exp_obd->obd_name, cmd, len, karg, uarg); - if (count == 0) RETURN(-ENOTTY); + /* exit early for unknown ioctl types */ + if (unlikely(_IOC_TYPE(cmd) != 'f' && cmd != IOC_OSC_SET_ACTIVE)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "unknown", -ENOTTY)); + + /* handle commands that don't use @karg first */ + switch (cmd) { + case LL_IOC_GET_CONNECT_FLAGS: + tgt = lmv_tgt(lmv, 0); + rc = -ENODATA; + if (tgt && tgt->ltd_exp) + rc = obd_iocontrol(cmd, tgt->ltd_exp, len, NULL, uarg); + RETURN(rc); + } + + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", -EINVAL)); + switch (cmd) { case IOC_OBD_STATFS: { struct obd_ioctl_data *data = karg; @@ -946,13 +962,6 @@ static int lmv_iocontrol(unsigned int cmd, struct obd_export *exp, OBD_FREE_PTR(oqctl); break; } - case LL_IOC_GET_CONNECT_FLAGS: { - tgt = lmv_tgt(lmv, 0); - rc = -ENODATA; - if (tgt && tgt->ltd_exp) - rc = obd_iocontrol(cmd, tgt->ltd_exp, len, karg, uarg); - break; - } case LL_IOC_FID2MDTIDX: { struct lu_fid *fid = karg; int mdt_index; @@ -1089,12 +1098,16 @@ hsm_req_err: tgt->ltd_uuid.uuid, err); if (!rc) rc = err; + if (unlikely(err == -ENOTTY)) + break; } - } else + } else { set = 1; + } } if (!set && !rc) rc = -EIO; + break; } RETURN(rc); } diff --git a/lustre/lod/lod_dev.c b/lustre/lod/lod_dev.c index a7364cf..7ecd1e9 100644 --- a/lustre/lod/lod_dev.c +++ b/lustre/lod/lod_dev.c @@ -2738,13 +2738,16 @@ static int lod_iocontrol(unsigned int cmd, struct obd_export *exp, int len, { struct obd_device *obd = exp->exp_obd; struct lod_device *lod = lu2lod_dev(obd->obd_lu_dev); - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; struct lu_env env; int rc; ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", obd->obd_name, cmd, len, karg, uarg); + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", -EINVAL)); + data = karg; rc = lu_env_init(&env, LCT_LOCAL | LCT_MD_THREAD); if (rc) { diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 8af1b53..f48a82b 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -957,15 +957,28 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len, CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", exp->exp_obd->obd_name, cmd, len, karg, uarg); + /* exit early for unknown ioctl types */ + if (unlikely(_IOC_TYPE(cmd) != 'f' && cmd != IOC_OSC_SET_ACTIVE)) + RETURN(OBD_IOC_DEBUG(D_IOCTL, obd->obd_name, cmd, "unknown", + -ENOTTY)); + + /* can't do a generic karg == NULL check here, since it is too noisy and + * we need to return -ENOTTY for unsupported ioctls instead of -EINVAL. + */ switch (cmd) { case IOC_OBD_STATFS: { - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; struct obd_device *osc_obd; struct obd_statfs stat_buf = {0}; struct obd_import *imp; __u32 index; __u32 flags; + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=null", + -EINVAL)); + data = karg; + memcpy(&index, data->ioc_inlbuf2, sizeof(index)); if (index >= count) RETURN(-ENODEV); @@ -1005,11 +1018,16 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len, break; } case OBD_IOC_QUOTACTL: { - struct if_quotactl *qctl = karg; + struct if_quotactl *qctl; struct lov_tgt_desc *tgt = NULL; struct obd_quotactl *oqctl; struct obd_import *imp; + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=null", + -EINVAL)); + qctl = karg; + if (qctl->qc_valid == QC_OSTIDX) { if (count <= qctl->qc_idx) RETURN(-EINVAL); @@ -1091,6 +1109,9 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len, err); if (!rc) rc = err; + + if (err == -ENOTTY) + break; } } else { set = 1; @@ -1098,6 +1119,7 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len, } if (!set && !rc) rc = -EIO; + break; } } diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c index dd69a1d..f87acc7 100644 --- a/lustre/mdc/mdc_request.c +++ b/lustre/mdc/mdc_request.c @@ -2197,7 +2197,7 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, void *karg, void __user *uarg) { struct obd_device *obd = exp->exp_obd; - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; struct obd_import *imp = obd->u.cli.cl_import; int rc; @@ -2205,6 +2205,19 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", obd->obd_name, cmd, len, karg, uarg); + /* handle commands that do not need @karg first */ + switch (cmd) { + case LL_IOC_GET_CONNECT_FLAGS: + if (copy_to_user(uarg, exp_connect_flags_ptr(exp), + sizeof(*exp_connect_flags_ptr(exp)))) + RETURN(-EFAULT); + RETURN(0); + } + + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", -EINVAL)); + data = karg; + if (!try_module_get(THIS_MODULE)) { CERROR("%s: cannot get module '%s'\n", obd->obd_name, module_name(THIS_MODULE)); @@ -2294,12 +2307,6 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, OBD_FREE_PTR(oqctl); GOTO(out, rc); } - case LL_IOC_GET_CONNECT_FLAGS: - if (copy_to_user(uarg, exp_connect_flags_ptr(exp), - sizeof(*exp_connect_flags_ptr(exp)))) - GOTO(out, rc = -EFAULT); - - GOTO(out, rc = 0); case LL_IOC_LOV_SWAP_LAYOUTS: rc = mdc_ioc_swap_layouts(exp, karg); GOTO(out, rc); diff --git a/lustre/mdd/mdd_device.c b/lustre/mdd/mdd_device.c index 396c422..ca2eae0 100644 --- a/lustre/mdd/mdd_device.c +++ b/lustre/mdd/mdd_device.c @@ -2271,12 +2271,15 @@ static int mdd_iocontrol(const struct lu_env *env, struct md_device *m, { struct mdd_device *mdd = lu2mdd_dev(&m->md_lu_dev); struct obd_device *obd = mdd2obd_dev(mdd); - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; int rc = -EINVAL; ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK\n", obd->obd_name, cmd, len, karg); + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", rc)); + data = karg; /* Doesn't use obd_ioctl_data */ switch (cmd) { diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index ebdf5d6..ea6c082c3 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -7655,6 +7655,7 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, struct obd_device *obd = exp->exp_obd; struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); struct dt_device *dt = mdt->mdt_bottom; + struct obd_ioctl_data *data; struct lu_env env; int rc; @@ -7666,25 +7667,35 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, if (rc) RETURN(rc); + /* handle commands that don't use @karg first */ switch (cmd) { case OBD_IOC_SYNC: rc = mdt_device_sync(&env, mdt); - break; + GOTO(out, rc); case OBD_IOC_SET_READONLY: rc = dt_sync(&env, dt); if (rc == 0) rc = dt_ro(&env, dt); - break; - case OBD_IOC_ABORT_RECOVERY: { - struct obd_ioctl_data *data = karg; + GOTO(out, rc); + } + if (unlikely(karg == NULL)) { + OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", rc = -EINVAL); + GOTO(out, rc); + } + data = karg; + + switch (cmd) { + case OBD_IOC_ABORT_RECOVERY: { if (data->ioc_type & OBD_FLG_ABORT_RECOV_MDT) { - CWARN("%s: Aborting MDT recovery\n", obd->obd_name); + LCONSOLE_WARN("%s: Aborting MDT recovery\n", + obd->obd_name); obd->obd_abort_mdt_recovery = 1; wake_up(&obd->obd_next_transno_waitq); } else { /* if (data->ioc_type & OBD_FLG_ABORT_RECOV_OST) */ /* lctl didn't set OBD_FLG_ABORT_RECOV_OST < 2.13.57 */ - CWARN("%s: Aborting client recovery\n", obd->obd_name); + LCONSOLE_WARN("%s: Aborting client recovery\n", + obd->obd_name); obd->obd_abort_recovery = 1; target_stop_recovery_thread(obd); } @@ -7696,29 +7707,21 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, case OBD_IOC_CHANGELOG_CLEAR: case OBD_IOC_LLOG_PRINT: case OBD_IOC_LLOG_CANCEL: - rc = mdt->mdt_child->md_ops->mdo_iocontrol(&env, - mdt->mdt_child, + rc = mdt->mdt_child->md_ops->mdo_iocontrol(&env, mdt->mdt_child, cmd, len, karg); break; case OBD_IOC_START_LFSCK: { struct md_device *next = mdt->mdt_child; - struct obd_ioctl_data *data = karg; struct lfsck_start_param lsp; - if (unlikely(data == NULL)) { - rc = -EINVAL; - break; - } - lsp.lsp_start = (struct lfsck_start *)(data->ioc_inlbuf1); lsp.lsp_index_valid = 0; rc = next->md_ops->mdo_iocontrol(&env, next, cmd, 0, &lsp); break; } case OBD_IOC_STOP_LFSCK: { - struct md_device *next = mdt->mdt_child; - struct obd_ioctl_data *data = karg; - struct lfsck_stop stop; + struct md_device *next = mdt->mdt_child; + struct lfsck_stop stop; stop.ls_status = LS_STOPPED; /* Old lfsck utils may pass NULL @stop. */ @@ -7732,8 +7735,7 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, break; } case OBD_IOC_QUERY_LFSCK: { - struct md_device *next = mdt->mdt_child; - struct obd_ioctl_data *data = karg; + struct md_device *next = mdt->mdt_child; rc = next->md_ops->mdo_iocontrol(&env, next, cmd, 0, data->ioc_inlbuf1); @@ -7764,7 +7766,7 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, rc = OBD_IOC_ERROR(obd->obd_name, cmd, "unrecognized", -ENOTTY); break; } - +out: lu_env_fini(&env); RETURN(rc); } diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h index 13df6a2..fb6a4f9 100644 --- a/lustre/mdt/mdt_internal.h +++ b/lustre/mdt/mdt_internal.h @@ -1441,7 +1441,7 @@ static inline bool mdt_changelog_allow(struct mdt_thread_info *info) /* return true in case old client did not send mdt body */ return true; #else - return false + return false; #endif rc = mdt_init_ucred(info, (struct mdt_body *)info->mti_body); diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c index 0d8b4ec..26287c1 100644 --- a/lustre/mgs/mgs_handler.c +++ b/lustre/mgs/mgs_handler.c @@ -991,13 +991,16 @@ static int mgs_iocontrol(unsigned int cmd, struct obd_export *exp, int len, { struct obd_device *obd = exp->exp_obd; struct mgs_device *mgs = exp2mgs_dev(exp); - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; struct lu_env env; int rc = -EINVAL; ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", obd->obd_name, cmd, len, karg, uarg); + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", rc)); + data = karg; rc = lu_env_init(&env, LCT_MG_THREAD); if (rc) @@ -1067,8 +1070,8 @@ out_free: rc = mgs_replace_nids(&env, mgs, data->ioc_inlbuf1, data->ioc_inlbuf2); if (rc) - CERROR("%s: error replacing nids: rc = %d\n", - obd->obd_name, rc); + CERROR("%s: error replacing NIDs for '%s': rc = %d\n", + obd->obd_name, data->ioc_inlbuf1, rc); break; case OBD_IOC_CLEAR_CONFIGS: @@ -1119,20 +1122,20 @@ out_free: break; case OBD_IOC_LLOG_CANCEL: case OBD_IOC_LLOG_REMOVE: - case OBD_IOC_LLOG_CHECK: - case OBD_IOC_LLOG_INFO: - case OBD_IOC_LLOG_PRINT: { - struct llog_ctxt *ctxt; + case OBD_IOC_LLOG_CHECK: + case OBD_IOC_LLOG_INFO: + case OBD_IOC_LLOG_PRINT: { + struct llog_ctxt *ctxt; ctxt = llog_get_context(mgs->mgs_obd, LLOG_CONFIG_ORIG_CTXT); rc = llog_ioctl(&env, ctxt, cmd, data); llog_ctxt_put(ctxt); break; - } - default: + } + default: rc = OBD_IOC_ERROR(obd->obd_name, cmd, "unrecognized", -ENOTTY); break; - } + } out: lu_env_fini(&env); RETURN(rc); diff --git a/lustre/obdclass/class_obd.c b/lustre/obdclass/class_obd.c index b71f767..6b13000 100644 --- a/lustre/obdclass/class_obd.c +++ b/lustre/obdclass/class_obd.c @@ -296,16 +296,11 @@ int obd_ioctl_getdata(struct obd_ioctl_data **datap, int *len, void __user *arg) } *len = hdr.ioc_len; - if (copy_from_user(data, arg, hdr.ioc_len)) { - OBD_FREE_LARGE(data, hdr.ioc_len); - RETURN(-EFAULT); - } + if (copy_from_user(data, arg, hdr.ioc_len)) + GOTO(out_free, rc = -EFAULT); - if (obd_ioctl_is_invalid(data)) { - CERROR("ioctl not correctly formatted\n"); - OBD_FREE_LARGE(data, hdr.ioc_len); - RETURN(-EINVAL); - } + if (obd_ioctl_is_invalid(data)) + GOTO(out_free, rc = -EINVAL); if (data->ioc_inllen1) { data->ioc_inlbuf1 = &data->ioc_bulk[0]; @@ -328,6 +323,10 @@ int obd_ioctl_getdata(struct obd_ioctl_data **datap, int *len, void __user *arg) *datap = data; RETURN(0); + +out_free: + OBD_FREE_LARGE(data, *len); + RETURN(rc); } EXPORT_SYMBOL(obd_ioctl_getdata); @@ -339,6 +338,10 @@ int class_handle_ioctl(unsigned int cmd, void __user *uarg) ENTRY; CDEBUG(D_IOCTL, "obdclass: cmd=%x len=%u uarg=%pK\n", cmd, len, uarg); + if (unlikely(_IOC_TYPE(cmd) != 'f' && cmd != IOC_OSC_SET_ACTIVE && + cmd != OBD_IOC_BARRIER)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "unknown", -ENOTTY)); + rc = obd_ioctl_getdata(&data, &len, uarg); if (rc) { CERROR("%s: ioctl data error: rc = %d\n", current->comm, rc); diff --git a/lustre/obdecho/echo_client.c b/lustre/obdecho/echo_client.c index d6bc42e..16075ba 100644 --- a/lustre/obdecho/echo_client.c +++ b/lustre/obdecho/echo_client.c @@ -2264,7 +2264,7 @@ echo_client_iocontrol(unsigned int cmd, struct obd_export *exp, int len, struct echo_device *ed = obd2echo_dev(obd); struct echo_client_obd *ec = ed->ed_ec; struct echo_object *eco; - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; struct lu_env *env; unsigned long env_tags = 0; __u16 refcheck; @@ -2276,6 +2276,9 @@ echo_client_iocontrol(unsigned int cmd, struct obd_export *exp, int len, ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", exp->exp_obd->obd_name, cmd, len, karg, uarg); + if (unlikely(karg == NULL)) + RETURN(OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", rc)); + data = karg; oa = &data->ioc_obdo1; if (!(oa->o_valid & OBD_MD_FLGROUP)) { diff --git a/lustre/ofd/ofd_obd.c b/lustre/ofd/ofd_obd.c index 9313dd6..1f59f94 100644 --- a/lustre/ofd/ofd_obd.c +++ b/lustre/ofd/ofd_obd.c @@ -1181,8 +1181,7 @@ static int ofd_ioc_get_obj_version(const struct lu_env *env, int rc = 0; ENTRY; - - if (!data->ioc_inlbuf2 || data->ioc_inllen2 != sizeof(version)) + if (!data || !data->ioc_inlbuf2 || data->ioc_inllen2 != sizeof(version)) GOTO(out, rc = -EINVAL); if (data->ioc_inlbuf1 && data->ioc_inllen1 == sizeof(fid)) { @@ -1252,6 +1251,7 @@ static int ofd_iocontrol(unsigned int cmd, struct obd_export *exp, int len, struct lu_env env; struct ofd_device *ofd = ofd_exp(exp); struct obd_device *obd = ofd_obd(ofd); + struct obd_ioctl_data *data; int rc; ENTRY; @@ -1261,38 +1261,42 @@ static int ofd_iocontrol(unsigned int cmd, struct obd_export *exp, int len, if (rc) RETURN(rc); + /* handle commands that don't use @karg first */ + rc = -EINVAL; switch (cmd) { case OBD_IOC_ABORT_RECOVERY: - CWARN("%s: Aborting recovery\n", obd->obd_name); + LCONSOLE_WARN("%s: Aborting recovery\n", obd->obd_name); obd->obd_abort_recovery = 1; target_stop_recovery_thread(obd); - break; + GOTO(out, rc); case OBD_IOC_SYNC: CDEBUG(D_RPCTRACE, "syncing ost %s\n", obd->obd_name); rc = dt_sync(&env, ofd->ofd_osd); - break; + GOTO(out, rc); case OBD_IOC_SET_READONLY: rc = dt_sync(&env, ofd->ofd_osd); if (rc == 0) rc = dt_ro(&env, ofd->ofd_osd); - break; + GOTO(out, rc); + } + + if (unlikely(karg == NULL)) { + OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", rc = -EINVAL); + GOTO(out, rc); + } + data = karg; + + switch (cmd) { case OBD_IOC_START_LFSCK: { - struct obd_ioctl_data *data = karg; struct lfsck_start_param lsp; - if (unlikely(!data)) { - rc = -EINVAL; - break; - } - lsp.lsp_start = (struct lfsck_start *)(data->ioc_inlbuf1); lsp.lsp_index_valid = 0; rc = lfsck_start(&env, ofd->ofd_osd, &lsp); break; } case OBD_IOC_STOP_LFSCK: { - struct obd_ioctl_data *data = karg; - struct lfsck_stop stop; + struct lfsck_stop stop; stop.ls_status = LS_STOPPED; /* Old lfsck utils may pass NULL @stop. */ @@ -1312,7 +1316,7 @@ static int ofd_iocontrol(unsigned int cmd, struct obd_export *exp, int len, rc = OBD_IOC_ERROR(obd->obd_name, cmd, "unrecognized", -ENOTTY); break; } - +out: lu_env_fini(&env); RETURN(rc); } diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index bc3be85..5ee8a36 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -3333,8 +3333,8 @@ static int osc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, void *karg, void __user *uarg) { struct obd_device *obd = exp->exp_obd; - struct obd_ioctl_data *data = karg; - int rc = 0; + struct obd_ioctl_data *data; + int rc; ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", @@ -3343,19 +3343,38 @@ static int osc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, if (!try_module_get(THIS_MODULE)) { CERROR("%s: cannot get module '%s'\n", obd->obd_name, module_name(THIS_MODULE)); - return -EINVAL; + RETURN(-EINVAL); } + switch (cmd) { case OBD_IOC_CLIENT_RECOVER: + if (unlikely(karg == NULL)) { + OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", + rc = -EINVAL); + break; + } + data = karg; rc = ptlrpc_recover_import(obd->u.cli.cl_import, data->ioc_inlbuf1, 0); if (rc > 0) rc = 0; break; case OBD_IOC_GETATTR: + if (unlikely(karg == NULL)) { + OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", + rc = -EINVAL); + break; + } + data = karg; rc = obd_getattr(NULL, exp, &data->ioc_obdo1); break; case IOC_OSC_SET_ACTIVE: + if (unlikely(karg == NULL)) { + OBD_IOC_ERROR(obd->obd_name, cmd, "karg=NULL", + rc = -EINVAL); + break; + } + data = karg; rc = ptlrpc_set_import_active(obd->u.cli.cl_import, data->ioc_offset); break; diff --git a/lustre/osp/osp_dev.c b/lustre/osp/osp_dev.c index 3566877..ae3bb90 100644 --- a/lustre/osp/osp_dev.c +++ b/lustre/osp/osp_dev.c @@ -1669,12 +1669,18 @@ static int osp_iocontrol(unsigned int cmd, struct obd_export *exp, int len, { struct obd_device *obd = exp->exp_obd; struct osp_device *d; - struct obd_ioctl_data *data = karg; + struct obd_ioctl_data *data; int rc = -EINVAL; ENTRY; CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n", exp->exp_obd->obd_name, cmd, len, karg, uarg); + if (unlikely(karg == NULL)) { + CERROR("%s: iocontrol from '%s' cmd=%x karg=NULL: rc = %d\n", + obd->obd_name, current->comm, cmd, rc); + RETURN(rc); + } + data = karg; LASSERT(obd->obd_lu_dev); d = lu2osp_dev(obd->obd_lu_dev); diff --git a/lustre/tests/group_lock_test.c b/lustre/tests/group_lock_test.c index f20010d..1c3a10f 100644 --- a/lustre/tests/group_lock_test.c +++ b/lustre/tests/group_lock_test.c @@ -288,23 +288,23 @@ static void helper_test20(int fd) gid = 1234; rc = ioctl(fd, LL_IOC_GROUP_LOCK, gid); - ASSERTF(rc == -1 && errno == ENOTTY, "unexpected retval: %d %s", - rc, strerror(errno)); + ASSERTF(rc == -1 && (errno == ENOTTY || errno == EINVAL), + "unexpected retval: %d %s", rc, strerror(errno)); gid = 0; rc = ioctl(fd, LL_IOC_GROUP_LOCK, gid); - ASSERTF(rc == -1 && errno == ENOTTY, "unexpected retval: %d %s", - rc, strerror(errno)); + ASSERTF(rc == -1 && (errno == ENOTTY || errno == EINVAL), + "unexpected retval: %d %s", rc, strerror(errno)); gid = 1; rc = ioctl(fd, LL_IOC_GROUP_LOCK, gid); - ASSERTF(rc == -1 && errno == ENOTTY, "unexpected retval: %d %s", - rc, strerror(errno)); + ASSERTF(rc == -1 && (errno == ENOTTY || errno == EINVAL), + "unexpected retval: %d %s", rc, strerror(errno)); gid = -1; rc = ioctl(fd, LL_IOC_GROUP_LOCK, gid); - ASSERTF(rc == -1 && errno == ENOTTY, "unexpected retval: %d %s", - rc, strerror(errno)); + ASSERTF(rc == -1 && (errno == ENOTTY || errno == EINVAL), + "unexpected retval: %d %s", rc, strerror(errno)); } /* Test lock / unlock on a directory */ -- 1.8.3.1