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 <adilger@whamcloud.com>
Reported-by: Tao Lyu <tao.lyu@epfl.ch>
Change-Id: I1a0d2f839949debf346aa15c65b0f407e9ce7057
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/50314
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Vitaliy Kuznetsov <vkuznetsov@ddn.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/libcfs/libcfs/crypto/policy.c b/libcfs/libcfs/crypto/policy.c
index 5d094d5..38477bd 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -36,19 +36,20 @@
 #define DEBUG_SUBSYSTEM S_LLITE
 
 #include <linux/cpu.h>
+#include <linux/delay.h>
+#include <linux/file.h>
+#include <linux/fs_struct.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/random.h>
 #include <linux/statfs.h>
 #include <linux/time.h>
-#include <linux/file.h>
 #include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/uidgid.h>
+#include <linux/user_namespace.h>
 #include <linux/uuid.h>
 #include <linux/version.h>
-#include <linux/mm.h>
-#include <linux/user_namespace.h>
-#include <linux/delay.h>
-#include <linux/uidgid.h>
-#include <linux/fs_struct.h>
 
 #ifndef HAVE_CPUS_READ_LOCK
 #include <libcfs/linux/linux-cpu.h>
@@ -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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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..ea6c082 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 */