RE: [PATCH 1/3] libusbg: Fix potential memory leak in usbg_init()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> -----Original Message-----
> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Sergei Shtylyov
> Sent: Tuesday, June 10, 2014 6:42 PM
> To: Krzysztof Opasiak; mporter@xxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx; philippedeswert@xxxxxxxxx; Stanislaw
> Wadas; Andrzej Pietrasiewicz; Karol Lewandowski; Marek Szyprowski;
> Robert Baldyga
> Subject: Re: [PATCH 1/3] libusbg: Fix potential memory leak in
> usbg_init()
> 
> On 06/10/2014 08:34 PM, Krzysztof Opasiak wrote:
> 
> [...]
> 
> >>> Memory allocated with asprintf() for variable path
> >>> could be not free() in some cases. Fix this issue by
> >>> doing some small refactoring.
> 
> >>> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
> >>> ---
> >>>    src/usbg.c |   34 +++++++++++++++++++++++-----------
> >>>    1 file changed, 23 insertions(+), 11 deletions(-)
> 
> >>> diff --git a/src/usbg.c b/src/usbg.c
> >>> index edb7fc3..054c603 100644
> >>> --- a/src/usbg.c
> >>> +++ b/src/usbg.c
> >> [...]
> >>> @@ -1227,21 +1228,32 @@ int usbg_init(char *configfs_path,
> >> usbg_state **state)
> >> [...]
> >>> +	ret = usbg_init_state(path, s);
> >>> +	if (ret != USBG_SUCCESS) {
> >>> +		ERRORNO("couldn't init gadget state\n");
> >>> +		usbg_free_state(s);
> >>> +		goto out;
> >>> +	}
> >>> +
> >>> +	*state = s;
> >>> +out:
> >>> +	return ret;
> >>> +
> >>> +err:
> >>> +	free(path);
> 
> >>      Why not put the 'out' label here and not duplicate
> *return*?
> 
> > That's because free(path) should not be executed in some cases.
> Function
> > usbg_init_state() takes the ownership of memory allocated for
> path and
> > it is free() along with usbg_state structure in
> usbg_free_state(). So
> > after usbg_init_state() call we should use usbg_free_state and
> not free
> > the path directly.
> 
>     Ah, you can't un-duplicate *return* indeed (sorry about that)
> but still
> the 'out' label used for the failure path fits better here than
> above.
> 
> WBR, Sergei

Yeah, definitely true because goto out; is executed in error context.
I'll fix this for v2. Thanks.

I have also noticed now that I didn't update show-gadgets example to add
there support for F_FFS. This one will also be fixed in v2.

--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@xxxxxxxxxxx





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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux