diff mbox

[5/7] chtls: free beyond end of array rspq_skb_cache

Message ID 1524668782-1581-1-git-send-email-atul.gupta@chelsio.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show

Commit Message

Atul Gupta April 25, 2018, 3:06 p.m. UTC
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
 drivers/crypto/chelsio/chtls/chtls_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Dan Carpenter April 26, 2018, 6:52 a.m. UTC | #1
On Wed, Apr 25, 2018 at 08:36:22PM +0530, Atul Gupta wrote:
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
> ---
>  drivers/crypto/chelsio/chtls/chtls_main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
> index e9ffc3d..4dc3d0e 100644
> --- a/drivers/crypto/chelsio/chtls/chtls_main.c
> +++ b/drivers/crypto/chelsio/chtls/chtls_main.c
> @@ -198,7 +198,7 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
>  {
>  	struct cxgb4_lld_info *lldi;
>  	struct chtls_dev *cdev;
> -	int i, j;
> +	int i;
>  
>  	cdev = kzalloc(sizeof(*cdev) + info->nports *
>  		      (sizeof(struct net_device *)), GFP_KERNEL);
> @@ -250,8 +250,8 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
>  
>  	return cdev;
>  out_rspq_skb:
> -	for (j = 0; j <= i; j++)
> -		kfree_skb(cdev->rspq_skb_cache[j]);
> +	for (; i > 0; --i)
> +		kfree_skb(cdev->rspq_skb_cache[i]);


It should be i >= 0 so that we free cdev->rspq_skb_cache[0].

regards,
dan carpenter
Dan Carpenter April 26, 2018, 6:58 a.m. UTC | #2
On Thu, Apr 26, 2018 at 09:52:57AM +0300, Dan Carpenter wrote:
> On Wed, Apr 25, 2018 at 08:36:22PM +0530, Atul Gupta wrote:
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
> > ---
> >  drivers/crypto/chelsio/chtls/chtls_main.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
> > index e9ffc3d..4dc3d0e 100644
> > --- a/drivers/crypto/chelsio/chtls/chtls_main.c
> > +++ b/drivers/crypto/chelsio/chtls/chtls_main.c
> > @@ -198,7 +198,7 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
> >  {
> >  	struct cxgb4_lld_info *lldi;
> >  	struct chtls_dev *cdev;
> > -	int i, j;
> > +	int i;
> >  
> >  	cdev = kzalloc(sizeof(*cdev) + info->nports *
> >  		      (sizeof(struct net_device *)), GFP_KERNEL);
> > @@ -250,8 +250,8 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
> >  
> >  	return cdev;
> >  out_rspq_skb:
> > -	for (j = 0; j <= i; j++)
> > -		kfree_skb(cdev->rspq_skb_cache[j]);
> > +	for (; i > 0; --i)
> > +		kfree_skb(cdev->rspq_skb_cache[i]);
> 
> 
> It should be i >= 0 so that we free cdev->rspq_skb_cache[0].
> 

Also, it's still freeing beyond the end of the array...  All we needed
to do was change the j <= i in the original to j < i.  So it looks like
this:

	for (j = 0; j < i; j++)
		kfree_skb(cdev->rspq_skb_cache[j]);

regards,
dan carpenter
diff mbox

Patch

diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
index e9ffc3d..4dc3d0e 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -198,7 +198,7 @@  static void *chtls_uld_add(const struct cxgb4_lld_info *info)
 {
 	struct cxgb4_lld_info *lldi;
 	struct chtls_dev *cdev;
-	int i, j;
+	int i;
 
 	cdev = kzalloc(sizeof(*cdev) + info->nports *
 		      (sizeof(struct net_device *)), GFP_KERNEL);
@@ -250,8 +250,8 @@  static void *chtls_uld_add(const struct cxgb4_lld_info *info)
 
 	return cdev;
 out_rspq_skb:
-	for (j = 0; j <= i; j++)
-		kfree_skb(cdev->rspq_skb_cache[j]);
+	for (; i > 0; --i)
+		kfree_skb(cdev->rspq_skb_cache[i]);
 	kfree_skb(cdev->askb);
 out_skb:
 	kfree(lldi);