linux-kernel - Re: [PATCH 3/3] hid: return -ENOMEM when memory allocation fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 21 Jan 2011 10:52:14 +0100 (CET)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Davidlohr Bueso <dave@....org>
Cc:	linux-input@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] hid: return -ENOMEM when memory allocation fails

On Thu, 20 Jan 2011, Davidlohr Bueso wrote:

> From: Davidlohr Bueso <dave@....org>
> 
> Signed-off-by: Davidlohr Bueso <dave@....org>
> ---
>  drivers/hid/hid-core.c      |    2 +-
>  drivers/hid/usbhid/hiddev.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index d678cf3..eb2251c 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -129,7 +129,7 @@ static int open_collection(struct hid_parser *parser, unsigned type)
>  				parser->device->collection_size * 2, GFP_KERNEL);
>  		if (collection == NULL) {
>  			dbg_hid("failed to reallocate collection array\n");
> -			return -1;
> +			return -ENOMEM;
>  		}

That's not really complete, as the return value will be lost at caller 
anyway, see hid_parse_report():

                if (dispatch_type[item.type](parser, &item)) {
                        dbg_hid("item %u %u %u %u parsing failed\n",
                                item.format, (unsigned)item.size,
                                (unsigned)item.type, (unsigned)item.tag);
                        goto err;
                }

Could you please re-spin the patch and add also the return value 
propagation here as well?


> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index af0a7c1..38513cc 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -894,7 +894,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>  	}
>  
>  	if (!(hiddev = kzalloc(sizeof(struct hiddev), GFP_KERNEL)))
> -		return -1;
> +		return -ENOMEM;

The same here. The actual return value from hiddev_connect() is being 
checked by the caller only whether it's 0 (success) or not. See 
hid_connect():

        if ((connect_mask & HID_CONNECT_HIDDEV) && hdev->hiddev_connect &&
                        !hdev->hiddev_connect(hdev,
                                connect_mask & HID_CONNECT_HIDDEV_FORCE))
                hdev->claimed |= HID_CLAIMED_HIDDEV;


-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/