diff mbox

xen-scsifront: Add a missing call to kfree

Message ID 20161119182256.9081-1-lambert.quentin@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Quentin Lambert Nov. 19, 2016, 6:22 p.m. UTC
Most error branches following the call to kmalloc contain
a call to kfree. This patch add these calls where they are
missing.

This issue was found with Hector.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

---
 drivers/scsi/xen-scsifront.c |    1 +
 1 file changed, 1 insertion(+)

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

Comments

Jürgen Groß Nov. 21, 2016, 6:01 a.m. UTC | #1
On 19/11/16 19:22, Quentin Lambert wrote:
> Most error branches following the call to kmalloc contain
> a call to kfree. This patch add these calls where they are
> missing.
> 
> This issue was found with Hector.
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

Nice catch. I think this will need some more work, I'll do a
follow-on patch.

Reviewed-by: Juergen Gross <jgross@suse.com>

> 
> ---
>  drivers/scsi/xen-scsifront.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/scsi/xen-scsifront.c
> +++ b/drivers/scsi/xen-scsifront.c
> @@ -627,6 +627,7 @@ static int scsifront_action_handler(stru
>  
>  	if (scsifront_enter(info)) {
>  		spin_unlock_irq(host->host_lock);
> +		kfree(shadow);
>  		return FAILED;
>  	}
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin K. Petersen Nov. 22, 2016, 3:40 a.m. UTC | #2
>>>>> "Juergen" == Juergen Gross <jgross@suse.com> writes:

Juergen,

Juergen> On 19/11/16 19:22, Quentin Lambert wrote:
>> Most error branches following the call to kmalloc contain a call to
>> kfree. This patch add these calls where they are missing.
>> 
>> This issue was found with Hector.
>> 
>> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

Juergen> Nice catch. I think this will need some more work, I'll do a
Juergen> follow-on patch.

Juergen> Reviewed-by: Juergen Gross <jgross@suse.com>

Are you taking this patch through the Xen tree or should I queue it up?
Jürgen Groß Nov. 22, 2016, 5:19 a.m. UTC | #3
On 22/11/16 04:40, Martin K. Petersen wrote:
>>>>>> "Juergen" == Juergen Gross <jgross@suse.com> writes:
> 
> Juergen,
> 
> Juergen> On 19/11/16 19:22, Quentin Lambert wrote:
>>> Most error branches following the call to kmalloc contain a call to
>>> kfree. This patch add these calls where they are missing.
>>>
>>> This issue was found with Hector.
>>>
>>> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> 
> Juergen> Nice catch. I think this will need some more work, I'll do a
> Juergen> follow-on patch.
> 
> Juergen> Reviewed-by: Juergen Gross <jgross@suse.com>
> 
> Are you taking this patch through the Xen tree or should I queue it up?

I'm taking it through the xen tree, thanks.


Juergen

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jürgen Groß Nov. 24, 2016, 8:24 a.m. UTC | #4
On 19/11/16 19:22, Quentin Lambert wrote:
> Most error branches following the call to kmalloc contain
> a call to kfree. This patch add these calls where they are
> missing.
> 
> This issue was found with Hector.
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

Applied to xen/tip.git for-linus-4.10


Juergen

> 
> ---
>  drivers/scsi/xen-scsifront.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/scsi/xen-scsifront.c
> +++ b/drivers/scsi/xen-scsifront.c
> @@ -627,6 +627,7 @@ static int scsifront_action_handler(stru
>  
>  	if (scsifront_enter(info)) {
>  		spin_unlock_irq(host->host_lock);
> +		kfree(shadow);
>  		return FAILED;
>  	}
>  
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Nov. 25, 2016, 9:28 p.m. UTC | #5
On Mon, Nov 21, 2016 at 07:01:36AM +0100, Juergen Gross wrote:
> On 19/11/16 19:22, Quentin Lambert wrote:
> > Most error branches following the call to kmalloc contain
> > a call to kfree. This patch add these calls where they are
> > missing.
> > 
> > This issue was found with Hector.
> > 
> > Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> 
> Nice catch. I think this will need some more work, I'll do a
> follow-on patch.

Yeah.  It's weird how we free it on the success path and all the failure
paths except one.  But it looks so deliberate.  What's going on with
that?

Could you send your follow on patch as a reply to the thread?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Dec. 5, 2016, 8:53 p.m. UTC | #6
On Mon, Nov 21, 2016 at 07:01:36AM +0100, Juergen Gross wrote:
> On 19/11/16 19:22, Quentin Lambert wrote:
> > Most error branches following the call to kmalloc contain
> > a call to kfree. This patch add these calls where they are
> > missing.
> > 
> > This issue was found with Hector.
> > 
> > Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> 
> Nice catch. I think this will need some more work, I'll do a
> follow-on patch.
> 

The error handling is really weird.  Could you send your follow on to
this thread?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jürgen Groß Dec. 6, 2016, 5:45 a.m. UTC | #7
On 05/12/16 21:53, Dan Carpenter wrote:
> On Mon, Nov 21, 2016 at 07:01:36AM +0100, Juergen Gross wrote:
>> On 19/11/16 19:22, Quentin Lambert wrote:
>>> Most error branches following the call to kmalloc contain
>>> a call to kfree. This patch add these calls where they are
>>> missing.
>>>
>>> This issue was found with Hector.
>>>
>>> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
>>
>> Nice catch. I think this will need some more work, I'll do a
>> follow-on patch.
>>
> 
> The error handling is really weird.  Could you send your follow on to
> this thread?

I did:

https://lkml.org/lkml/2016/12/2/17


Juergen

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Dec. 6, 2016, 10:22 a.m. UTC | #8
Oops.  Sorry for the noise.

regards,
dan carpenter


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

Patch

--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -627,6 +627,7 @@  static int scsifront_action_handler(stru
 
 	if (scsifront_enter(info)) {
 		spin_unlock_irq(host->host_lock);
+		kfree(shadow);
 		return FAILED;
 	}