All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou
@ 2012-10-07 12:56 devendra.aaru
  2012-10-22 10:42 ` Alois Schloegl
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: devendra.aaru @ 2012-10-07 12:56 UTC (permalink / raw)
  To: kernel-janitors

Hi, Fengguang,

On Sun, Oct 7, 2012 at 7:07 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
> Hi Alois,
>
> FYI, there are new smatch warnings show up in
>
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-next
> head:   e1878957b4676a17cf398f7f5723b365e9a2ca48
> commit: 2eae6bdc12f4e49b7f94f032b82d664dcf3881bc [115/274] Staging: add ced1401 USB driver
>
>
> + drivers/staging/ced1401/ced_ioc.c:898 GetTransfer() warn: 'tx' puts 3104 bytes on stack

I have a following patch to send to Greg, when the merge window closes:


From 7d9b8485ab837bf9e0f960d090f90c6518f7e2db Mon Sep 17 00:00:00 2001
From: Devendra Naga <devendra.aaru@gmail.com>
Date: Sat, 6 Oct 2012 02:57:42 -0400
Subject: [PATCH 1/4] staging: ced1401: fix a frame size warning

gcc/sparse complain about the following:

drivers/staging/ced1401/ced_ioc.c:931:1: warning: the frame size of
4144 bytes is larger than 2048 bytes [-Wframe-larger-than=]

fix it by using a pointer of the TGET_TX_BLOCK struct

Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
---
 drivers/staging/ced1401/ced_ioc.c |   28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index c9492ed..a136ca3 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -913,17 +913,23 @@ int GetTransfer(DEVICE_EXTENSION * pdx,
TGET_TX_BLOCK __user * pTX)
                iReturn = U14ERR_BADAREA;
        else {
                // Return the best information we have - we don't have
physical addresses
-               TGET_TX_BLOCK tx;
-               memset(&tx, 0, sizeof(tx));     // clean out local
work structure
-               tx.size = pdx->rTransDef[dwIdent].dwLength;
-               tx.linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
-               tx.avail = GET_TX_MAXENTRIES;   // how many blocks we
could return
-               tx.used = 1;    // number we actually return
-               tx.entries[0].physical -                   (long long)(tx.linear + pdx->StagedOffset);
-               tx.entries[0].size = tx.size;
-
-               if (copy_to_user(pTX, &tx, sizeof(tx)))
+               TGET_TX_BLOCK *tx;
+
+               tx = kzalloc(sizeof(*tx), GFP_KERNEL);
+               if (!tx) {
+                       mutex_unlock(&pdx->io_mutex);
+                       return -ENOMEM;

+               }
+
+               tx->size = pdx->rTransDef[dwIdent].dwLength;
+               tx->linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
+               tx->avail = GET_TX_MAXENTRIES;  // how many blocks we
could return
+               tx->used = 1;   // number we actually return
+               tx->entries[0].physical +                   (long long)(tx->linear + pdx->StagedOffset);
+               tx->entries[0].size = tx->size;
+
+               if (copy_to_user(pTX, tx, sizeof(*tx)))
                        iReturn = -EFAULT;
        }
        mutex_unlock(&pdx->io_mutex);

> 0-DAY kernel build testing backend         Open Source Technology Center
> Fengguang Wu, Yuanhan Liu                              Intel Corporation
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
>

Thanks,

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou
  2012-10-07 12:56 [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou devendra.aaru
@ 2012-10-22 10:42 ` Alois Schloegl
  2012-10-22 11:43 ` Dan Carpenter
  2012-10-22 15:52 ` devendra.aaru
  2 siblings, 0 replies; 4+ messages in thread
From: Alois Schloegl @ 2012-10-22 10:42 UTC (permalink / raw)
  To: kernel-janitors

Hi,


Please include (Greg Smith <greg@ced.co.uk>) into any communication.
Greg Smith from CED noticed that there is no kfree, causing a memory 
leak, and memset() has been removed, thus tx->used was not cleared.

The patch below should fix this.

   Alois




diff --git a/ced_ioc.c b/ced_ioc.c
index 4a13c10..fe15c44 100644
--- a/ced_ioc.c
+++ b/ced_ioc.c
@@ -895,15 +895,23 @@ int GetTransfer(DEVICE_EXTENSION *pdx, 
TGET_TX_BLOCK __user *pTX)
      else
      {
          // Return the best information we have - we don't have 
physical addresses
-        TGET_TX_BLOCK tx;
-        memset(&tx, 0, sizeof(tx));         // clean out local work 
structure
-        tx.size = pdx->rTransDef[dwIdent].dwLength;
-        tx.linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
-        tx.avail = GET_TX_MAXENTRIES;       // how many blocks we could 
return
-        tx.used = 1;                        // number we actually return
-        tx.entries[0].physical = (long long)(tx.linear+pdx->StagedOffset);
-        tx.entries[0].size = tx.size;
-        copy_to_user(pTX, &tx, sizeof(tx));
+        TGET_TX_BLOCK *tx;
+        tx = kzalloc(sizeof(*tx), GFP_KERNEL);
+        if (!tx) {
+                mutex_unlock(&pdx->io_mutex);
+                return -ENOMEM;
+        }
+
+        memset(tx, 0, sizeof(*tx));         // clean out local work 
structure
+        tx->size = pdx->rTransDef[dwIdent].dwLength;
+        tx->linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
+        tx->avail = GET_TX_MAXENTRIES;       // how many blocks we 
could return
+        tx->used = 1;                        // number we actually return
+        tx->entries[0].physical = (long 
long)(tx->linear+pdx->StagedOffset);
+        tx->entries[0].size = tx->size;
+        iReturn = copy_to_user(pTX, tx, sizeof(*tx));
+        kfree(tx);
+        return (iReturn);
      }
      mutex_unlock(&pdx->io_mutex);
      return iReturn;




On 10/07/2012 02:56 PM, devendra.aaru wrote:
> Hi, Fengguang,
>
> On Sun, Oct 7, 2012 at 7:07 AM, Fengguang Wu<fengguang.wu@intel.com>  wrote:
>> Hi Alois,
>>
>> FYI, there are new smatch warnings show up in
>>
>> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-next
>> head:   e1878957b4676a17cf398f7f5723b365e9a2ca48
>> commit: 2eae6bdc12f4e49b7f94f032b82d664dcf3881bc [115/274] Staging: add ced1401 USB driver
>>
>>
>> + drivers/staging/ced1401/ced_ioc.c:898 GetTransfer() warn: 'tx' puts 3104 bytes on stack
>
> I have a following patch to send to Greg, when the merge window closes:
>
>
>  From 7d9b8485ab837bf9e0f960d090f90c6518f7e2db Mon Sep 17 00:00:00 2001
> From: Devendra Naga<devendra.aaru@gmail.com>
> Date: Sat, 6 Oct 2012 02:57:42 -0400
> Subject: [PATCH 1/4] staging: ced1401: fix a frame size warning
>
> gcc/sparse complain about the following:
>
> drivers/staging/ced1401/ced_ioc.c:931:1: warning: the frame size of
> 4144 bytes is larger than 2048 bytes [-Wframe-larger-than=]
>
> fix it by using a pointer of the TGET_TX_BLOCK struct
>
> Signed-off-by: Devendra Naga<devendra.aaru@gmail.com>
> ---
>   drivers/staging/ced1401/ced_ioc.c |   28 +++++++++++++++++-----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/ced1401/ced_ioc.c
> b/drivers/staging/ced1401/ced_ioc.c
> index c9492ed..a136ca3 100644
> --- a/drivers/staging/ced1401/ced_ioc.c
> +++ b/drivers/staging/ced1401/ced_ioc.c
> @@ -913,17 +913,23 @@ int GetTransfer(DEVICE_EXTENSION * pdx,
> TGET_TX_BLOCK __user * pTX)
>                  iReturn = U14ERR_BADAREA;
>          else {
>                  // Return the best information we have - we don't have
> physical addresses
> -               TGET_TX_BLOCK tx;
> -               memset(&tx, 0, sizeof(tx));     // clean out local
> work structure
> -               tx.size = pdx->rTransDef[dwIdent].dwLength;
> -               tx.linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
> -               tx.avail = GET_TX_MAXENTRIES;   // how many blocks we
> could return
> -               tx.used = 1;    // number we actually return
> -               tx.entries[0].physical > -                   (long long)(tx.linear + pdx->StagedOffset);
> -               tx.entries[0].size = tx.size;
> -
> -               if (copy_to_user(pTX,&tx, sizeof(tx)))
> +               TGET_TX_BLOCK *tx;
> +
> +               tx = kzalloc(sizeof(*tx), GFP_KERNEL);
> +               if (!tx) {
> +                       mutex_unlock(&pdx->io_mutex);
> +                       return -ENOMEM;
>
> +               }
> +
> +               tx->size = pdx->rTransDef[dwIdent].dwLength;
> +               tx->linear = (long long)((long)pdx->rTransDef[dwIdent].lpvBuff);
> +               tx->avail = GET_TX_MAXENTRIES;  // how many blocks we
> could return
> +               tx->used = 1;   // number we actually return
> +               tx->entries[0].physical > +                   (long long)(tx->linear + pdx->StagedOffset);
> +               tx->entries[0].size = tx->size;
> +
> +               if (copy_to_user(pTX, tx, sizeof(*tx)))
>                          iReturn = -EFAULT;
>          }
>          mutex_unlock(&pdx->io_mutex);
>
>> 0-DAY kernel build testing backend         Open Source Technology Center
>> Fengguang Wu, Yuanhan Liu                              Intel Corporation
>>
>> _______________________________________________
>> devel mailing list
>> devel@linuxdriverproject.org
>> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
>>
>
> Thanks,


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou
  2012-10-07 12:56 [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou devendra.aaru
  2012-10-22 10:42 ` Alois Schloegl
@ 2012-10-22 11:43 ` Dan Carpenter
  2012-10-22 15:52 ` devendra.aaru
  2 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2012-10-22 11:43 UTC (permalink / raw)
  To: kernel-janitors

On Mon, Oct 22, 2012 at 12:42:49PM +0200, Alois Schloegl wrote:
> Hi,
> 
> 
> Please include (Greg Smith <greg@ced.co.uk>) into any communication.
> Greg Smith from CED noticed that there is no kfree, causing a memory
> leak, and memset() has been removed, thus tx->used was not cleared.
> 
> The patch below should fix this.

This isn't the right way to submit patches.  Run checkpatch over the
patch.  (No signed off by etc).  Also it's totally corrupted.  Email
it to yourself, save the email, cat raw_email.txt | git am.  Look
over the changelog.  When that works, then resend.

Read Documentation/SubmittingPatches for more info.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou
  2012-10-07 12:56 [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou devendra.aaru
  2012-10-22 10:42 ` Alois Schloegl
  2012-10-22 11:43 ` Dan Carpenter
@ 2012-10-22 15:52 ` devendra.aaru
  2 siblings, 0 replies; 4+ messages in thread
From: devendra.aaru @ 2012-10-22 15:52 UTC (permalink / raw)
  To: kernel-janitors

On Mon, Oct 22, 2012 at 6:42 AM, Alois Schloegl
<alois.schloegl@ist.ac.at> wrote:
> Hi,
>
>
> Please include (Greg Smith <greg@ced.co.uk>) into any communication.
> Greg Smith from CED noticed that there is no kfree, causing a memory leak,

right i fixed it up and sent a patch.

> and memset() has been removed, thus tx->used was not cleared.

because i used kzalloc which does memset of allocated memory.

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-22 15:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-07 12:56 [staging:staging-next 115/274] drivers/staging/ced1401/usb1401.c:1065 Handle1401Esc() error: dou devendra.aaru
2012-10-22 10:42 ` Alois Schloegl
2012-10-22 11:43 ` Dan Carpenter
2012-10-22 15:52 ` devendra.aaru

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.