staging: lustre: potential underflow in mdc_iocontrol()
Subsystems (2)
PATCH 1/1 staging: lustre: potential underflow in mdc_iocontrol()
Smatch complains that "data->ioc_plen2" is a user controlled value and, since we cast to signed int, the limit check can underflow. It's not very serious because probably the copy_to_user() would return -EFAULT on every arch that matters instead of creating an info leak. Also I haven't followed it through to see if the value is really user controlled.
But definitely it would be safer to cast to unsigned so let's do that.
drivers/staging/lustre/lustre/mdc/mdc_request.c +4 -4
@@ -1908,8 +1908,8 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,/* copy UUID */ if (copy_to_user (data->ioc_pbuf2,obd2cli_tgt (obd), -min ((int ) data->ioc_plen2, - (int )sizeof (struct obd_uuid )))) { +min_t (size_t, data->ioc_plen2, +sizeof (struct obd_uuid )))) { rc = -EFAULT ; goto out; }
@@ -1921,8 +1921,8 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len, goto out;if (copy_to_user (data->ioc_pbuf1, &stat_buf, -min ((int ) data->ioc_plen1, - (int )sizeof (stat_buf)))) { +min_t (size_t, data->ioc_plen1, +sizeof (stat_buf)))) { rc = -EFAULT ; goto out; }
PATCH 1/1 staging: lustre: potential underflow in libcfs_kkuc_group_add()
My static checker says that "group" is a user controlled number that can be negative leading to an array underflow. I have looked at it, and I'm not an expert enough in lustre to say for sure if it is really a bug. Anyway, it's simple enough to make the variable unsigned which pleases the static checker and makes it easier to audit.
drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h +1 -1
@@ -91,7 +91,7 @@ typedef int (*libcfs_kkuc_cb_t)(__u32 data, void *cb_arg);/* Kernel methods */ int libcfs_kkuc_msg_put (struct file *fp,void *payload);int libcfs_kkuc_group_put (int group,void *payload); -int libcfs_kkuc_group_add (struct file *fp,int uid,int group, +int libcfs_kkuc_group_add (struct file *fp,int uid,unsigned int group,__u32 data);int libcfs_kkuc_group_rem (int uid,int group); int libcfs_kkuc_group_foreach(int group, libcfs_kkuc_cb_t cb_func,
drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c +2 -1
@@ -110,7 +110,8 @@ static DECLARE_RWSEM(kg_sem);* @param uid identifier for this receiver * @param group group number */ -int libcfs_kkuc_group_add (struct file *filp,int uid,int group,__u32 data) +int libcfs_kkuc_group_add (struct file *filp,int uid,unsigned int group, +__u32 data) {struct kkuc_reg *reg;
RE: [patch] staging: lustre: potential underflow in libcfs_kkuc_group_add()
Quoted 8 lines
My static checker says that "group" is a user controlled number that can be negative leading to an array underflow. I have looked at it, and I'm not an expert enough in lustre to say for sure if it is really a bug. Anyway, it's simple enough to make the variable unsigned which pleases the static checker and makes it easier to audit.
Signed-off-by Dan Carpenter <[email protected]>
Dmitry do you this this could be placed under place under LU-6303 or does a new ticket need to be open. I also CC Frank Zago who is very familiar with this code. To me it looks okay.
drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h +1 -1
@@ -91,7 +91,7 @@ typedef int (*libcfs_kkuc_cb_t)(__u32 data, void *cb_arg);/* Kernel methods */ int libcfs_kkuc_msg_put (struct file *fp,void *payload);int libcfs_kkuc_group_put (int group,void *payload); -int libcfs_kkuc_group_add (struct file *fp,int uid,int group, +int libcfs_kkuc_group_add (struct file *fp,int uid,unsigned int group,__u32 data);int libcfs_kkuc_group_rem (int uid,int group); int libcfs_kkuc_group_foreach(int group, libcfs_kkuc_cb_t cb_func,
drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c +2 -1
@@ -110,7 +110,8 @@ static DECLARE_RWSEM(kg_sem);* @param uid identifier for this receiver * @param group group number */ -int libcfs_kkuc_group_add (struct file *filp,int uid,int group,__u32 data) +int libcfs_kkuc_group_add (struct file *filp,int uid,unsigned int group, +__u32 data) {struct kkuc_reg *reg;
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: lustre: potential underflow in libcfs_kkuc_group_add()
Quoted 30 lines · James A.
Quoted 8 lines
My static checker says that "group" is a user controlled number that can be negative leading to an array underflow. I have looked at it, and I'm not an expert enough in lustre to say for sure if it is really a bug. Anyway, it's simple enough to make the variable unsigned which pleases the static checker and makes it easier to audit.
Signed-off-by Dan Carpenter <[email protected]>Dmitry do you this this could be placed under place under LU-6303 or does a new ticket need to be open. I also CC Frank Zago who is very familiar with this code. To me it looks okay.
drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h +1
-1@@ -91,7 +91,7 @@ typedef int (*libcfs_kkuc_cb_t)(__u32 data, void *cb_arg);/* Kernel methods */ int libcfs_kkuc_msg_put (struct file *fp,void *payload);int libcfs_kkuc_group_put (int group,void *payload); -int libcfs_kkuc_group_add (struct file *fp,int uid,int group, +int libcfs_kkuc_group_add (struct file *fp,int uid,unsigned int group,__u32 data);int libcfs_kkuc_group_rem (int uid,int group); int libcfs_kkuc_group_foreach(int group, libcfs_kkuc_cb_t cb_func,drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c +0
-0
drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c static DECLARE_RWSEM(kg_sem); · line 116
Quoted 8 lines
drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c +2
-1@@ -110,7 +110,8 @@ static DECLARE_RWSEM(kg_sem);* @param uid identifier for this receiver * @param group group number */ -int libcfs_kkuc_group_add (struct file *filp,int uid,int group,__u32 data) +int libcfs_kkuc_group_add (struct file *filp,int uid,unsigned int group, +__u32 data) {struct kkuc_reg *reg;
Yes, but for consistency, all 4 functions should be changed.
Regards,
Frank.
Re: [patch] staging: lustre: potential underflow in libcfs_kkuc_group_add()
Quoted 1 line · Frank Zago
Yes, but for consistency, all 4 functions should be changed.
Thanks for the review. I will send a v2.
regards,
dan carpenter
RE: [patch] staging: lustre: potential underflow in libcfs_kkuc_group_add()
Quoted 4 lines · Frank Zago
Quoted 1 line
Yes, but for consistency, all 4 functions should be changed.
Thanks for the review. I will send a v2.
Greg merged your original patch so it will be a new patch.