From 496b2b5190703ae6cec88568a6837f0347185fd9 Mon Sep 17 00:00:00 2001
From: Andreas Dilger <adilger@whamcloud.com>
Date: Mon, 13 Mar 2023 23:26:10 -0600
Subject: [PATCH] LU-16634 misc: standardize handling of ioctl params

Pass "void __user *" for @uarg argument to *_iocontrol() functions
so that it does not need to be typecast on every access therein.

Consistently log the ioctl params and errors in the *_iocontrol()
methods to aid in debugging.  Include the process name in error
messages to help find programs calling bad ioctl commands.

Check return pointer before doing lots of work for some commands.
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.

Validate uarg and karg early in command processing where possible.

Convert indentation to tabs for lines previously using spaces.
Change local varaiable declarations to use only a single space.
Minor other code style cleanups in related code.

Change checkpatch.pl to allow RETURN/GOTO as "end of switch case".
Improve CERROR/CWARN/LCONSOLE/CDEBUG message checking/warning.
Allow "DFID|DOSTID" within long error strings without complaint.

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I1a0d2f839949debf346aa15c65b0f407e9ce7057
---

diff --git a/contrib/scripts/checkpatch.pl b/contrib/scripts/checkpatch.pl
index e91aeb3..7372ed5 100755
--- a/contrib/scripts/checkpatch.pl
+++ b/contrib/scripts/checkpatch.pl
@@ -2894,6 +2894,10 @@
 			if ($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(?:KERN_\S+\s*|[^"]*))?($String\s*(?:|,|\)\s*;)\s*)$/ &&
 			    length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) {
 				$msg_type = "";
+			# a Lustre message that contains embedded formatting
+			} elsif ($line =~ /^\+\s*(?:$logFunctions\s*\()?($String(?:DFID|DOSTID)$String\s*(?:|,|\)\s*;)?\s*)$/ &&
+				 length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) {
+				$msg_type = ""
 
 			# lines with only strings (w/ possible termination)
 			# #defines with only strings
@@ -5168,28 +5172,42 @@
 			}
 		}
 
-# try to replace assertions with error handling
+# Lustre try to replace assertions with error handling
 		if ($line =~ /\bLASSERTF?\s*\(/) {
 		    WARN("LASSERT",
 			 "try to replace assertions with error handling\n" .
 			 $herecurr);
 		}
 
-# avoid new console messages
+# Lustre avoid new console messages
 		if ($line =~ /\bLCONSOLE[A-Z_]*\s*\(/) {
 		    WARN("LCONSOLE",
 			 "avoid adding new console messages\n" .
 			 $herecurr);
 		}
 
-# minimize new CERROR messages
-		if ($line =~ /\bC(EMERG|ERROR|NETERR|WARN)\s*\(/) {
-		    WARN("CERROR",
-			 "think hard when adding new CERROR messages\n" .
-			 $herecurr);
+# Lustre minimize new CERROR messages
+		if ($line =~ /\b(CEMERG|CERROR|CNETERR|CWARN|LCONSOLE)\s*\(/) {
+			if (! $line =~ /\(\"\%s: /) {
+				WARN("CERROR_DEV",
+				     "console messages should start with '%s:' to print device name\n" . $herecurr)
+			}
+			if (! $line =~ /: rc = \%d\\n\",/) {
+				WARN("CERROR_RET",
+				     "console messages should end with ': rc = %d'\n" . $herecurr);
+			}
+			if ($line =~ /(CEMERG|CERROR|CNETERR)/) {
+				WARN("CERROR",
+				     "errors should be useful to fix failure conditions, not status/debug\n" . $herecurr);
+			}
+		}
+# Lustre avoid unlimited message printing to the console
+		if ($line =~ /CDEBUG\(.*(D_ERROR|D_WARNING|D_CONSOLE|[a-z])/) {
+			WARN("CDEBUG_LIMIT",
+			     "CDEBUG does not rate-limit messages to console, use CDEBUG_LIMIT\n". $herecurr);
 		}
 
-# don't allow GPLv3 license files
+# Lustre don't allow GPLv3 license files
 		if ($rawline =~ /version 3/) {
 			WARN("GPLV3",
 			     "using GPLv3 is usually wrong\n" . $herecurr);
@@ -6116,7 +6134,7 @@
 				next if ($fline =~ /^.[\s$;]*$/);
 				$has_statement = 1;
 				$count++;
-				$has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|return\b|goto\b|continue\b)/);
+				$has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|return\b|goto\b|continue\b)/i);
 			}
 			if (!$has_break && $has_statement) {
 				WARN("MISSING_BREAK",
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 @@
 		     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 4f2bd8a..2df2de7 100644
--- a/lustre/include/lustre_compat.h
+++ b/lustre/include/lustre_compat.h
@@ -620,4 +620,10 @@
 }
 #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
+
 #endif /* _LUSTRE_COMPAT_H */
diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h
index d56fc5b..4033524 100644
--- a/lustre/include/obd_class.h
+++ b/lustre/include/obd_class.h
@@ -61,7 +61,7 @@
 
 /* OBD Operations Declarations */
 extern struct obd_device *class_exp2obd(struct obd_export *);
-extern int class_handle_ioctl(unsigned int cmd, unsigned long arg);
+extern int class_handle_ioctl(unsigned int cmd, void __user *uarg);
 int lustre_get_jobid(char *jobid, size_t len);
 void lustre_jobid_clear(const char *jobid);
 void jobid_cache_fini(void);
diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c
index 0ff2e45..c3723de 100644
--- a/lustre/llite/dir.c
+++ b/lustre/llite/dir.c
@@ -1496,11 +1496,12 @@
 	struct inode *inode = file_inode(file);
 	struct ll_sb_info *sbi = ll_i2sbi(inode);
 	struct obd_ioctl_data *data = NULL;
+	void __user *uarg = (void __user *)arg;
 	int rc = 0;
 	ENTRY;
 
-	CDEBUG(D_VFSTRACE, "VFS Op:inode="DFID"(%p), cmd=%#x\n",
-	       PFID(ll_inode2fid(inode)), inode, cmd);
+	CDEBUG(D_VFSTRACE|D_IOCTL, "VFS Op:inode="DFID"(%pK) cmd=%x arg=%lx\n",
+	       PFID(ll_inode2fid(inode)), inode, cmd, arg);
 
 	/* asm-ppc{,64} declares TCGETS, et. al. as type 't' not 'T' */
 	if (_IOC_TYPE(cmd) == 'T' || _IOC_TYPE(cmd) == 't') /* tty ioctls */
@@ -1510,7 +1511,7 @@
 	switch (cmd) {
 	case FS_IOC_GETFLAGS:
 	case FS_IOC_SETFLAGS:
-		RETURN(ll_iocontrol(inode, file, cmd, arg));
+		RETURN(ll_iocontrol(inode, file, cmd, uarg));
 	case FSFILT_IOC_GETVERSION:
 	case FS_IOC_GETVERSION:
 		RETURN(put_user(inode->i_generation, (int __user *)arg));
@@ -1536,7 +1537,7 @@
 		int namelen, len = 0;
 		char *filename;
 
-		rc = obd_ioctl_getdata(&data, &len, (void __user *)arg);
+		rc = obd_ioctl_getdata(&data, &len, uarg);
 		if (rc != 0)
 			RETURN(rc);
 
@@ -1567,7 +1568,7 @@
 		int len;
 		int rc;
 
-		rc = obd_ioctl_getdata(&data, &len, (void __user *)arg);
+		rc = obd_ioctl_getdata(&data, &len, uarg);
 		if (rc)
 			RETURN(rc);
 
@@ -1616,10 +1617,9 @@
 
 	}
 	case LL_IOC_LMV_SET_DEFAULT_STRIPE: {
-		struct lmv_user_md	  lum;
-		struct lmv_user_md __user *ulump =
-					(struct lmv_user_md __user *)arg;
-		int			  rc;
+		struct lmv_user_md lum;
+		struct lmv_user_md __user *ulump = uarg;
+		int rc;
 
 		if (copy_from_user(&lum, ulump, sizeof(lum)))
 			RETURN(-EFAULT);
@@ -1636,12 +1636,9 @@
 		struct lov_user_md_v3 *lumv3 = NULL;
 		struct lov_user_md_v1 lumv1;
 		struct lov_user_md_v1 *lumv1_ptr = &lumv1;
-		struct lov_user_md_v1 __user *lumv1p =
-			(struct lov_user_md_v1 __user *)arg;
-		struct lov_user_md_v3 __user *lumv3p =
-			(struct lov_user_md_v3 __user *)arg;
+		struct lov_user_md_v1 __user *lumv1p = uarg;
+		struct lov_user_md_v3 __user *lumv3p = uarg;
 		int lum_size = 0;
-
 		int set_default = 0;
 
 		BUILD_BUG_ON(sizeof(struct lov_user_md_v3) <=
@@ -1681,21 +1678,20 @@
 		RETURN(rc);
 	}
 	case LL_IOC_LMV_GETSTRIPE: {
-		struct lmv_user_md __user *ulmv =
-					(struct lmv_user_md __user *)arg;
-		struct lmv_user_md	lum;
-		struct ptlrpc_request	*request = NULL;
-		struct ptlrpc_request	*root_request = NULL;
-		union lmv_mds_md	*lmm = NULL;
-		int			lmmsize;
-		u64			valid = 0;
-		struct lmv_user_md	*tmp = NULL;
-		int			mdt_index;
-		int			lum_size;
-		int			stripe_count;
-		int			max_stripe_count;
-		int			i;
-		int			rc;
+		struct lmv_user_md __user *ulmv = uarg;
+		struct lmv_user_md lum;
+		struct ptlrpc_request *request = NULL;
+		struct ptlrpc_request *root_request = NULL;
+		union lmv_mds_md *lmm = NULL;
+		int lmmsize;
+		u64 valid = 0;
+		struct lmv_user_md *tmp = NULL;
+		int mdt_index;
+		int lum_size;
+		int stripe_count;
+		int max_stripe_count;
+		int i;
+		int rc;
 
 		if (copy_from_user(&lum, ulmv, sizeof(*ulmv)))
 			RETURN(-EFAULT);
@@ -1872,7 +1868,7 @@
 		if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_LVB_TYPE))
 			RETURN(-ENOTSUPP);
 
-		filename = ll_getname((const char __user *)arg);
+		filename = ll_getname(uarg);
 		if (IS_ERR(filename))
 			RETURN(PTR_ERR(filename));
 
@@ -1882,16 +1878,16 @@
 
 		rc = ll_rmdir_entry(inode, filename, namelen);
 out_rmdir:
-                if (filename)
-                        ll_putname(filename);
+		if (filename)
+			ll_putname(filename);
 		RETURN(rc);
 	}
 	case LL_IOC_RMFID:
-		RETURN(ll_rmfid(file, (void __user *)arg));
+		RETURN(ll_rmfid(file, uarg));
 	case LL_IOC_LOV_SWAP_LAYOUTS:
 		RETURN(-EPERM);
 	case IOC_OBD_STATFS:
-		RETURN(ll_obd_statfs(inode, (void __user *)arg));
+		RETURN(ll_obd_statfs(inode, uarg));
 	case LL_IOC_LOV_GETSTRIPE:
 	case LL_IOC_LOV_GETSTRIPE_NEW:
 	case LL_IOC_MDC_GETINFO_V1:
@@ -1916,7 +1912,7 @@
 		if (cmd == IOC_MDC_GETFILEINFO_V1 ||
 		    cmd == IOC_MDC_GETFILEINFO_V2 ||
 		    cmd == IOC_MDC_GETFILESTRIPE) {
-			filename = ll_getname((const char __user *)arg);
+			filename = ll_getname(uarg);
 			if (IS_ERR(filename))
 				RETURN(PTR_ERR(filename));
 
@@ -1950,18 +1946,18 @@
 		if (cmd == IOC_MDC_GETFILESTRIPE ||
 		    cmd == LL_IOC_LOV_GETSTRIPE ||
 		    cmd == LL_IOC_LOV_GETSTRIPE_NEW) {
-			lump = (struct lov_user_md __user *)arg;
+			lump = uarg;
 		} else if (cmd == IOC_MDC_GETFILEINFO_V1 ||
 			   cmd == LL_IOC_MDC_GETINFO_V1){
 			struct lov_user_mds_data_v1 __user *lmdp;
 
-			lmdp = (struct lov_user_mds_data_v1 __user *)arg;
+			lmdp = uarg;
 			statp = &lmdp->lmd_st;
 			lump = &lmdp->lmd_lmm;
 		} else {
 			struct lov_user_mds_data __user *lmdp;
 
-			lmdp = (struct lov_user_mds_data __user *)arg;
+			lmdp = uarg;
 			fidp = &lmdp->lmd_fid;
 			stxp = &lmdp->lmd_stx;
 			flagsp = &lmdp->lmd_flags;
@@ -2085,11 +2081,11 @@
 		if (!qctl)
 			RETURN(-ENOMEM);
 
-		if (copy_from_user(qctl, (void __user *)arg, sizeof(*qctl)))
+		if (copy_from_user(qctl, uarg, sizeof(*qctl)))
 			GOTO(out_quotactl, rc = -EFAULT);
 
 		if (LUSTRE_Q_CMD_IS_POOL(qctl->qc_cmd)) {
-			char __user *from = (char __user *)arg +
+			char __user *from = uarg +
 					offsetof(typeof(*qctl), qc_poolname);
 			if (copy_from_user(qctl->qc_poolname, from,
 					   LOV_MAXPOOLNAME + 1))
@@ -2098,61 +2094,58 @@
 
 		rc = quotactl_ioctl(inode->i_sb, qctl);
 		if ((rc == 0 || rc == -ENODATA) &&
-		    copy_to_user((void __user *)arg, qctl, sizeof(*qctl)))
-                        rc = -EFAULT;
-
+		    copy_to_user(uarg, qctl, sizeof(*qctl)))
+			rc = -EFAULT;
 out_quotactl:
 		OBD_FREE(qctl, qctl_len);
 		RETURN(rc);
-        }
+	}
 	case OBD_IOC_GETNAME_OLD:
 	case OBD_IOC_GETDTNAME:
 	case OBD_IOC_GETMDNAME:
-                RETURN(ll_get_obd_name(inode, cmd, arg));
-        case LL_IOC_FLUSHCTX:
-                RETURN(ll_flush_ctx(inode));
-        case LL_IOC_GETOBDCOUNT: {
+		RETURN(ll_get_obd_name(inode, cmd, uarg));
+	case LL_IOC_FLUSHCTX:
+		RETURN(ll_flush_ctx(inode));
+	case LL_IOC_GETOBDCOUNT: {
 		u32 count, vallen;
-                struct obd_export *exp;
+		struct obd_export *exp;
 
-		if (copy_from_user(&count, (int __user *)arg, sizeof(int)))
-                        RETURN(-EFAULT);
+		if (copy_from_user(&count, uarg, sizeof(int)))
+			RETURN(-EFAULT);
 
-                /* get ost count when count is zero, get mdt count otherwise */
-                exp = count ? sbi->ll_md_exp : sbi->ll_dt_exp;
-                vallen = sizeof(count);
+		/* get ost count when count is zero, get mdt count otherwise */
+		exp = count ? sbi->ll_md_exp : sbi->ll_dt_exp;
+		vallen = sizeof(count);
 		rc = obd_get_info(NULL, exp, sizeof(KEY_TGT_COUNT),
 				  KEY_TGT_COUNT, &vallen, &count);
-                if (rc) {
-                        CERROR("get target count failed: %d\n", rc);
-                        RETURN(rc);
-                }
+		if (rc) {
+			CERROR("%s: get target count failed: rc = %d\n",
+			       sbi->ll_fsname, rc);
+			RETURN(rc);
+		}
 
-		if (copy_to_user((int __user *)arg, &count, sizeof(int)))
-                        RETURN(-EFAULT);
+		if (copy_to_user(uarg, &count, sizeof(int)))
+			RETURN(-EFAULT);
 
-                RETURN(0);
-        }
-        case LL_IOC_PATH2FID:
-		if (copy_to_user((void __user *)arg, ll_inode2fid(inode),
-                                     sizeof(struct lu_fid)))
-                        RETURN(-EFAULT);
-                RETURN(0);
-        case LL_IOC_GET_CONNECT_FLAGS: {
-		RETURN(obd_iocontrol(cmd, sbi->ll_md_exp, 0, NULL,
-				     (void __user *)arg));
-        }
+		RETURN(0);
+	}
+	case LL_IOC_PATH2FID:
+		if (copy_to_user(uarg, ll_inode2fid(inode),
+				 sizeof(struct lu_fid)))
+			RETURN(-EFAULT);
+		RETURN(0);
+	case LL_IOC_GET_CONNECT_FLAGS:
+		RETURN(obd_iocontrol(cmd, sbi->ll_md_exp, 0, NULL, uarg));
 	case OBD_IOC_FID2PATH:
-		RETURN(ll_fid2path(inode, (void __user *)arg));
+		RETURN(ll_fid2path(inode, uarg));
 	case LL_IOC_GETPARENT:
-		RETURN(ll_getparent(file, (void __user *)arg));
+		RETURN(ll_getparent(file, uarg));
 	case LL_IOC_FID2MDTIDX: {
 		struct obd_export *exp = ll_i2mdexp(inode);
-		struct lu_fid	  fid;
-		__u32		  index;
+		struct lu_fid fid;
+		__u32 index;
 
-		if (copy_from_user(&fid, (const struct lu_fid __user *)arg,
-				   sizeof(fid)))
+		if (copy_from_user(&fid, uarg, sizeof(fid)))
 			RETURN(-EFAULT);
 
 		/* Call mdc_iocontrol */
@@ -2164,15 +2157,15 @@
 		RETURN(index);
 	}
 	case LL_IOC_HSM_REQUEST: {
-		struct hsm_user_request	*hur;
-		ssize_t			 totalsize;
+		struct hsm_user_request *hur;
+		ssize_t totalsize;
 
 		OBD_ALLOC_PTR(hur);
 		if (hur == NULL)
 			RETURN(-ENOMEM);
 
 		/* We don't know the true size yet; copy the fixed-size part */
-		if (copy_from_user(hur, (void __user *)arg, sizeof(*hur))) {
+		if (copy_from_user(hur, uarg, sizeof(*hur))) {
 			OBD_FREE_PTR(hur);
 			RETURN(-EFAULT);
 		}
@@ -2192,7 +2185,7 @@
 			RETURN(-ENOMEM);
 
 		/* Copy the whole struct */
-		if (copy_from_user(hur, (void __user *)arg, totalsize))
+		if (copy_from_user(hur, uarg, totalsize))
 			GOTO(out_hur, rc = -EFAULT);
 
 		if (hur->hur_request.hr_action == HUA_RELEASE) {
@@ -2217,17 +2210,16 @@
 			rc = obd_iocontrol(cmd, ll_i2mdexp(inode), totalsize,
 					   hur, NULL);
 		}
-
 out_hur:
 		OBD_FREE_LARGE(hur, totalsize);
 
 		RETURN(rc);
 	}
 	case LL_IOC_HSM_PROGRESS: {
-		struct hsm_progress_kernel	hpk;
-		struct hsm_progress		hp;
+		struct hsm_progress_kernel hpk;
+		struct hsm_progress hp;
 
-		if (copy_from_user(&hp, (void __user *)arg, sizeof(hp)))
+		if (copy_from_user(&hp, uarg, sizeof(hp)))
 			RETURN(-EFAULT);
 
 		hpk.hpk_fid = hp.hp_fid;
@@ -2247,43 +2239,42 @@
 		if (!capable(CAP_SYS_ADMIN))
 			RETURN(-EPERM);
 
-		rc = copy_and_ct_start(cmd, sbi->ll_md_exp,
-				       (struct lustre_kernelcomm __user *)arg);
+		rc = copy_and_ct_start(cmd, sbi->ll_md_exp, uarg);
 		RETURN(rc);
 
 	case LL_IOC_HSM_COPY_START: {
-		struct hsm_copy	*copy;
-		int		 rc;
+		struct hsm_copy *copy;
+		int rc;
 
 		OBD_ALLOC_PTR(copy);
 		if (copy == NULL)
 			RETURN(-ENOMEM);
-		if (copy_from_user(copy, (char __user *)arg, sizeof(*copy))) {
+		if (copy_from_user(copy, uarg, sizeof(*copy))) {
 			OBD_FREE_PTR(copy);
 			RETURN(-EFAULT);
 		}
 
 		rc = ll_ioc_copy_start(inode->i_sb, copy);
-		if (copy_to_user((char __user *)arg, copy, sizeof(*copy)))
+		if (copy_to_user(uarg, copy, sizeof(*copy)))
 			rc = -EFAULT;
 
 		OBD_FREE_PTR(copy);
 		RETURN(rc);
 	}
 	case LL_IOC_HSM_COPY_END: {
-		struct hsm_copy	*copy;
-		int		 rc;
+		struct hsm_copy *copy;
+		int rc;
 
 		OBD_ALLOC_PTR(copy);
 		if (copy == NULL)
 			RETURN(-ENOMEM);
-		if (copy_from_user(copy, (char __user *)arg, sizeof(*copy))) {
+		if (copy_from_user(copy, uarg, sizeof(*copy))) {
 			OBD_FREE_PTR(copy);
 			RETURN(-EFAULT);
 		}
 
 		rc = ll_ioc_copy_end(inode->i_sb, copy);
-		if (copy_to_user((char __user *)arg, copy, sizeof(*copy)))
+		if (copy_to_user(uarg, copy, sizeof(*copy)))
 			rc = -EFAULT;
 
 		OBD_FREE_PTR(copy);
@@ -2297,7 +2288,7 @@
 		__u32 flags;
 		int rc;
 
-		rc = obd_ioctl_getdata(&data, &len, (void __user *)arg);
+		rc = obd_ioctl_getdata(&data, &len, uarg);
 		if (rc)
 			RETURN(rc);
 
@@ -2330,11 +2321,11 @@
 		RETURN(rc);
 	}
 	case FS_IOC_FSGETXATTR:
-		RETURN(ll_ioctl_fsgetxattr(inode, cmd, arg));
+		RETURN(ll_ioctl_fsgetxattr(inode, cmd, uarg));
 	case FS_IOC_FSSETXATTR:
-		RETURN(ll_ioctl_fssetxattr(inode, cmd, arg));
+		RETURN(ll_ioctl_fssetxattr(inode, cmd, uarg));
 	case LL_IOC_PROJECT:
-		RETURN(ll_ioctl_project(file, cmd, arg));
+		RETURN(ll_ioctl_project(file, cmd, uarg));
 	case LL_IOC_PCC_DETACH_BY_FID: {
 		struct lu_pcc_detach_fid *detach;
 		struct lu_fid *fid;
@@ -2345,9 +2336,7 @@
 		if (detach == NULL)
 			RETURN(-ENOMEM);
 
-		if (copy_from_user(detach,
-				   (const struct lu_pcc_detach_fid __user *)arg,
-				   sizeof(*detach)))
+		if (copy_from_user(detach, uarg, sizeof(*detach)))
 			GOTO(out_detach, rc = -EFAULT);
 
 		fid = &detach->pccd_fid;
@@ -2376,15 +2365,15 @@
 	case LL_IOC_SET_ENCRYPTION_POLICY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_set_policy(file, (const void __user *)arg);
+		return llcrypt_ioctl_set_policy(file, uarg);
 	case LL_IOC_GET_ENCRYPTION_POLICY_EX:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_get_policy_ex(file, (void __user *)arg);
+		return llcrypt_ioctl_get_policy_ex(file, uarg);
 	case LL_IOC_ADD_ENCRYPTION_KEY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		rc = llcrypt_ioctl_add_key(file, (void __user *)arg);
+		rc = llcrypt_ioctl_add_key(file, uarg);
 #ifdef CONFIG_LL_ENCRYPTION
 		if (!rc)
 			sptlrpc_enc_pool_add_user();
@@ -2393,20 +2382,18 @@
 	case LL_IOC_REMOVE_ENCRYPTION_KEY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_remove_key(file, (void __user *)arg);
+		return llcrypt_ioctl_remove_key(file, uarg);
 	case LL_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_remove_key_all_users(file,
-							  (void __user *)arg);
+		return llcrypt_ioctl_remove_key_all_users(file, uarg);
 	case LL_IOC_GET_ENCRYPTION_KEY_STATUS:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_get_key_status(file, (void __user *)arg);
+		return llcrypt_ioctl_get_key_status(file, uarg);
 #endif
 	default:
-		RETURN(obd_iocontrol(cmd, sbi->ll_dt_exp, 0, NULL,
-				     (void __user *)arg));
+		RETURN(obd_iocontrol(cmd, sbi->ll_dt_exp, 0, NULL, uarg));
 	}
 }
 
diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index e254b0b..b4df76b 100644
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -1376,7 +1376,7 @@
  * After lease is taken, send the RPC MDS_REINT_RESYNC to the MDT
  */
 static int ll_lease_file_resync(struct obd_client_handle *och,
-				struct inode *inode, unsigned long arg)
+				struct inode *inode, void __user *uarg)
 {
 	struct ll_sb_info *sbi = ll_i2sbi(inode);
 	struct md_op_data *op_data;
@@ -1390,8 +1390,7 @@
 	if (IS_ERR(op_data))
 		RETURN(PTR_ERR(op_data));
 
-	if (copy_from_user(&ioc, (struct ll_ioc_lease_id __user *)arg,
-			   sizeof(ioc)))
+	if (copy_from_user(&ioc, uarg, sizeof(ioc)))
 		RETURN(-EFAULT);
 
 	/* before starting file resync, it's necessary to clean up page cache
@@ -2579,11 +2578,11 @@
 static int ll_lov_setea(struct inode *inode, struct file *file,
 			void __user *arg)
 {
-	__u64			 flags = MDS_OPEN_HAS_OBJS | FMODE_WRITE;
-	struct lov_user_md	*lump;
-	int			 lum_size = sizeof(struct lov_user_md) +
-					    sizeof(struct lov_user_ost_data);
-	int			 rc;
+	__u64 flags = MDS_OPEN_HAS_OBJS | FMODE_WRITE;
+	struct lov_user_md *lump;
+	int lum_size = sizeof(struct lov_user_md) +
+		       sizeof(struct lov_user_ost_data);
+	int rc;
 	ENTRY;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -2591,7 +2590,7 @@
 
 	OBD_ALLOC_LARGE(lump, lum_size);
 	if (lump == NULL)
-                RETURN(-ENOMEM);
+		RETURN(-ENOMEM);
 
 	if (copy_from_user(lump, arg, lum_size))
 		GOTO(out_lump, rc = -EFAULT);
@@ -2607,9 +2606,9 @@
 
 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;
 
 	env = cl_env_get(&refcheck);
@@ -2624,10 +2623,10 @@
 static int ll_lov_setstripe(struct inode *inode, struct file *file,
 			    void __user *arg)
 {
-	struct lov_user_md __user *lum = (struct lov_user_md __user *)arg;
-	struct lov_user_md	  *klum;
-	int			   lum_size, rc;
-	__u64			   flags = FMODE_WRITE;
+	struct lov_user_md __user *lum = arg;
+	struct lov_user_md *klum;
+	int lum_size, rc;
+	__u64 flags = FMODE_WRITE;
 	ENTRY;
 
 	rc = ll_copy_user_md(lum, &klum);
@@ -2686,8 +2685,9 @@
 	if (file->f_flags & O_NONBLOCK) {
 		if (!mutex_trylock(&lli->lli_group_mutex))
 			RETURN(-EAGAIN);
-	} else
+	} else {
 		mutex_lock(&lli->lli_group_mutex);
+	}
 
 	if (fd->fd_flags & LL_FILE_GROUP_LOCKED) {
 		CWARN("group lock already existed with gid %lu\n",
@@ -3729,21 +3729,18 @@
 }
 
 int ll_ioctl_fsgetxattr(struct inode *inode, unsigned int cmd,
-			unsigned long arg)
+			void __user *uarg)
 {
 	struct fsxattr fsxattr;
 
-	if (copy_from_user(&fsxattr,
-			   (const struct fsxattr __user *)arg,
-			   sizeof(fsxattr)))
+	if (copy_from_user(&fsxattr, uarg, sizeof(fsxattr)))
 		RETURN(-EFAULT);
 
 	fsxattr.fsx_xflags = ll_inode_flags_to_xflags(inode->i_flags);
 	if (test_bit(LLIF_PROJECT_INHERIT, &ll_i2info(inode)->lli_flags))
 		fsxattr.fsx_xflags |= FS_XFLAG_PROJINHERIT;
 	fsxattr.fsx_projid = ll_i2info(inode)->lli_projid;
-	if (copy_to_user((struct fsxattr __user *)arg,
-			 &fsxattr, sizeof(fsxattr)))
+	if (copy_to_user(uarg, &fsxattr, sizeof(fsxattr)))
 		RETURN(-EFAULT);
 
 	RETURN(0);
@@ -3834,23 +3831,20 @@
 }
 
 int ll_ioctl_fssetxattr(struct inode *inode, unsigned int cmd,
-			unsigned long arg)
+			void __user *uarg)
 {
 	struct fsxattr fsxattr;
 
 	ENTRY;
 
-	if (copy_from_user(&fsxattr,
-			   (const struct fsxattr __user *)arg,
-			   sizeof(fsxattr)))
+	if (copy_from_user(&fsxattr, uarg, sizeof(fsxattr)))
 		RETURN(-EFAULT);
 
 	RETURN(ll_set_project(inode, fsxattr.fsx_xflags,
 			      fsxattr.fsx_projid));
 }
 
-int ll_ioctl_project(struct file *file, unsigned int cmd,
-		     unsigned long arg)
+int ll_ioctl_project(struct file *file, unsigned int cmd, void __user *uarg)
 {
 	struct lu_project lu_project;
 	struct dentry *dentry = file_dentry(file);
@@ -3858,9 +3852,7 @@
 	struct dentry *child_dentry = NULL;
 	int rc = 0, name_len;
 
-	if (copy_from_user(&lu_project,
-			   (const struct lu_project __user *)arg,
-			   sizeof(lu_project)))
+	if (copy_from_user(&lu_project, uarg, sizeof(lu_project)))
 		RETURN(-EFAULT);
 
 	/* apply child dentry if name is valid */
@@ -3896,8 +3888,7 @@
 			     &ll_i2info(inode)->lli_flags))
 			lu_project.project_xflags |= FS_XFLAG_PROJINHERIT;
 		lu_project.project_id = ll_i2info(inode)->lli_projid;
-		if (copy_to_user((struct lu_project __user *)arg,
-				 &lu_project, sizeof(lu_project))) {
+		if (copy_to_user(uarg, &lu_project, sizeof(lu_project))) {
 			rc = -EFAULT;
 			goto out;
 		}
@@ -3913,7 +3904,7 @@
 }
 
 static long ll_file_unlock_lease(struct file *file, struct ll_ioc_lease *ioc,
-				 unsigned long arg)
+				 void __user *uarg)
 {
 	struct inode		*inode = file_inode(file);
 	struct ll_file_data	*fd = file->private_data;
@@ -3955,7 +3946,7 @@
 		if (!data)
 			GOTO(out_lease_close, rc = -ENOMEM);
 
-		if (copy_from_user(data, (void __user *)arg, data_size))
+		if (copy_from_user(data, uarg, data_size))
 			GOTO(out_lease_close, rc = -EFAULT);
 
 		bias = MDS_CLOSE_RESYNC_DONE;
@@ -3965,8 +3956,8 @@
 		if (ioc->lil_count != 1)
 			GOTO(out_lease_close, rc = -EINVAL);
 
-		arg += sizeof(*ioc);
-		if (copy_from_user(&fdv, (void __user *)arg, sizeof(__u32)))
+		uarg += sizeof(*ioc);
+		if (copy_from_user(&fdv, uarg, sizeof(__u32)))
 			GOTO(out_lease_close, rc = -EFAULT);
 
 		layout_file = fget(fdv);
@@ -3987,13 +3978,12 @@
 		if (ioc->lil_count != 2)
 			GOTO(out_lease_close, rc = -EINVAL);
 
-		arg += sizeof(*ioc);
-		if (copy_from_user(&fdv, (void __user *)arg, sizeof(__u32)))
+		uarg += sizeof(*ioc);
+		if (copy_from_user(&fdv, uarg, sizeof(__u32)))
 			GOTO(out_lease_close, rc = -EFAULT);
 
-		arg += sizeof(__u32);
-		if (copy_from_user(&mirror_id, (void __user *)arg,
-				   sizeof(__u32)))
+		uarg += sizeof(__u32);
+		if (copy_from_user(&mirror_id, uarg, sizeof(__u32)))
 			GOTO(out_lease_close, rc = -EFAULT);
 
 		layout_file = fget(fdv);
@@ -4014,9 +4004,8 @@
 		if (IS_ENCRYPTED(inode))
 			RETURN(-EOPNOTSUPP);
 
-		arg += sizeof(*ioc);
-		if (copy_from_user(&param.pa_archive_id, (void __user *)arg,
-				   sizeof(__u32)))
+		uarg += sizeof(*ioc);
+		if (copy_from_user(&param.pa_archive_id, uarg, sizeof(__u32)))
 			GOTO(out_lease_close, rc2 = -EFAULT);
 
 		rc2 = pcc_readwrite_attach(file, inode, param.pa_archive_id);
@@ -4075,7 +4064,7 @@
 }
 
 static long ll_file_set_lease(struct file *file, struct ll_ioc_lease *ioc,
-			      unsigned long arg)
+			      void __user *uarg)
 {
 	struct inode *inode = file_inode(file);
 	struct ll_inode_info *lli = ll_i2info(inode);
@@ -4099,7 +4088,7 @@
 		fmode = FMODE_READ;
 		break;
 	case LL_LEASE_UNLCK:
-		RETURN(ll_file_unlock_lease(file, ioc, arg));
+		RETURN(ll_file_unlock_lease(file, ioc, uarg));
 	default:
 		RETURN(-EINVAL);
 	}
@@ -4114,7 +4103,7 @@
 		RETURN(PTR_ERR(och));
 
 	if (ioc->lil_flags & LL_LEASE_RESYNC) {
-		rc = ll_lease_file_resync(och, inode, arg);
+		rc = ll_lease_file_resync(och, inode, uarg);
 		if (rc) {
 			ll_lease_close(och, inode, NULL);
 			RETURN(rc);
@@ -4179,13 +4168,14 @@
 static long
 ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct inode		*inode = file_inode(file);
-	struct ll_file_data	*fd = file->private_data;
-	int			 flags, rc;
+	struct inode *inode = file_inode(file);
+	struct ll_file_data *fd = file->private_data;
+	void __user *uarg = (void __user *)arg;
+	int flags, rc;
 	ENTRY;
 
-	CDEBUG(D_VFSTRACE, "VFS Op:inode="DFID"(%p), cmd=%x\n",
-	       PFID(ll_inode2fid(inode)), inode, cmd);
+	CDEBUG(D_VFSTRACE|D_IOCTL, "VFS Op:inode="DFID"(%pK) cmd=%x arg=%lx\n",
+	       PFID(ll_inode2fid(inode)), inode, cmd, arg);
 	ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_IOCTL, 1);
 
 	/* asm-ppc{,64} declares TCGETS, et. al. as type 't' not 'T' */
@@ -4196,39 +4186,40 @@
 	case LL_IOC_GETFLAGS:
 		/* Get the current value of the file flags */
 		return put_user(fd->fd_flags, (int __user *)arg);
-        case LL_IOC_SETFLAGS:
-        case LL_IOC_CLRFLAGS:
-                /* Set or clear specific file flags */
-                /* XXX This probably needs checks to ensure the flags are
-                 *     not abused, and to handle any flag side effects.
-                 */
-		if (get_user(flags, (int __user *) arg))
-                        RETURN(-EFAULT);
+	case LL_IOC_SETFLAGS:
+	case LL_IOC_CLRFLAGS:
+		/* Set or clear specific file flags */
+		/* XXX This probably needs checks to ensure the flags are
+		 *     not abused, and to handle any flag side effects.
+		 */
+		if (get_user(flags, (int __user *)arg))
+			RETURN(-EFAULT);
 
-                if (cmd == LL_IOC_SETFLAGS) {
-                        if ((flags & LL_FILE_IGNORE_LOCK) &&
-                            !(file->f_flags & O_DIRECT)) {
-                                CERROR("%s: unable to disable locking on "
-                                       "non-O_DIRECT file\n", current->comm);
-                                RETURN(-EINVAL);
-                        }
+		if (cmd == LL_IOC_SETFLAGS) {
+			if ((flags & LL_FILE_IGNORE_LOCK) &&
+			    !(file->f_flags & O_DIRECT)) {
+				rc = -EINVAL;
+				CERROR("%s: unable to disable locking on non-O_DIRECT file "DFID": rc = %d\n",
+				       current->comm, PFID(ll_inode2fid(inode)),
+				       rc);
+				RETURN(rc);
+			}
 
-                        fd->fd_flags |= flags;
-                } else {
-                        fd->fd_flags &= ~flags;
-                }
-                RETURN(0);
+			fd->fd_flags |= flags;
+		} else {
+			fd->fd_flags &= ~flags;
+		}
+		RETURN(0);
 	case LL_IOC_LOV_SETSTRIPE:
 	case LL_IOC_LOV_SETSTRIPE_NEW:
-		RETURN(ll_lov_setstripe(inode, file, (void __user *)arg));
+		RETURN(ll_lov_setstripe(inode, file, uarg));
 	case LL_IOC_LOV_SETEA:
-		RETURN(ll_lov_setea(inode, file, (void __user *)arg));
+		RETURN(ll_lov_setea(inode, file, uarg));
 	case LL_IOC_LOV_SWAP_LAYOUTS: {
 		struct file *file2;
 		struct lustre_swap_layouts lsl;
 
-		if (copy_from_user(&lsl, (char __user *)arg,
-				   sizeof(struct lustre_swap_layouts)))
+		if (copy_from_user(&lsl, uarg, sizeof(lsl)))
 			RETURN(-EFAULT);
 
 		if ((file->f_flags & O_ACCMODE) == O_RDONLY)
@@ -4243,9 +4234,9 @@
 			GOTO(out, rc = -EPERM);
 
 		if (lsl.sl_flags & SWAP_LAYOUTS_CLOSE) {
-			struct inode			*inode2;
-			struct ll_inode_info		*lli;
-			struct obd_client_handle	*och = NULL;
+			struct obd_client_handle *och = NULL;
+			struct ll_inode_info *lli;
+			struct inode *inode2;
 
 			lli = ll_i2info(inode);
 			mutex_lock(&lli->lli_och_mutex);
@@ -4267,10 +4258,10 @@
 	}
 	case LL_IOC_LOV_GETSTRIPE:
 	case LL_IOC_LOV_GETSTRIPE_NEW:
-		RETURN(ll_file_getstripe(inode, (void __user *)arg, 0));
+		RETURN(ll_file_getstripe(inode, uarg, 0));
 	case FS_IOC_GETFLAGS:
 	case FS_IOC_SETFLAGS:
-		RETURN(ll_iocontrol(inode, file, cmd, arg));
+		RETURN(ll_iocontrol(inode, file, cmd, uarg));
 	case FSFILT_IOC_GETVERSION:
 	case FS_IOC_GETVERSION:
 		RETURN(put_user(inode->i_generation, (int __user *)arg));
@@ -4279,65 +4270,67 @@
 	 * network encode the arg field. */
 	case FS_IOC_SETVERSION:
 		RETURN(-ENOTSUPP);
-
-        case LL_IOC_GROUP_LOCK:
-                RETURN(ll_get_grouplock(inode, file, arg));
-        case LL_IOC_GROUP_UNLOCK:
-                RETURN(ll_put_grouplock(inode, file, arg));
-        case IOC_OBD_STATFS:
-		RETURN(ll_obd_statfs(inode, (void __user *)arg));
-
+	case LL_IOC_GROUP_LOCK:
+		RETURN(ll_get_grouplock(inode, file, arg));
+	case LL_IOC_GROUP_UNLOCK:
+		RETURN(ll_put_grouplock(inode, file, arg));
+	case IOC_OBD_STATFS:
+		RETURN(ll_obd_statfs(inode, uarg));
 	case LL_IOC_FLUSHCTX:
 		RETURN(ll_flush_ctx(inode));
 	case LL_IOC_PATH2FID: {
-		if (copy_to_user((void __user *)arg, ll_inode2fid(inode),
+		if (copy_to_user(uarg, ll_inode2fid(inode),
 				 sizeof(struct lu_fid)))
 			RETURN(-EFAULT);
 
 		RETURN(0);
 	}
 	case LL_IOC_GETPARENT:
-		RETURN(ll_getparent(file, (struct getparent __user *)arg));
-
+		RETURN(ll_getparent(file, uarg));
 	case OBD_IOC_FID2PATH:
-		RETURN(ll_fid2path(inode, (void __user *)arg));
+		RETURN(ll_fid2path(inode, uarg));
 	case LL_IOC_DATA_VERSION: {
-		struct ioc_data_version	idv;
+		struct ioc_data_version idv;
 		int rc;
 
-		if (copy_from_user(&idv, (char __user *)arg, sizeof(idv)))
+		if (copy_from_user(&idv, uarg, sizeof(idv)))
 			RETURN(-EFAULT);
 
 		idv.idv_flags &= LL_DV_RD_FLUSH | LL_DV_WR_FLUSH;
 		rc = ll_ioc_data_version(inode, &idv);
 
-		if (rc == 0 &&
-		    copy_to_user((char __user *)arg, &idv, sizeof(idv)))
+		if (rc == 0 && copy_to_user(uarg, &idv, sizeof(idv)))
 			RETURN(-EFAULT);
 
 		RETURN(rc);
 	}
 
-        case LL_IOC_GET_MDTIDX: {
-                int mdtidx;
+	case LL_IOC_GET_MDTIDX: {
+		int mdtidx;
 
-                mdtidx = ll_get_mdt_idx(inode);
-                if (mdtidx < 0)
-                        RETURN(mdtidx);
+		if (!ll_access_ok(uarg, sizeof(mdtidx)))
+			RETURN(-EFAULT);
 
-		if (put_user((int)mdtidx, (int __user *)arg))
-                        RETURN(-EFAULT);
+		mdtidx = ll_get_mdt_idx(inode);
+		if (mdtidx < 0)
+			RETURN(mdtidx);
 
-                RETURN(0);
-        }
+		if (put_user((int)mdtidx, (int __user *)uarg))
+			RETURN(-EFAULT);
+
+		RETURN(0);
+	}
 	case OBD_IOC_GETNAME_OLD:
 	case OBD_IOC_GETDTNAME:
 	case OBD_IOC_GETMDNAME:
-		RETURN(ll_get_obd_name(inode, cmd, arg));
+		RETURN(ll_get_obd_name(inode, cmd, uarg));
 	case LL_IOC_HSM_STATE_GET: {
-		struct md_op_data	*op_data;
-		struct hsm_user_state	*hus;
-		int			 rc;
+		struct md_op_data *op_data;
+		struct hsm_user_state *hus;
+		int rc;
+
+		if (!ll_access_ok(uarg, sizeof(*hus)))
+			RETURN(-EFAULT);
 
 		OBD_ALLOC_PTR(hus);
 		if (hus == NULL)
@@ -4353,7 +4346,7 @@
 		rc = obd_iocontrol(cmd, ll_i2mdexp(inode), sizeof(*op_data),
 				   op_data, NULL);
 
-		if (copy_to_user((void __user *)arg, hus, sizeof(*hus)))
+		if (copy_to_user(uarg, hus, sizeof(*hus)))
 			rc = -EFAULT;
 
 		ll_finish_md_op_data(op_data);
@@ -4361,14 +4354,14 @@
 		RETURN(rc);
 	}
 	case LL_IOC_HSM_STATE_SET: {
-		struct hsm_state_set	*hss;
-		int			 rc;
+		struct hsm_state_set *hss;
+		int rc;
 
 		OBD_ALLOC_PTR(hss);
 		if (hss == NULL)
 			RETURN(-ENOMEM);
 
-		if (copy_from_user(hss, (char __user *)arg, sizeof(*hss))) {
+		if (copy_from_user(hss, uarg, sizeof(*hss))) {
 			OBD_FREE_PTR(hss);
 			RETURN(-EFAULT);
 		}
@@ -4384,6 +4377,9 @@
 		const char *action;
 		int rc;
 
+		if (!ll_access_ok(uarg, sizeof(*hca)))
+			RETURN(-EFAULT);
+
 		OBD_ALLOC_PTR(hca);
 		if (hca == NULL)
 			RETURN(-ENOMEM);
@@ -4414,7 +4410,7 @@
 			       hca->hca_location.offset, hca->hca_location.length);
 		}
 
-		if (copy_to_user((char __user *)arg, hca, sizeof(*hca)))
+		if (copy_to_user(uarg, hca, sizeof(*hca)))
 			rc = -EFAULT;
 skip_copy:
 		ll_finish_md_op_data(op_data);
@@ -4422,17 +4418,17 @@
 		RETURN(rc);
 	}
 	case LL_IOC_SET_LEASE_OLD: {
-		struct ll_ioc_lease ioc = { .lil_mode = (__u32)arg };
+		struct ll_ioc_lease ioc = { .lil_mode = arg };
 
 		RETURN(ll_file_set_lease(file, &ioc, 0));
 	}
 	case LL_IOC_SET_LEASE: {
 		struct ll_ioc_lease ioc;
 
-		if (copy_from_user(&ioc, (void __user *)arg, sizeof(ioc)))
+		if (copy_from_user(&ioc, uarg, sizeof(ioc)))
 			RETURN(-EFAULT);
 
-		RETURN(ll_file_set_lease(file, &ioc, arg));
+		RETURN(ll_file_set_lease(file, &ioc, uarg));
 	}
 	case LL_IOC_GET_LEASE: {
 		struct ll_inode_info *lli = ll_i2info(inode);
@@ -4464,7 +4460,7 @@
 		if (hui == NULL)
 			RETURN(-ENOMEM);
 
-		if (copy_from_user(hui, (void __user *)arg, sizeof(*hui))) {
+		if (copy_from_user(hui, uarg, sizeof(*hui))) {
 			OBD_FREE_PTR(hui);
 			RETURN(-EFAULT);
 		}
@@ -4477,9 +4473,7 @@
 	case LL_IOC_FUTIMES_3: {
 		struct ll_futimes_3 lfu;
 
-		if (copy_from_user(&lfu,
-				   (const struct ll_futimes_3 __user *)arg,
-				   sizeof(lfu)))
+		if (copy_from_user(&lfu, uarg, sizeof(lfu)))
 			RETURN(-EFAULT);
 
 		RETURN(ll_file_futimes_3(file, &lfu));
@@ -4492,7 +4486,7 @@
 		int alloc_size = sizeof(*k_ladvise_hdr);
 
 		rc = 0;
-		u_ladvise_hdr = (void __user *)arg;
+		u_ladvise_hdr = uarg;
 		OBD_ALLOC_PTR(k_ladvise_hdr);
 		if (k_ladvise_hdr == NULL)
 			RETURN(-ENOMEM);
@@ -4568,15 +4562,15 @@
 		if (!(file->f_flags & O_DIRECT))
 			RETURN(-EINVAL);
 
-		fd->fd_designated_mirror = (__u32)arg;
+		fd->fd_designated_mirror = arg;
 		RETURN(0);
 	}
 	case FS_IOC_FSGETXATTR:
-		RETURN(ll_ioctl_fsgetxattr(inode, cmd, arg));
+		RETURN(ll_ioctl_fsgetxattr(inode, cmd, uarg));
 	case FS_IOC_FSSETXATTR:
-		RETURN(ll_ioctl_fssetxattr(inode, cmd, arg));
+		RETURN(ll_ioctl_fssetxattr(inode, cmd, uarg));
 	case LL_IOC_PROJECT:
-		RETURN(ll_ioctl_project(file, cmd, arg));
+		RETURN(ll_ioctl_project(file, cmd, uarg));
 	case BLKSSZGET:
 		RETURN(put_user(PAGE_SIZE, (int __user *)arg));
 	case LL_IOC_HEAT_GET: {
@@ -4584,7 +4578,7 @@
 		struct lu_heat *heat;
 		int size;
 
-		if (copy_from_user(&uheat, (void __user *)arg, sizeof(uheat)))
+		if (copy_from_user(&uheat, uarg, sizeof(uheat)))
 			RETURN(-EFAULT);
 
 		if (uheat.lh_count > OBD_HEAT_COUNT)
@@ -4597,14 +4591,14 @@
 
 		heat->lh_count = uheat.lh_count;
 		ll_heat_get(inode, heat);
-		rc = copy_to_user((char __user *)arg, heat, size);
+		rc = copy_to_user(uarg, heat, size);
 		OBD_FREE(heat, size);
 		RETURN(rc ? -EFAULT : 0);
 	}
 	case LL_IOC_HEAT_SET: {
 		__u64 flags;
 
-		if (copy_from_user(&flags, (void __user *)arg, sizeof(flags)))
+		if (copy_from_user(&flags, uarg, sizeof(flags)))
 			RETURN(-EFAULT);
 
 		rc = ll_heat_set(inode, flags);
@@ -4617,9 +4611,7 @@
 		if (detach == NULL)
 			RETURN(-ENOMEM);
 
-		if (copy_from_user(detach,
-				   (const struct lu_pcc_detach __user *)arg,
-				   sizeof(*detach)))
+		if (copy_from_user(detach, uarg, sizeof(*detach)))
 			GOTO(out_detach_free, rc = -EFAULT);
 
 		if (!S_ISREG(inode->i_mode))
@@ -4634,8 +4626,7 @@
 		RETURN(rc);
 	}
 	case LL_IOC_PCC_STATE: {
-		struct lu_pcc_state __user *ustate =
-			(struct lu_pcc_state __user *)arg;
+		struct lu_pcc_state __user *ustate = uarg;
 		struct lu_pcc_state *state;
 
 		OBD_ALLOC_PTR(state);
@@ -4660,28 +4651,27 @@
 	case LL_IOC_SET_ENCRYPTION_POLICY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_set_policy(file, (const void __user *)arg);
+		return llcrypt_ioctl_set_policy(file, uarg);
 	case LL_IOC_GET_ENCRYPTION_POLICY_EX:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_get_policy_ex(file, (void __user *)arg);
+		return llcrypt_ioctl_get_policy_ex(file, uarg);
 	case LL_IOC_ADD_ENCRYPTION_KEY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_add_key(file, (void __user *)arg);
+		return llcrypt_ioctl_add_key(file, uarg);
 	case LL_IOC_REMOVE_ENCRYPTION_KEY:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_remove_key(file, (void __user *)arg);
+		return llcrypt_ioctl_remove_key(file, uarg);
 	case LL_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_remove_key_all_users(file,
-							  (void __user *)arg);
+		return llcrypt_ioctl_remove_key_all_users(file, uarg);
 	case LL_IOC_GET_ENCRYPTION_KEY_STATUS:
 		if (!ll_sbi_has_encrypt(ll_i2sbi(inode)))
 			return -EOPNOTSUPP;
-		return llcrypt_ioctl_get_key_status(file, (void __user *)arg);
+		return llcrypt_ioctl_get_key_status(file, uarg);
 #endif
 
 	case LL_IOC_UNLOCK_FOREIGN: {
@@ -4698,8 +4688,7 @@
 	}
 
 	default:
-		RETURN(obd_iocontrol(cmd, ll_i2dtexp(inode), 0, NULL,
-				     (void __user *)arg));
+		RETURN(obd_iocontrol(cmd, ll_i2dtexp(inode), 0, NULL, uarg));
 	}
 }
 
diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index 89edc09..01345cd 100644
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -1229,11 +1229,11 @@
 			int mask);
 int ll_ioctl_check_project(struct inode *inode, __u32 xflags, __u32 projid);
 int ll_ioctl_fsgetxattr(struct inode *inode, unsigned int cmd,
-			unsigned long arg);
+			void __user *uarg);
 int ll_ioctl_fssetxattr(struct inode *inode, unsigned int cmd,
-			unsigned long arg);
+			void __user *uarg);
 int ll_ioctl_project(struct file *file, unsigned int cmd,
-		     unsigned long arg);
+		     void __user *uarg);
 
 int ll_lov_setstripe_ea_info(struct inode *inode, struct dentry *dentry,
 			     __u64 flags, struct lov_user_md *lum,
@@ -1309,7 +1309,7 @@
 void ll_truncate_inode_pages_final(struct inode *inode);
 void ll_delete_inode(struct inode *inode);
 int ll_iocontrol(struct inode *inode, struct file *file,
-                 unsigned int cmd, unsigned long arg);
+		 unsigned int cmd, void __user *uarg);
 int ll_flush_ctx(struct inode *inode);
 void ll_umount_begin(struct super_block *sb);
 int ll_remount_fs(struct super_block *sb, int *flags, char *data);
@@ -1329,7 +1329,7 @@
 				      __u32 mode, enum md_op_code opc,
 				      void *data);
 void ll_finish_md_op_data(struct md_op_data *op_data);
-int ll_get_obd_name(struct inode *inode, unsigned int cmd, unsigned long arg);
+int ll_get_obd_name(struct inode *inode, unsigned int cmd, void __user *arg);
 void ll_compute_rootsquash_state(struct ll_sb_info *sbi);
 ssize_t ll_copy_user_md(const struct lov_user_md __user *md,
 			struct lov_user_md **kbuf);
diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 054661e..90dd377 100644
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -2970,7 +2970,7 @@
 }
 
 int ll_iocontrol(struct inode *inode, struct file *file,
-                 unsigned int cmd, unsigned long arg)
+		 unsigned int cmd, void __user *uarg)
 {
 	struct ll_sb_info *sbi = ll_i2sbi(inode);
 	struct ptlrpc_request *req = NULL;
@@ -2979,26 +2979,28 @@
 
 	switch (cmd) {
 	case FS_IOC_GETFLAGS: {
-                struct mdt_body *body;
-                struct md_op_data *op_data;
+		struct mdt_body *body;
+		struct md_op_data *op_data;
 
-                op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL,
-                                             0, 0, LUSTRE_OPC_ANY,
-                                             NULL);
-                if (IS_ERR(op_data))
-                        RETURN(PTR_ERR(op_data));
+		if (!ll_access_ok(uarg, sizeof(int)))
+			RETURN(-EFAULT);
 
-                op_data->op_valid = OBD_MD_FLFLAGS;
-                rc = md_getattr(sbi->ll_md_exp, op_data, &req);
-                ll_finish_md_op_data(op_data);
-                if (rc) {
+		op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0,
+					     LUSTRE_OPC_ANY, NULL);
+		if (IS_ERR(op_data))
+			RETURN(PTR_ERR(op_data));
+
+		op_data->op_valid = OBD_MD_FLFLAGS;
+		rc = md_getattr(sbi->ll_md_exp, op_data, &req);
+		ll_finish_md_op_data(op_data);
+		if (rc) {
 			CERROR("%s: failure inode "DFID": rc = %d\n",
 			       sbi->ll_md_exp->exp_obd->obd_name,
 			       PFID(ll_inode2fid(inode)), rc);
-                        RETURN(-abs(rc));
-                }
+			RETURN(-abs(rc));
+		}
 
-                body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
+		body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
 
 		flags = body->mbo_flags;
 		/* if Lustre specific LUSTRE_ENCRYPT_FL flag is set, also set
@@ -3009,7 +3011,7 @@
 
 		ptlrpc_req_finished(req);
 
-		RETURN(put_user(flags, (int __user *)arg));
+		RETURN(put_user(flags, (int __user *)uarg));
 	}
 	case FS_IOC_SETFLAGS: {
 		struct iattr *attr;
@@ -3017,7 +3019,7 @@
 		struct cl_object *obj;
 		struct fsxattr fa = { 0 };
 
-		if (get_user(flags, (int __user *)arg))
+		if (get_user(flags, (int __user *)uarg))
 			RETURN(-EFAULT);
 
 		fa.fsx_projid = ll_i2info(inode)->lli_projid;
@@ -3037,8 +3039,8 @@
 		op_data->op_attr_flags = flags;
 		op_data->op_xvalid |= OP_XVALID_FLAGS;
 		rc = md_setattr(sbi->ll_md_exp, op_data, NULL, 0, &req);
-                ll_finish_md_op_data(op_data);
-                ptlrpc_req_finished(req);
+		ll_finish_md_op_data(op_data);
+		ptlrpc_req_finished(req);
 		if (rc)
 			RETURN(rc);
 
@@ -3056,12 +3058,12 @@
 
 		OBD_FREE_PTR(attr);
 		RETURN(rc);
-        }
-        default:
-                RETURN(-ENOSYS);
-        }
+	}
+	default:
+		RETURN(-ENOTTY);
+	}
 
-        RETURN(0);
+	RETURN(0);
 }
 
 int ll_flush_ctx(struct inode *inode)
@@ -3361,7 +3363,7 @@
 	return rc;
 }
 
-int ll_obd_statfs(struct inode *inode, void __user *arg)
+int ll_obd_statfs(struct inode *inode, void __user *uarg)
 {
 	struct ll_sb_info *sbi = NULL;
 	struct obd_export *exp;
@@ -3374,7 +3376,7 @@
 	if (!sbi)
 		GOTO(out_statfs, rc = -EINVAL);
 
-	rc = obd_ioctl_getdata(&data, &len, arg);
+	rc = obd_ioctl_getdata(&data, &len, uarg);
 	if (rc)
 		GOTO(out_statfs, rc);
 
@@ -3624,7 +3626,7 @@
 /**
  * Get obd name by cmd, and copy out to user space
  */
-int ll_get_obd_name(struct inode *inode, unsigned int cmd, unsigned long arg)
+int ll_get_obd_name(struct inode *inode, unsigned int cmd, void __user *uarg)
 {
         struct ll_sb_info *sbi = ll_i2sbi(inode);
         struct obd_device *obd;
@@ -3640,8 +3642,7 @@
         if (!obd)
                 RETURN(-ENOENT);
 
-	if (copy_to_user((void __user *)arg, obd->obd_name,
-			 strlen(obd->obd_name) + 1))
+	if (copy_to_user(uarg, obd->obd_name, strlen(obd->obd_name) + 1))
 		RETURN(-EFAULT);
 
 	RETURN(0);
diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c
index 945061f..91a35cf 100644
--- a/lustre/lmv/lmv_obd.c
+++ b/lustre/lmv/lmv_obd.c
@@ -851,10 +851,25 @@
 	int rc = 0;
 
 	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);
 
+	/* 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, karg, uarg);
+		RETURN(rc);
+	}
+	}
+
+	if (unlikely(karg == NULL))
+		RETURN(-EINVAL);
+
 	switch (cmd) {
 	case IOC_OBD_STATFS: {
 		struct obd_ioctl_data *data = karg;
@@ -944,13 +959,6 @@
 		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;
@@ -1082,15 +1090,16 @@
 			err = obd_iocontrol(cmd, tgt->ltd_exp, len, karg, uarg);
 			if (err) {
 				if (tgt->ltd_active) {
-					CERROR("error: iocontrol MDC %s on MDT"
-					       " idx %d cmd %x: err = %d\n",
+					CERROR("%s: iocontrol from '%s' on %s on cmd=%x: rc = %d\n",
+					       obd->obd_name, current->comm,
 					       tgt->ltd_uuid.uuid,
-					       tgt->ltd_index, cmd, err);
+					       cmd, err);
 					if (!rc)
 						rc = err;
 				}
-			} else
+			} else {
 				set = 1;
+			}
 		}
 		if (!set && !rc)
 			rc = -EIO;
diff --git a/lustre/lod/lod_dev.c b/lustre/lod/lod_dev.c
index f0c7075..f7e24b7 100644
--- a/lustre/lod/lod_dev.c
+++ b/lustre/lod/lod_dev.c
@@ -2774,9 +2774,9 @@
 		rc = lod_llog_cancel(&env, lod);
 		break;
 	default:
-		CERROR("%s: unrecognized ioctl %#x by %s\n",
-		       obd->obd_name, cmd, current->comm);
 		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 	}
 	lu_env_fini(&env);
 
diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c
index 04cc3ad..fbfe5c4 100644
--- a/lustre/lov/lov_obd.c
+++ b/lustre/lov/lov_obd.c
@@ -954,6 +954,10 @@
 	int i = 0, rc = 0, count = lov->desc.ld_tgt_count;
 
 	ENTRY;
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       exp->exp_obd->obd_name, cmd, len, karg, uarg);
+
+	/* handle cases that don't use @karg first */
 	switch (cmd) {
 	case IOC_OBD_STATFS: {
 		struct obd_ioctl_data *data = karg;
@@ -963,6 +967,9 @@
 		__u32 index;
 		__u32 flags;
 
+		if (!data)
+			RETURN(-EINVAL);
+
 		memcpy(&index, data->ioc_inlbuf2, sizeof(index));
 		if (index >= count)
 			RETURN(-ENODEV);
@@ -1007,6 +1014,9 @@
 		struct obd_quotactl *oqctl;
 		struct obd_import *imp;
 
+		if (!qctl)
+			RETURN(-EINVAL);
+
 		if (qctl->qc_valid == QC_OSTIDX) {
 			if (count <= qctl->qc_idx)
 				RETURN(-EINVAL);
@@ -1083,9 +1093,11 @@
 				if (lov->lov_tgts[i]->ltd_active) {
 					CDEBUG_LIMIT(err == -ENOTTY ?
 						     D_IOCTL : D_WARNING,
-						     "iocontrol OSC %s on OST idx %d cmd %x: err = %d\n",
+						     "%s: iocontrol from '%s' on %s cmd=%x: rc = %d\n",
+						     obd->obd_name,
+						     current->comm,
 						     lov_uuid2str(lov, i),
-						     i, cmd, err);
+						     cmd, err);
 					if (!rc)
 						rc = err;
 				}
diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c
index 0fcdb49..75e5f9d 100644
--- a/lustre/mdc/mdc_request.c
+++ b/lustre/mdc/mdc_request.c
@@ -2190,7 +2190,22 @@
 	struct obd_ioctl_data *data = karg;
 	struct obd_import *imp = obd->u.cli.cl_import;
 	int rc;
+
 	ENTRY;
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       exp->exp_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);
+		break;
+	}
+
+	if (!karg)
+		RETURN(-EINVAL);
 
 	if (!try_module_get(THIS_MODULE)) {
 		CERROR("%s: cannot get module '%s'\n", obd->obd_name,
@@ -2281,18 +2296,13 @@
 		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);
 	default:
-		CERROR("unrecognised ioctl: cmd = %#x\n", cmd);
-		GOTO(out, rc = -ENOTTY);
+		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 	}
 out:
 	module_put(THIS_MODULE);
diff --git a/lustre/mdd/mdd_device.c b/lustre/mdd/mdd_device.c
index 18cefa3..4cc2fe0 100644
--- a/lustre/mdd/mdd_device.c
+++ b/lustre/mdd/mdd_device.c
@@ -2272,7 +2272,12 @@
 	struct mdd_device *mdd = lu2mdd_dev(&m->md_lu_dev);
 	struct obd_ioctl_data *data = karg;
 	int rc;
+
 	ENTRY;
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK\n",
+	       mdd2obd_dev(mdd)->obd_name, cmd, len, karg);
+	if (unlikely(karg == NULL))
+		RETURN(-EINVAL);
 
 	/* Doesn't use obd_ioctl_data */
 	switch (cmd) {
@@ -2310,8 +2315,8 @@
 
 	/* Below ioctls use obd_ioctl_data */
 	if (data->ioc_version != OBD_IOCTL_VERSION) {
-		CERROR("Bad magic %x != %x\n", data->ioc_version,
-		       OBD_IOCTL_VERSION);
+		CERROR("%s: bad magic %x != %x\n", mdd2obd_dev(mdd)->obd_name,
+		       data->ioc_version, OBD_IOCTL_VERSION);
 		RETURN(-EINVAL);
 	}
 
@@ -2336,6 +2341,8 @@
 		break;
 	default:
 		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       mdd2obd_dev(mdd)->obd_name, current->comm, cmd, rc);
 	}
 
 	RETURN(rc);
diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c
index 01a3610..2b499ec 100644
--- a/lustre/mdt/mdt_handler.c
+++ b/lustre/mdt/mdt_handler.c
@@ -7498,20 +7498,30 @@
 	int rc;
 
 	ENTRY;
-	CDEBUG(D_IOCTL, "handling ioctl cmd %#x\n", cmd);
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       exp->exp_obd->obd_name, cmd, len, karg, uarg);
+	if (unlikely(uarg != NULL))
+		RETURN(-EINVAL);
+
 	rc = lu_env_init(&env, LCT_MD_THREAD);
 	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;
+		GOTO(out, rc);
+	}
+	if (unlikely(karg == NULL))
+		GOTO(out, rc = -EINVAL);
+
+	switch (cmd) {
 	case OBD_IOC_ABORT_RECOVERY: {
 		struct obd_ioctl_data *data = karg;
 
@@ -7544,11 +7554,6 @@
 		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);
@@ -7601,10 +7606,10 @@
 	 }
 	default:
 		rc = -EOPNOTSUPP;
-		CERROR("%s: Not supported cmd = %d, rc = %d\n",
-			mdt_obd_name(mdt), cmd, rc);
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       mdt_obd_name(mdt), current->comm, cmd, rc);
 	}
-
+out:
 	lu_env_fini(&env);
 	RETURN(rc);
 }
diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c
index f89d20e..9da94ff 100644
--- a/lustre/mgs/mgs_handler.c
+++ b/lustre/mgs/mgs_handler.c
@@ -985,80 +985,82 @@
 static int mgs_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 			 void *karg, void __user *uarg)
 {
+	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 = karg;
 	struct lu_env env;
-        int rc = 0;
+	int rc = 0;
 
-        ENTRY;
-        CDEBUG(D_IOCTL, "handling ioctl cmd %#x\n", cmd);
+	ENTRY;
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       obd->obd_name, cmd, len, karg, uarg);
+	if (unlikely(uarg != NULL || data == NULL))
+		RETURN(-EINVAL);
 
 	rc = lu_env_init(&env, LCT_MG_THREAD);
 	if (rc)
 		RETURN(rc);
 
-        switch (cmd) {
-
+	switch (cmd) {
 	case OBD_IOC_PARAM: {
 		struct lustre_cfg *lcfg;
 
 		if (data->ioc_type != LUSTRE_CFG_TYPE) {
 			CERROR("%s: unknown cfg record type: %d\n",
-			       mgs->mgs_obd->obd_name, data->ioc_type);
+			       obd->obd_name, data->ioc_type);
 			GOTO(out, rc = -EINVAL);
-                }
+		}
 
-                OBD_ALLOC(lcfg, data->ioc_plen1);
-                if (lcfg == NULL)
+		OBD_ALLOC(lcfg, data->ioc_plen1);
+		if (lcfg == NULL)
 			GOTO(out, rc = -ENOMEM);
 		if (copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1))
-                        GOTO(out_free, rc = -EFAULT);
+			GOTO(out_free, rc = -EFAULT);
 
-                if (lcfg->lcfg_bufcount < 1)
-                        GOTO(out_free, rc = -EINVAL);
+		if (lcfg->lcfg_bufcount < 1)
+			GOTO(out_free, rc = -EINVAL);
 
 		rc = mgs_set_param(&env, mgs, lcfg);
 		if (rc)
 			CERROR("%s: setparam err: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 out_free:
-                OBD_FREE(lcfg, data->ioc_plen1);
+		OBD_FREE(lcfg, data->ioc_plen1);
 		break;
-        }
-
+	}
 	case OBD_IOC_REPLACE_NIDS: {
 		if (!data->ioc_inllen1 || !data->ioc_inlbuf1) {
 			rc = -EINVAL;
 			CERROR("%s: no device or fsname specified: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (data->ioc_inllen1 > MTI_NAME_MAXLEN) {
 			rc = -EOVERFLOW;
 			CERROR("%s: device or fsname is too long: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (data->ioc_inlbuf1[data->ioc_inllen1 - 1] != 0) {
 			rc = -EINVAL;
-			CERROR("%s: device or fsname is not NUL terminated: "
-			       "rc = %d\n", exp->exp_obd->obd_name, rc);
+			CERROR("%s: unterminated device or fsname: rc = %d\n",
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (!data->ioc_inllen2 || !data->ioc_inlbuf2) {
 			rc = -EINVAL;
 			CERROR("%s: no NIDs specified: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (data->ioc_inlbuf2[data->ioc_inllen2 - 1] != 0) {
 			rc = -EINVAL;
-			CERROR("%s: NID list is not NUL terminated: "
-			       "rc = %d\n", exp->exp_obd->obd_name, rc);
+			CERROR("%s: NID list is not NUL terminated: rc = %d\n",
+			       obd->obd_name, rc);
 			break;
 		}
 
@@ -1066,8 +1068,8 @@
 		rc = mgs_replace_nids(&env, mgs, data->ioc_inlbuf1,
 				      data->ioc_inlbuf2);
 		if (rc)
-			CERROR("%s: error replacing nids: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			CERROR("%s: error replacing NIDs for '%s': rc = %d\n",
+			       obd->obd_name, data->ioc_inlbuf1, rc);
 
 		break;
 	}
@@ -1076,21 +1078,21 @@
 		if (!data->ioc_inllen1 || !data->ioc_inlbuf1) {
 			rc = -EINVAL;
 			CERROR("%s: no device or fsname specified: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (data->ioc_inllen1 > MTI_NAME_MAXLEN) {
 			rc = -EOVERFLOW;
 			CERROR("%s: device or fsname is too long: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 			break;
 		}
 
 		if (data->ioc_inlbuf1[data->ioc_inllen1 - 1] != 0) {
 			rc = -EINVAL;
-			CERROR("%s: device or fsname is not NUL terminated: "
-			       "rc = %d\n", exp->exp_obd->obd_name, rc);
+			CERROR("%s: unterminated device or fsname: rc = %d\n",
+			       obd->obd_name, rc);
 			break;
 		}
 
@@ -1098,7 +1100,7 @@
 		rc = mgs_clear_configs(&env, mgs, data->ioc_inlbuf1);
 		if (rc)
 			CERROR("%s: error clearing config log: rc = %d\n",
-			       exp->exp_obd->obd_name, rc);
+			       obd->obd_name, rc);
 
 		break;
 	}
@@ -1129,23 +1131,22 @@
 		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:
-		CERROR("%s: unknown command %#x\n",
-		       mgs->mgs_obd->obd_name,  cmd);
+	}
+	default:
 		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 		break;
-        }
+	}
 out:
 	lu_env_fini(&env);
 	RETURN(rc);
diff --git a/lustre/obdclass/class_obd.c b/lustre/obdclass/class_obd.c
index 9db9d2a..aa1716a 100644
--- a/lustre/obdclass/class_obd.c
+++ b/lustre/obdclass/class_obd.c
@@ -235,25 +235,27 @@
 	struct obd_ioctl_hdr hdr;
 	struct obd_ioctl_data *data;
 	int offset = 0;
+	int rc = 0;
 
 	ENTRY;
 	if (copy_from_user(&hdr, arg, sizeof(hdr)))
 		RETURN(-EFAULT);
 
 	if (hdr.ioc_version != OBD_IOCTL_VERSION) {
-		CERROR("Version mismatch kernel (%x) vs application (%x)\n",
-		       OBD_IOCTL_VERSION, hdr.ioc_version);
+		CERROR("%s: version mismatch kernel (%x) vs application (%x)\n",
+		       current->comm, OBD_IOCTL_VERSION, hdr.ioc_version);
 		RETURN(-EINVAL);
 	}
 
 	if (hdr.ioc_len > OBD_MAX_IOCTL_BUFFER) {
-		CERROR("User buffer len %d exceeds %d max buffer\n",
-		       hdr.ioc_len, OBD_MAX_IOCTL_BUFFER);
+		CERROR("%s: user buffer len %d exceeds %d max buffer\n",
+		       current->comm, hdr.ioc_len, OBD_MAX_IOCTL_BUFFER);
 		RETURN(-EINVAL);
 	}
 
 	if (hdr.ioc_len < sizeof(struct obd_ioctl_data)) {
-		CERROR("User buffer too small for ioctl (%d)\n", hdr.ioc_len);
+		CERROR("%s: user buffer too small for ioctl (%d)\n",
+		       current->comm, hdr.ioc_len);
 		RETURN(-EINVAL);
 	}
 
@@ -264,22 +266,17 @@
 	 */
 	OBD_ALLOC_LARGE(data, hdr.ioc_len);
 	if (!data) {
-		CERROR("Cannot allocate control buffer of len %d\n",
-		       hdr.ioc_len);
-		RETURN(-EINVAL);
+		CERROR("%s: cannot allocate control buffer of len %d\n",
+		       current->comm, hdr.ioc_len);
+		RETURN(-ENOMEM);
 	}
 	*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];
@@ -302,10 +299,14 @@
 	*datap = data;
 
 	RETURN(0);
+
+out_free:
+	OBD_FREE_LARGE(data, *len);
+	RETURN(rc);
 }
 EXPORT_SYMBOL(obd_ioctl_getdata);
 
-int class_handle_ioctl(unsigned int cmd, unsigned long arg)
+int class_handle_ioctl(unsigned int cmd, void __user *uarg)
 {
 	struct obd_ioctl_data *data;
 	struct obd_device *obd = NULL;
@@ -313,32 +314,33 @@
 
 	ENTRY;
 	CDEBUG(D_IOCTL, "cmd = %x\n", cmd);
-	if (obd_ioctl_getdata(&data, &len, (void __user *)arg)) {
-		CERROR("OBD ioctl: data error\n");
+	err = obd_ioctl_getdata(&data, &len, uarg);
+	if (err) {
+		CERROR("%s: ioctl data error\n", current->comm);
 		RETURN(-EINVAL);
 	}
 
-        switch (cmd) {
-        case OBD_IOC_PROCESS_CFG: {
-                struct lustre_cfg *lcfg;
+	switch (cmd) {
+	case OBD_IOC_PROCESS_CFG: {
+		struct lustre_cfg *lcfg;
 
-                if (!data->ioc_plen1 || !data->ioc_pbuf1) {
-                        CERROR("No config buffer passed!\n");
-                        GOTO(out, err = -EINVAL);
-                }
-                OBD_ALLOC(lcfg, data->ioc_plen1);
-                if (lcfg == NULL)
-                        GOTO(out, err = -ENOMEM);
+		if (!data->ioc_plen1 || !data->ioc_pbuf1) {
+			CERROR("No config buffer passed!\n");
+			GOTO(out, err = -EINVAL);
+		}
+		OBD_ALLOC(lcfg, data->ioc_plen1);
+		if (lcfg == NULL)
+			GOTO(out, err = -ENOMEM);
 		err = copy_from_user(lcfg, data->ioc_pbuf1,
-                                         data->ioc_plen1);
-                if (!err)
-                        err = lustre_cfg_sanity_check(lcfg, data->ioc_plen1);
-                if (!err)
-                        err = class_process_config(lcfg);
+				     data->ioc_plen1);
+		if (!err)
+			err = lustre_cfg_sanity_check(lcfg, data->ioc_plen1);
+		if (!err)
+			err = class_process_config(lcfg);
 
-                OBD_FREE(lcfg, data->ioc_plen1);
-                GOTO(out, err);
-        }
+		OBD_FREE(lcfg, data->ioc_plen1);
+		GOTO(out, err);
+	}
 
 #if LUSTRE_VERSION_CODE < OBD_OCD_VERSION(3, 0, 53, 0)
 	case OBD_GET_VERSION: {
@@ -363,77 +365,77 @@
 		memcpy(data->ioc_bulk, LUSTRE_VERSION_STRING,
 		       strlen(LUSTRE_VERSION_STRING) + 1);
 
-		if (copy_to_user((void __user *)arg, data, len))
+		if (copy_to_user(uarg, data, len))
 			err = -EFAULT;
 		GOTO(out, err);
 	}
 #endif
-        case OBD_IOC_NAME2DEV: {
-                /* Resolve a device name.  This does not change the
-                 * currently selected device.
-                 */
-                int dev;
+	case OBD_IOC_NAME2DEV: {
+		/* Resolve a device name.  This does not change the
+		 * currently selected device.
+		 */
+		int dev;
 
-                dev = class_resolve_dev_name(data->ioc_inllen1,
-                                             data->ioc_inlbuf1);
-                data->ioc_dev = dev;
-                if (dev < 0)
-                        GOTO(out, err = -EINVAL);
+		dev = class_resolve_dev_name(data->ioc_inllen1,
+					     data->ioc_inlbuf1);
+		data->ioc_dev = dev;
+		if (dev < 0)
+			GOTO(out, err = -EINVAL);
 
-		if (copy_to_user((void __user *)arg, data, sizeof(*data)))
-                        err = -EFAULT;
-                GOTO(out, err);
-        }
+		if (copy_to_user(uarg, data, sizeof(*data)))
+			err = -EFAULT;
+		GOTO(out, err);
+	}
 
-        case OBD_IOC_UUID2DEV: {
-                /* Resolve a device uuid.  This does not change the
-                 * currently selected device.
-                 */
-                int dev;
-                struct obd_uuid uuid;
+	case OBD_IOC_UUID2DEV: {
+		/* Resolve a device uuid.  This does not change the
+		 * currently selected device.
+		 */
+		int dev;
+		struct obd_uuid uuid;
 
-                if (!data->ioc_inllen1 || !data->ioc_inlbuf1) {
-                        CERROR("No UUID passed!\n");
-                        GOTO(out, err = -EINVAL);
-                }
-                if (data->ioc_inlbuf1[data->ioc_inllen1 - 1] != 0) {
-                        CERROR("UUID not NUL terminated!\n");
-                        GOTO(out, err = -EINVAL);
-                }
+		if (!data->ioc_inllen1 || !data->ioc_inlbuf1) {
+			CERROR("No UUID passed!\n");
+			GOTO(out, err = -EINVAL);
+		}
+		if (data->ioc_inlbuf1[data->ioc_inllen1 - 1] != 0) {
+			CERROR("UUID not NUL terminated!\n");
+			GOTO(out, err = -EINVAL);
+		}
 
-                CDEBUG(D_IOCTL, "device name %s\n", data->ioc_inlbuf1);
-                obd_str2uuid(&uuid, data->ioc_inlbuf1);
-                dev = class_uuid2dev(&uuid);
-                data->ioc_dev = dev;
-                if (dev == -1) {
-                        CDEBUG(D_IOCTL, "No device for UUID %s!\n",
-                               data->ioc_inlbuf1);
-                        GOTO(out, err = -EINVAL);
-                }
+		CDEBUG(D_IOCTL, "device name %s\n", data->ioc_inlbuf1);
+		obd_str2uuid(&uuid, data->ioc_inlbuf1);
+		dev = class_uuid2dev(&uuid);
+		data->ioc_dev = dev;
+		if (dev == -1) {
+			CDEBUG(D_IOCTL, "No device for UUID %s!\n",
+			       data->ioc_inlbuf1);
+			GOTO(out, err = -EINVAL);
+		}
 
-                CDEBUG(D_IOCTL, "device name %s, dev %d\n", data->ioc_inlbuf1,
-                       dev);
-		if (copy_to_user((void __user *)arg, data, sizeof(*data)))
-                        err = -EFAULT;
-                GOTO(out, err);
-        }
+		CDEBUG(D_IOCTL, "device name %s, dev %d\n", data->ioc_inlbuf1,
+		       dev);
+		if (copy_to_user(uarg, data, sizeof(*data)))
+			err = -EFAULT;
+		GOTO(out, err);
+	}
 
-        case OBD_IOC_GETDEVICE: {
-                int     index = data->ioc_count;
-                char    *status, *str;
+	case OBD_IOC_GETDEVICE: {
+		int index = data->ioc_count;
+		char *status, *str;
 
-                if (!data->ioc_inlbuf1) {
-                        CERROR("No buffer passed in ioctl\n");
-                        GOTO(out, err = -EINVAL);
-                }
-                if (data->ioc_inllen1 < 128) {
-                        CERROR("ioctl buffer too small to hold version\n");
-                        GOTO(out, err = -EINVAL);
-                }
+		if (!data->ioc_inlbuf1) {
+			CERROR("No buffer passed in ioctl\n");
+			GOTO(out, err = -EINVAL);
+		}
+		if (data->ioc_inllen1 < 128) {
+			CERROR("ioctl buffer too small to hold version\n");
+			GOTO(out, err = -EINVAL);
+		}
 
-                obd = class_num2obd(index);
-                if (!obd)
-                        GOTO(out, err = -ENOENT);
+		obd = class_num2obd(index);
+		if (!obd)
+			GOTO(out, err = -ENOENT);
 
 		if (obd->obd_stopping)
 			status = "ST";
@@ -446,49 +448,53 @@
 		else
 			status = "--";
 
-                str = (char *)data->ioc_bulk;
-                snprintf(str, len - sizeof(*data), "%3d %s %s %s %s %d",
-                         (int)index, status, obd->obd_type->typ_name,
-                         obd->obd_name, obd->obd_uuid.uuid,
+		str = (char *)data->ioc_bulk;
+		snprintf(str, len - sizeof(*data), "%3d %s %s %s %s %d",
+			 (int)index, status, obd->obd_type->typ_name,
+			 obd->obd_name, obd->obd_uuid.uuid,
 			 atomic_read(&obd->obd_refcount));
 
-		if (copy_to_user((void __user *)arg, data, len))
+		if (copy_to_user(uarg, data, len))
 			err = -EFAULT;
 
 		GOTO(out, err);
-        }
+	}
+	}
 
-        }
+	if (data->ioc_dev == OBD_DEV_BY_DEVNAME) {
+		if (data->ioc_inllen4 <= 0 || data->ioc_inlbuf4 == NULL)
+			GOTO(out, err = -EINVAL);
+		if (strnlen(data->ioc_inlbuf4, MAX_OBD_NAME) >= MAX_OBD_NAME)
+			GOTO(out, err = -EINVAL);
+		obd = class_name2obd(data->ioc_inlbuf4);
+	} else if (data->ioc_dev < class_devno_max()) {
+		obd = class_num2obd(data->ioc_dev);
+	} else {
+		err = -EINVAL;
+		CERROR("OBD ioctl: no device given: rc = %d\n", err);
+		GOTO(out, err);
+	}
 
-        if (data->ioc_dev == OBD_DEV_BY_DEVNAME) {
-                if (data->ioc_inllen4 <= 0 || data->ioc_inlbuf4 == NULL)
-                        GOTO(out, err = -EINVAL);
-                if (strnlen(data->ioc_inlbuf4, MAX_OBD_NAME) >= MAX_OBD_NAME)
-                        GOTO(out, err = -EINVAL);
-                obd = class_name2obd(data->ioc_inlbuf4);
-        } else if (data->ioc_dev < class_devno_max()) {
-                obd = class_num2obd(data->ioc_dev);
-        } else {
-                CERROR("OBD ioctl: No device\n");
-                GOTO(out, err = -EINVAL);
-        }
+	if (obd == NULL) {
+		err = -EINVAL;
+		CERROR("OBD ioctl: no device %d: rc = %d\n",
+		       data->ioc_dev, err);
+		GOTO(out, err);
+	}
+	LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC);
 
-        if (obd == NULL) {
-                CERROR("OBD ioctl : No Device %d\n", data->ioc_dev);
-                GOTO(out, err = -EINVAL);
-        }
-        LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC);
-
-        if (!obd->obd_set_up || obd->obd_stopping) {
-                CERROR("OBD ioctl: device not setup %d \n", data->ioc_dev);
-                GOTO(out, err = -EINVAL);
-        }
+	if (!obd->obd_set_up || obd->obd_stopping) {
+		err = -EINVAL;
+		CERROR("OBD ioctl: device %d not set up: rc = %d\n",
+		       data->ioc_dev, err);
+		GOTO(out, err);
+	}
 
 	err = obd_iocontrol(cmd, obd->obd_self_export, len, data, NULL);
 	if (err)
 		GOTO(out, err);
 
-	if (copy_to_user((void __user *)arg, data, len))
+	if (copy_to_user(uarg, data, len))
 		err = -EFAULT;
 out:
 	OBD_FREE_LARGE(data, len);
@@ -504,12 +510,12 @@
 	ENTRY;
 	/* Allow non-root access for some limited ioctls */
 	if (!capable(CAP_SYS_ADMIN))
-		RETURN(err = -EACCES);
+		RETURN(-EACCES);
 
 	if ((cmd & 0xffffff00) == ((int)'T') << 8) /* ignore all tty ioctls */
-		RETURN(err = -ENOTTY);
+		RETURN(-ENOTTY);
 
-	err = class_handle_ioctl(cmd, (unsigned long)arg);
+	err = class_handle_ioctl(cmd, (void __user *)arg);
 
 	RETURN(err);
 }
diff --git a/lustre/obdecho/echo_client.c b/lustre/obdecho/echo_client.c
index f9af2e2..07863c2 100644
--- a/lustre/obdecho/echo_client.c
+++ b/lustre/obdecho/echo_client.c
@@ -2253,20 +2253,25 @@
 #ifdef HAVE_SERVER_SUPPORT
 	struct tgt_session_info *tsi;
 #endif
-	struct obd_device      *obd = exp->exp_obd;
-	struct echo_device     *ed = obd2echo_dev(obd);
+	struct obd_device *obd = exp->exp_obd;
+	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 lu_env          *env;
-	unsigned long		env_tags = 0;
-	__u16			refcheck;
-	struct obdo            *oa;
-	struct lu_fid           fid;
-	int                     rw = OBD_BRW_READ;
-	int                     rc = 0;
+	struct echo_object *eco;
+	struct obd_ioctl_data *data = karg;
+	struct lu_env *env;
+	unsigned long env_tags = 0;
+	__u16 refcheck;
+	struct obdo *oa;
+	struct lu_fid fid;
+	int rw = OBD_BRW_READ;
+	int rc = 0;
 
 	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(data == NULL))
+		RETURN(-EINVAL);
+
 	oa = &data->ioc_obdo1;
 	if (!(oa->o_valid & OBD_MD_FLGROUP)) {
 		oa->o_valid |= OBD_MD_FLGROUP;
@@ -2404,10 +2409,10 @@
 	case OBD_IOC_BRW_READ:
 		rc = echo_client_brw_ioctl(env, rw, exp, data);
 		GOTO(out, rc);
-
 	default:
-		CERROR("echo_ioctl(): unrecognised ioctl %#x\n", cmd);
-		GOTO(out, rc = -ENOTTY);
+		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 	}
 
 	EXIT;
diff --git a/lustre/ofd/ofd_access_log.c b/lustre/ofd/ofd_access_log.c
index a1db9b2..935e6c5 100644
--- a/lustre/ofd/ofd_access_log.c
+++ b/lustre/ofd/ofd_access_log.c
@@ -341,7 +341,7 @@
 	return mask;
 }
 
-static long oal_ioctl_info(struct oal_circ_buf *ocb, unsigned long arg)
+static long oal_ioctl_info(struct oal_circ_buf *ocb, void __user *uarg)
 {
 	struct ofd_access_log *oal = ocb->ocb_access_log;
 
@@ -353,7 +353,7 @@
 				ocb->ocb_circ.tail,
 				oal->oal_log_size) / oal->oal_entry_size;
 
-	lali = (struct lustre_access_log_info_v1 __user *)arg;
+	lali = uarg;
 	BUILD_BUG_ON(sizeof(lali->lali_name) != sizeof(oal->oal_name));
 
 	if (put_user(LUSTRE_ACCESS_LOG_VERSION_1, &lali->lali_version))
@@ -393,7 +393,7 @@
 }
 
 static long oal_file_ioctl(struct file *filp, unsigned int cmd,
-			unsigned long arg)
+			   unsigned long arg)
 {
 	struct oal_circ_buf *ocb = filp->private_data;
 
@@ -401,7 +401,7 @@
 	case LUSTRE_ACCESS_LOG_IOCTL_VERSION:
 		return LUSTRE_ACCESS_LOG_VERSION_1;
 	case LUSTRE_ACCESS_LOG_IOCTL_INFO:
-		return oal_ioctl_info(ocb, arg);
+		return oal_ioctl_info(ocb, (void __user *)arg);
 	case LUSTRE_ACCESS_LOG_IOCTL_FILTER:
 		ocb->ocb_filter = arg;
 		return 0;
@@ -629,7 +629,7 @@
 }
 
 static long oal_control_file_ioctl(struct file *filp, unsigned int cmd,
-				unsigned long arg)
+				   unsigned long arg)
 {
 	struct oal_control_file *ccf = filp->private_data;
 
diff --git a/lustre/ofd/ofd_obd.c b/lustre/ofd/ofd_obd.c
index 166cee3..91953ad 100644
--- a/lustre/ofd/ofd_obd.c
+++ b/lustre/ofd/ofd_obd.c
@@ -1183,8 +1183,7 @@
 	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)) {
@@ -1257,36 +1256,37 @@
 	int rc;
 
 	ENTRY;
-
-	CDEBUG(D_IOCTL, "handling ioctl cmd %#x\n", cmd);
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       obd->obd_name, cmd, len, karg, uarg);
 	rc = lu_env_init(&env, LCT_DT_THREAD);
 	if (rc)
 		RETURN(rc);
 
+	/* handle commands that don't use @karg first */
 	switch (cmd) {
 	case OBD_IOC_ABORT_RECOVERY:
 		CERROR("%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))
+		GOTO(out, rc = -EINVAL);
+
+	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);
@@ -1294,7 +1294,7 @@
 	}
 	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. */
@@ -1302,7 +1302,7 @@
 			stop.ls_flags = 0;
 		else
 			stop.ls_flags =
-			((struct lfsck_stop *)(data->ioc_inlbuf1))->ls_flags;
+			   ((struct lfsck_stop *)(data->ioc_inlbuf1))->ls_flags;
 
 		rc = lfsck_stop(&env, ofd->ofd_osd, &stop);
 		break;
@@ -1311,10 +1311,11 @@
 		rc = ofd_ioc_get_obj_version(&env, ofd, karg);
 		break;
 	default:
-		CERROR("%s: not supported cmd = %#x\n", obd->obd_name, cmd);
 		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 	}
-
+out:
 	lu_env_fini(&env);
 	RETURN(rc);
 }
diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c
index 08a286d..7bed1a8 100644
--- a/lustre/osc/osc_request.c
+++ b/lustre/osc/osc_request.c
@@ -3334,18 +3334,25 @@
 	int rc = 0;
 
 	ENTRY;
+	CDEBUG(D_IOCTL, "%s: cmd=%x len=%u karg=%pK uarg=%pK\n",
+	       obd->obd_name, cmd, len, karg, uarg);
+	if (!data)
+		RETURN(-EINVAL);
+
 	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:
+	case OBD_IOC_CLIENT_RECOVER: {
 		rc = ptlrpc_recover_import(obd->u.cli.cl_import,
 					   data->ioc_inlbuf1, 0);
 		if (rc > 0)
 			rc = 0;
 		break;
+	}
 	case OBD_IOC_GETATTR:
 		rc = obd_getattr(NULL, exp, &data->ioc_obdo1);
 		break;
@@ -3355,8 +3362,9 @@
 		break;
 	default:
 		rc = -ENOTTY;
-		CDEBUG(D_INODE, "%s: unrecognised ioctl %#x by %s: rc = %d\n",
-		       obd->obd_name, cmd, current->comm, rc);
+		CDEBUG(D_IOCTL,
+		       "%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 		break;
 	}
 
diff --git a/lustre/osp/osp_dev.c b/lustre/osp/osp_dev.c
index 5497028..b86b702 100644
--- a/lustre/osp/osp_dev.c
+++ b/lustre/osp/osp_dev.c
@@ -1661,12 +1661,16 @@
 static int osp_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 			 void *karg, void __user *uarg)
 {
-	struct obd_device	*obd = exp->exp_obd;
-	struct osp_device	*d;
-	struct obd_ioctl_data	*data = karg;
-	int			 rc = 0;
+	struct obd_device *obd = exp->exp_obd;
+	struct osp_device *d;
+	struct obd_ioctl_data *data = karg;
+	int rc = 0;
 
 	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(data == NULL))
+		RETURN(-EINVAL);
 
 	LASSERT(obd->obd_lu_dev);
 	d = lu2osp_dev(obd->obd_lu_dev);
@@ -1690,15 +1694,13 @@
 					      data->ioc_offset);
 		break;
 	default:
-		CERROR("%s: unrecognized ioctl %#x by %s\n", obd->obd_name,
-		       cmd, current->comm);
-		rc = -ENOTTY;
+		CERROR("%s: iocontrol from '%s' unknown cmd=%#x: rc = %d\n",
+		       obd->obd_name, current->comm, cmd, rc);
 	}
 	module_put(THIS_MODULE);
 	return rc;
 }
 
-
 /**
  * Implementation of obd_ops::o_get_info
  *
