Branch HEAD

copy_{to,from}_user() do not return an error code, but rather the number
of uncopied bytes (0 if all bytes were copied).  Storing this return value
into "rc" is misleading because it then seems this should be returned to
the caller of the function.

Code was audited to remote storing of the return value into "rc" and
several incorrect uses were found.

diff --git a/lustre/liblustre/super.c b/lustre/liblustre/super.c
index ccc19b2..258de2e 100644 (file)
--- a/lustre/liblustre/super.c
+++ b/lustre/liblustre/super.c
@@ -1726,8 +1726,7 @@ static int llu_lov_dir_setstripe(struct inode *ino, unsigned long arg)
         LASSERT(sizeof(lum) == sizeof(*lump));
         LASSERT(sizeof(lum.lmm_objects[0]) ==
                 sizeof(lump->lmm_objects[0]));
-        rc = copy_from_user(&lum, lump, sizeof(lum));
-        if (rc)
+        if (copy_from_user(&lum, lump, sizeof(lum)))
                 return(-EFAULT);
 
         switch (lum.lmm_magic) {
@@ -1859,8 +1858,7 @@ static int llu_lov_file_setstripe(struct inode *ino, unsigned long arg)
 
         LASSERT(sizeof(lum) == sizeof(*lump));
         LASSERT(sizeof(lum.lmm_objects[0]) == sizeof(lump->lmm_objects[0]));
-        rc = copy_from_user(&lum, lump, sizeof(lum));
-        if (rc)
+        if (copy_from_user(&lum, lump, sizeof(lum)))
                 RETURN(-EFAULT);
 
         rc = llu_lov_setstripe_ea_info(ino, flags, &lum, sizeof(lum));

diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c
index b38e429..061f82e 100644 (file)
--- a/lustre/llite/dir.c
+++ b/lustre/llite/dir.c
@@ -616,14 +616,14 @@ int ll_dir_setstripe(struct inode *inode, struct lov_user_md *lump,
                         goto end;
 
                 /* Set root stripecount */
-                sprintf(param, "%s-MDT0000.lov.stripecount=%u", fsname,
+                sprintf(param, "%s-MDT0000.lov.stripecount=%hd", fsname,
                         lump->lmm_stripe_count);
                 rc = ll_send_mgc_param(mgc->u.cli.cl_mgc_mgsexp, param);
                 if (rc)
                         goto end;
 
                 /* Set root stripeoffset */
-                sprintf(param, "%s-MDT0000.lov.stripeoffset=%u", fsname,
+                sprintf(param, "%s-MDT0000.lov.stripeoffset=%hd", fsname,
                         lump->lmm_stripe_offset);
                 rc = ll_send_mgc_param(mgc->u.cli.cl_mgc_mgsexp, param);
                 if (rc)
@@ -781,13 +781,11 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                 LASSERT(sizeof(lumv3.lmm_objects[0]) ==
                         sizeof(lumv3p->lmm_objects[0]));
                 /* first try with v1 which is smaller than v3 */
-                rc = copy_from_user(lumv1, lumv1p, sizeof(*lumv1));
-                if (rc)
+                if (copy_from_user(lumv1, lumv1p, sizeof(*lumv1)))
                         RETURN(-EFAULT);
 
                 if (lumv1->lmm_magic == LOV_USER_MAGIC_V3) {
-                        rc = copy_from_user(&lumv3, lumv3p, sizeof(lumv3));
-                        if (rc)
+                        if (copy_from_user(&lumv3, lumv3p, sizeof(lumv3)))
                                 RETURN(-EFAULT);
                 }
 
@@ -848,8 +846,7 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                         lmdp = (struct lov_user_mds_data *)arg;
                         lump = &lmdp->lmd_lmm;
                 }
-                rc = copy_to_user(lump, lmm, lmmsize);
-                if (rc)
+                if (copy_to_user(lump, lmm, lmmsize))
                         GOTO(out_lmm, rc = -EFAULT);
         skip_lmm:
                 if (cmd == IOC_MDC_GETFILEINFO || cmd == LL_IOC_MDC_GETINFO) {
@@ -871,8 +868,7 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                         st.st_ino     = inode->i_ino;
 
                         lmdp = (struct lov_user_mds_data *)arg;
-                        rc = copy_to_user(&lmdp->lmd_st, &st, sizeof(st));
-                        if (rc)
+                        if (copy_to_user(&lmdp->lmd_st, &st, sizeof(st)))
                                 GOTO(out_lmm, rc = -EFAULT);
                 }
 
@@ -903,8 +899,7 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                         RETURN(rc);
 
                 OBD_ALLOC(lmm, lmmsize);
-                rc = copy_from_user(lmm, lum, lmmsize);
-                if (rc)
+                if (copy_from_user(lmm, lum, lmmsize))
                         GOTO(free_lmm, rc = -EFAULT);
 
                 switch (lmm->lmm_magic) {
@@ -945,8 +940,7 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                 if (rc)
                         GOTO(free_lsm, rc);
 
-                rc = copy_to_user(&lumd->lmd_st, &st, sizeof(st));
-                if (rc)
+                if (copy_to_user(&lumd->lmd_st, &st, sizeof(st)))
                         GOTO(free_lsm, rc = -EFAULT);
 
                 EXIT;
@@ -1005,7 +999,8 @@ static int ll_dir_ioctl(struct inode *inode, struct file *file,
                 if (!rc) {
                         str = req_capsule_server_get(&req->rq_pill,
                                                      &RMF_STRING);
-                        rc = copy_to_user(data->ioc_pbuf1, str, data->ioc_plen1);
+                        if (copy_to_user(data->ioc_pbuf1, str, data->ioc_plen1))
+                                rc = -EFAULT;
                 }
                 ptlrpc_req_finished(req);
         out_catinfo:

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 63f790a..530cefc 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -1971,11 +1971,10 @@ static int ll_lov_recreate_obj(struct inode *inode, struct file *file,
         if (!cfs_capable(CFS_CAP_SYS_ADMIN))
                 RETURN(-EPERM);
 
-        rc = copy_from_user(&ucreatp, (struct ll_recreate_obj *)arg,
-                            sizeof(struct ll_recreate_obj));
-        if (rc) {
+        if (copy_from_user(&ucreatp, (struct ll_recreate_obj *)arg,
+                           sizeof(struct ll_recreate_obj)))
                 RETURN(-EFAULT);
-        }
+
         OBDO_ALLOC(oa);
         if (oa == NULL)
                 RETURN(-ENOMEM);
@@ -2194,8 +2193,7 @@ static int ll_lov_setea(struct inode *inode, struct file *file,
         if (lump == NULL) {
                 RETURN(-ENOMEM);
         }
-        rc = copy_from_user(lump, (struct lov_user_md  *)arg, lum_size);
-        if (rc) {
+        if (copy_from_user(lump, (struct lov_user_md  *)arg, lum_size)) {
                 OBD_FREE(lump, lum_size);
                 RETURN(-EFAULT);
         }
@@ -2220,14 +2218,12 @@ static int ll_lov_setstripe(struct inode *inode, struct file *file,
 
         /* first try with v1 which is smaller than v3 */
         lum_size = sizeof(struct lov_user_md_v1);
-        rc = copy_from_user(lumv1, lumv1p, lum_size);
-        if (rc)
+        if (copy_from_user(lumv1, lumv1p, lum_size))
                 RETURN(-EFAULT);
 
         if (lumv1->lmm_magic == LOV_USER_MAGIC_V3) {
                 lum_size = sizeof(struct lov_user_md_v3);
-                rc = copy_from_user(&lumv3, lumv3p, lum_size);
-                if (rc)
+                if (copy_from_user(&lumv3, lumv3p, lum_size))
                         RETURN(-EFAULT);
         }
 

diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c
index bb19245..1c8f940 100644 (file)
--- a/lustre/lmv/lmv_obd.c
+++ b/lustre/lmv/lmv_obd.c
@@ -758,9 +758,10 @@ static int lmv_iocontrol(unsigned int cmd, struct obd_export *exp,
                 if (rc)
                         RETURN(rc);
                 if (copy_to_user(data->ioc_pbuf1, &stat_buf, data->ioc_plen1))
-                        RETURN(rc);
-                rc = copy_to_user(data->ioc_pbuf2, obd2cli_tgt(mdc_obd),
-                                  data->ioc_plen2);
+                        RETURN(-EFAULT);
+                if (copy_to_user(data->ioc_pbuf2, obd2cli_tgt(mdc_obd),
+                                  data->ioc_plen2))
+                        RETURN(-EFAULT);
                 break;
         }
         default : {

diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c
index c9240d1..84835e5 100644 (file)
--- a/lustre/lov/lov_obd.c
+++ b/lustre/lov/lov_obd.c
@@ -470,6 +470,7 @@ static int lov_disconnect(struct obd_export *exp)
         /* Let's hold another reference so lov_del_obd doesn't spin through
            putref every time */
         lov_getref(obd);
+
         for (i = 0; i < lov->desc.ld_tgt_count; i++) {
                 if (lov->lov_tgts[i] && lov->lov_tgts[i]->ltd_exp) {
                         /* Disconnection is the last we know about an obd */
@@ -2385,7 +2386,7 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 {
         struct obd_device *obddev = class_exp2obd(exp);
         struct lov_obd *lov = &obddev->u.lov;
-        int i, rc, count = lov->desc.ld_tgt_count;
+        int i, rc = 0, count = lov->desc.ld_tgt_count;
         struct obd_uuid *uuidp;
         ENTRY;
 
@@ -2418,10 +2419,11 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
                 if (rc)
                         RETURN(rc);
                 if (copy_to_user(data->ioc_pbuf1, &stat_buf, data->ioc_plen1))
-                        RETURN(rc);
+                        RETURN(-EFAULT);
                 /* copy UUID */
-                rc = copy_to_user(data->ioc_pbuf2, obd2cli_tgt(osc_obd),
-                                  data->ioc_plen2);
+                if (copy_to_user(data->ioc_pbuf2, obd2cli_tgt(osc_obd),
+                                 data->ioc_plen2))
+                        RETURN(-EFAULT);
                 break;
         }
         case OBD_IOC_LOV_GET_CONFIG: {
@@ -2464,8 +2466,7 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
                         *genp = lov->lov_tgts[i]->ltd_gen;
                 }
 
-                rc = copy_to_user((void *)uarg, buf, len);
-                if (rc)
+                if (copy_to_user((void *)uarg, buf, len))
                         rc = -EFAULT;
                 obd_ioctl_freedata(buf, len);
                 break;
@@ -2485,7 +2486,6 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
                 if (count == 0)
                         RETURN(-ENOTTY);
 
-                rc = 0;
                 for (i = 0; i < count; i++) {
                         int err;
 

diff --git a/lustre/lov/lov_pack.c b/lustre/lov/lov_pack.c
index a39e92b..d992e54 100644 (file)
--- a/lustre/lov/lov_pack.c
+++ b/lustre/lov/lov_pack.c
@@ -418,8 +418,7 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp,
         int rc;
         ENTRY;
 
-        rc = copy_from_user(&lumv3, lump, sizeof(struct lov_user_md_v1));
-        if (rc)
+        if (copy_from_user(&lumv3, lump, sizeof(struct lov_user_md_v1)))
                 RETURN(-EFAULT);
 
         lmm_magic = lumv1->lmm_magic;
@@ -428,12 +427,10 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp,
                 lustre_swab_lov_user_md_v1(lumv1);
                 lmm_magic = LOV_USER_MAGIC_V1;
         } else if (lmm_magic == LOV_USER_MAGIC_V3) {
-                rc = copy_from_user(&lumv3, lump, sizeof(lumv3));
-                if (rc)
+                if (copy_from_user(&lumv3, lump, sizeof(lumv3)))
                         RETURN(-EFAULT);
         } else if (lmm_magic == __swab32(LOV_USER_MAGIC_V3)) {
-                rc = copy_from_user(&lumv3, lump, sizeof(lumv3));
-                if (rc)
+                if (copy_from_user(&lumv3, lump, sizeof(lumv3)))
                         RETURN(-EFAULT);
                 lustre_swab_lov_user_md_v3(&lumv3);
                 lmm_magic = LOV_USER_MAGIC_V3;
@@ -616,9 +613,7 @@ int lov_getstripe(struct obd_export *exp, struct lov_stripe_md *lsm,
         /* we only need the header part from user space to get lmm_magic and
          * lmm_stripe_count, (the header part is common to v1 and v3) */
         lum_size = sizeof(struct lov_user_md_v1);
-        rc = copy_from_user(&lum, lump, lum_size);
-
-        if (rc)
+        if (copy_from_user(&lum, lump, lum_size))
                 rc = -EFAULT;
         else if ((lum.lmm_magic != LOV_USER_MAGIC) &&
                  (lum.lmm_magic != LOV_USER_MAGIC_V3))

diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c
index 99cb466..aca7b6d 100644 (file)
--- a/lustre/mgs/mgs_handler.c
+++ b/lustre/mgs/mgs_handler.c
@@ -773,17 +773,14 @@ static int mgs_iocontrol_pool(struct obd_device *obd,
         }
 
         OBD_ALLOC(lcfg, data->ioc_plen1);
-        if (lcfg == NULL) {
-                rc = -ENOMEM;
-                GOTO(out_pool, rc);
-        }
-        rc = copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1);
-        if (rc)
-                GOTO(out_pool, rc);
+        if (lcfg == NULL)
+                GOTO(out_pool, rc = -ENOMEM);
+
+        if (copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1))
+                GOTO(out_pool, rc = -EFAULT);
 
         if (lcfg->lcfg_bufcount < 2) {
-                rc = -EINVAL;
-                GOTO(out_pool, rc);
+                GOTO(out_pool, rc = -EFAULT);
         }
 
         /* first arg is always <fsname>.<poolname> */
@@ -879,9 +876,8 @@ int mgs_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
                 OBD_ALLOC(lcfg, data->ioc_plen1);
                 if (lcfg == NULL)
                         RETURN(-ENOMEM);
-                rc = copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1);
-                if (rc)
-                        GOTO(out_free, rc);
+                if (copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1))
+                        GOTO(out_free, rc = -EFAULT);
 
                 if (lcfg->lcfg_bufcount < 1)
                         GOTO(out_free, rc = -EINVAL);