Re: [PATCH 4/7] mptfusion: zero kernel-space source of copy_to_user

[Dan Carpenter]

On Wed, Jun 04, 2014 at 12:49:46PM -0400, Joe Lawrence wrote:
 Fixes the following smatch warning:
 
   drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
 possible info leak 'karg'
 

This is a false positive.  I will push a fix for this in Smatch next
week.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] mptfusion: zero kernel-space source of copy_to_user

[Christoph Hellwig]

On Wed, Jun 04, 2014 at 12:58:36PM -0400, Joe Lawrence wrote:
 Hi Dan,
 
 kzalloc silenced that smatch warning, but the code looks like:
 
   (calculate data_size)
   ...
   karg = kmalloc(data_size, GFP_KERNEL);
   ...
   if (copy_from_user(karg, uarg, data_size)) {
   ...
   if (copy_to_user((char __user *)arg, karg, data_size)) {
 
 where 'data_size' once calculated, is unchanged.  Since the size
 allocated is the same copied from the user and the same copied back out
 to the user, would this really be considered an info leak?

I think the stastic checker is wrong here.  But the code would still
benefit from switching to memdup_user, which should shut up the checker
in addition to simplifying the code.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] mptfusion: zero kernel-space source of copy_to_user

[Joe Lawrence]

Fixes the following smatch warning:

  drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
possible info leak 'karg'

Signed-off-by: Joe Lawrence joe.lawre...@stratus.com
Cc: Christoph Hellwig h...@infradead.org
Cc: Dan Carpenter dan.carpen...@oracle.com
Cc: Sreekanth Reddy sreekanth.re...@lsi.com
---
 drivers/message/fusion/mptctl.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index dcc8385..e6d8935 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -1261,7 +1261,7 @@ mptctl_getiocinfo (unsigned long arg, unsigned int 
data_size)
else
return -EFAULT;
 
-   karg = kmalloc(data_size, GFP_KERNEL);
+   karg = kzalloc(data_size, GFP_KERNEL);
if (karg == NULL) {
printk(KERN_ERR MYNAM %s::mpt_ioctl_iocinfo() @%d - no memory 
available!\n,
__FILE__, __LINE__);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] mptfusion: zero kernel-space source of copy_to_user

[Joe Lawrence]

On Wed, 4 Jun 2014 12:49:46 -0400
Joe Lawrence joe.lawre...@stratus.com wrote:

 Fixes the following smatch warning:
 
   drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
 possible info leak 'karg'
 
 Signed-off-by: Joe Lawrence joe.lawre...@stratus.com
 Cc: Christoph Hellwig h...@infradead.org
 Cc: Dan Carpenter dan.carpen...@oracle.com
 Cc: Sreekanth Reddy sreekanth.re...@lsi.com
 ---
  drivers/message/fusion/mptctl.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
 index dcc8385..e6d8935 100644
 --- a/drivers/message/fusion/mptctl.c
 +++ b/drivers/message/fusion/mptctl.c
 @@ -1261,7 +1261,7 @@ mptctl_getiocinfo (unsigned long arg, unsigned int 
 data_size)
   else
   return -EFAULT;
  
 - karg = kmalloc(data_size, GFP_KERNEL);
 + karg = kzalloc(data_size, GFP_KERNEL);
   if (karg == NULL) {
   printk(KERN_ERR MYNAM %s::mpt_ioctl_iocinfo() @%d - no memory 
 available!\n,
   __FILE__, __LINE__);

Hi Dan,

kzalloc silenced that smatch warning, but the code looks like:

  (calculate data_size)
  ...
  karg = kmalloc(data_size, GFP_KERNEL);
  ...
  if (copy_from_user(karg, uarg, data_size)) {
  ...
  if (copy_to_user((char __user *)arg, karg, data_size)) {

where 'data_size' once calculated, is unchanged.  Since the size
allocated is the same copied from the user and the same copied back out
to the user, would this really be considered an info leak?

Regards,

-- Joe
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html