[PATCH v3] kernel-device: expect non-NULL 'vendor' and 'product' argument in get_device_ids()

[Aleksander Morgado]

On 15/08/17 18:10, Ben Chan wrote:
> get_device_ids() in mm-kernel-device-udev.c accepts a NULL 'vendor' or
> 'product' argument, but the current implementation could result in a
> potential NULL dereferences of the 'vendor' argument. Given that
> get_device_ids() is a local helper and its only caller provides a
> non-NULL 'vendor' and 'product' argument, this patch removes the NULL
> checks (i.e. get_device_ids() expects non-NULL 'vendor' and 'product').
> 
> This patch also rearranges the code such that the 'vendor' argument is
> updated only when the function returns TRUE, just like how the 'product'
> argument is handled.

Thanks, pushed to git master. I also pushed a follow up commit to assert the parameters being non-NULL.

> ---
>  src/kerneldevice/mm-kernel-device-udev.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/src/kerneldevice/mm-kernel-device-udev.c b/src/kerneldevice/mm-kernel-device-udev.c
> index e8763f7e..b9f057bd 100644
> --- a/src/kerneldevice/mm-kernel-device-udev.c
> +++ b/src/kerneldevice/mm-kernel-device-udev.c
> @@ -119,29 +119,21 @@ get_device_ids (GUdevDevice *device,
>      if (strlen (vid) != 4)
>          goto out;
>  
> -    if (vendor) {
> -        *vendor = (guint16) (mm_utils_hex2byte (vid + 2) & 0xFF);
> -        *vendor |= (guint16) ((mm_utils_hex2byte (vid) & 0xFF) << 8);
> -    }
> -
>      if (!pid)
>          pid = g_udev_device_get_property (device, "ID_MODEL_ID");
> -    if (!pid) {
> -        *vendor = 0;
> +    if (!pid)
>          goto out;
> -    }
>  
>      if (strncmp (pid, "0x", 2) == 0)
>          pid += 2;
> -    if (strlen (pid) != 4) {
> -        *vendor = 0;
> +    if (strlen (pid) != 4)
>          goto out;
> -    }
>  
> -    if (product) {
> -        *product = (guint16) (mm_utils_hex2byte (pid + 2) & 0xFF);
> -        *product |= (guint16) ((mm_utils_hex2byte (pid) & 0xFF) << 8);
> -    }
> +    *vendor = (guint16) (mm_utils_hex2byte (vid + 2) & 0xFF);
> +    *vendor |= (guint16) ((mm_utils_hex2byte (vid) & 0xFF) << 8);
> +
> +    *product = (guint16) (mm_utils_hex2byte (pid + 2) & 0xFF);
> +    *product |= (guint16) ((mm_utils_hex2byte (pid) & 0xFF) << 8);
>  
>      success = TRUE;
>  
> 


-- 
Aleksander
https://aleksander.es