Re: [HPDD-discuss] [PATCH] staging: lustre: fid: lproc_fid: Removed variables that is never used
On 01/28/2015 04:46 PM, Rickard Strandqvist wrote:
Variable ar assigned a value that is never used.
s/ar/rc/
I have also removed all the code that thereby serves no purpose.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist
---
drivers/staging/lustre/lustre/fid/lproc_fid.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/lustre/lustre/fid/lproc_fid.c
b/drivers/staging/lustre/lustre/fid/lproc_fid.c
index 6a21f07..feb7425 100644
--- a/drivers/staging/lustre/lustre/fid/lproc_fid.c
+++ b/drivers/staging/lustre/lustre/fid/lproc_fid.c
@@ -63,7 +63,6 @@ static int lprocfs_fid_write_common(const char __user
*buffer, size_t count,
struct lu_seq_range *range)
{
struct lu_seq_range tmp;
- int rc;
char kernbuf[MAX_FID_RANGE_STRLEN];
LASSERT(range != NULL);
@@ -82,7 +81,7 @@ static int lprocfs_fid_write_common(const char __user
*buffer, size_t count,
}
/* of the form "[0x00024400 - 0x00028000400]" */
- rc = sscanf(kernbuf, "[%llx - %llx]\n",
+ sscanf(kernbuf, "[%llx - %llx]\n",
I would test rc instead:
if (rc != 2)
return -EINVAL;
Regards,
Frank.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
On 01/29/2015 12:47 PM, Rickard Strandqvist wrote: Fix a possible null pointer dereference, there is otherwise a risk of a possible null pointer dereference. This was found using a static code analysis program called cppcheck Signed-off-by: Rickard Strandqvist --- drivers/staging/lustre/lustre/include/lustre_update.h |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h b/drivers/staging/lustre/lustre/include/lustre_update.h index 84defce..00e1361 100644 --- a/drivers/staging/lustre/lustre/include/lustre_update.h +++ b/drivers/staging/lustre/lustre/include/lustre_update.h @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct update_reply *reply, void **buf, int result; ptr = update_get_buf_internal(reply, index, &size); + + LASSERT((ptr != NULL && size >= sizeof(int))); Now size is tested before result. So it could assert if result < 0, while the function would have returned before. + result = *(int *)ptr; if (result < 0) return result; - LASSERT((ptr != NULL && size >= sizeof(int))); *buf = ptr + sizeof(int); return size - sizeof(int); } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
On 01/29/2015 01:47 PM, Rickard Strandqvist wrote: 2015-01-29 20:40 GMT+01:00 Frank Zago : On 01/29/2015 12:47 PM, Rickard Strandqvist wrote: Fix a possible null pointer dereference, there is otherwise a risk of a possible null pointer dereference. This was found using a static code analysis program called cppcheck Signed-off-by: Rickard Strandqvist --- drivers/staging/lustre/lustre/include/lustre_update.h |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h b/drivers/staging/lustre/lustre/include/lustre_update.h index 84defce..00e1361 100644 --- a/drivers/staging/lustre/lustre/include/lustre_update.h +++ b/drivers/staging/lustre/lustre/include/lustre_update.h @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct update_reply *reply, void **buf, int result; ptr = update_get_buf_internal(reply, index, &size); + + LASSERT((ptr != NULL && size >= sizeof(int))); Now size is tested before result. So it could assert if result < 0, while the function would have returned before. + result = *(int *)ptr; if (result < 0) return result; - LASSERT((ptr != NULL && size >= sizeof(int))); *buf = ptr + sizeof(int); return size - sizeof(int); } But if prt is null krachar on the line: result = *(int *)ptr; Maybe there should be two LASSERT then. Yes, that would be safer. Frank. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: lustre: potential underflow in libcfs_kkuc_group_add()
On 11/03/2015 05:28 PM, Simmons, James A. wrote:
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
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.
diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
b/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
index a989d26..41f3d81 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
@@ -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,
diff --git a/drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c
b/drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c
index ad661a3..d8230ae 100644
--- a/drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c
+++ b/drivers/staging/lustre/lustre/libcfs/kernel_user_comm.c
@@ -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.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
