On Sun, Jan 26, 2014 at 05:21:34PM +0100, Christian Engelmayer wrote:
> Fix a memory leak in the usb_store_new_id() error paths. When bailing out
> due to sanity checks, the function left the already allocated usb_dynid
> struct in place. This regression was introduced by the following commits:
>
> c63fe8f6 (usb: core: add sanity checks when using bInterfaceClass with new_id)
> 1b9fb31f (usb: core: check for valid id_table when using the RefId feature)
> 52a6966c (usb: core: bail out if user gives an unknown RefId when using new_id)
>
> Detected by Coverity: CID 1162604.
>
> Signed-off-by: Christian Engelmayer <cengelma@xxxxxx>
> ---
> v2: Added changes suggested by Sergei Shtylyov:
>
> * Fix coding style violation regarding usage of braces in if-statements
> if only one branch of a conditional statement is a single statement.
> * Keep the information on commits introducing the issue in the changelog.
> ---
> drivers/usb/core/driver.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 5d01558..8fac6c6 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -63,8 +63,10 @@ ssize_t usb_store_new_id(struct usb_dynids *dynids,
> dynid->id.idProduct = idProduct;
> dynid->id.match_flags = USB_DEVICE_ID_MATCH_DEVICE;
> if (fields > 2 && bInterfaceClass) {
> - if (bInterfaceClass > 255)
> + if (bInterfaceClass > 255) {
> + kfree(dynid);
> return -EINVAL;
> + }
I'd prefer a
retval = -Esomething;
goto error;
approach for the errors. I probably found another case to handle which
would benefit from the additional exit path as well.
Thanks for picking this issue up.
Attachment:
signature.asc
Description: Digital signature
