Am 25.11.2016 14:57, schrieb Laurent Pinchart:
> Hi Dan,
>
> Thank you for the patch.
>
> On Friday 25 Nov 2016 13:28:35 Dan Carpenter wrote:
>> A recent cleanup introduced a potential dereference of -EFAULT when we
>> call kfree(map->menu_info).
>
> I should have caught that, my apologies :-(
>
> Thinking a bit more about this class of problems, would the following patch
> make sense ?
>
> commit 034b71306510643f9f059249a0c14418099eb436
> Author: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Date: Fri Nov 25 15:54:22 2016 +0200
>
> mm/slab: WARN_ON error pointers passed to kfree()
>
> Passing an error pointer to kfree() is invalid and can lead to crashes
> or memory corruption. Reject those pointers and WARN in order to catch
> the problems and fix them in the callers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 0b0550ca85b4..a7eb830c6684 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3819,6 +3819,8 @@ void kfree(const void *objp)
>
> if (unlikely(ZERO_OR_NULL_PTR(objp)))
> return;
> + if (WARN_ON(IS_ERR(objp)))
> + return;
> local_irq_save(flags);
> kfree_debugcheck(objp);
> c = virt_to_cache(objp);
I this is better in kfree_debugcheck().
1. it has the right name
2. is contains already a check
static void kfree_debugcheck(const void *objp)
{
if (!virt_addr_valid(objp)) {
pr_err("kfree_debugcheck: out of range ptr %lxh\n",
(unsigned long)objp);
BUG();
}
}
btw: should this read %p ?
re,
wh
>> Fixes: 4cc5bed1caeb ("[media] uvcvideo: Use memdup_user() rather than
>> duplicating its implementation")
>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Mauro, the bug is present in your branch only at the moment and queued for
> v4.10. Could you please pick this patch up as well ?
>
>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> b/drivers/media/usb/uvc/uvc_v4l2.c index a7e12fd..3e7e283 100644
>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -66,14 +66,14 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
>> *chain, if (xmap->menu_count == 0 ||
>> xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) {
>> ret = -EINVAL;
>> - goto done;
>> + goto free_map;
>> }
>>
>> size = xmap->menu_count * sizeof(*map->menu_info);
>> map->menu_info = memdup_user(xmap->menu_info, size);
>> if (IS_ERR(map->menu_info)) {
>> ret = PTR_ERR(map->menu_info);
>> - goto done;
>> + goto free_map;
>> }
>>
>> map->menu_count = xmap->menu_count;
>> @@ -83,13 +83,13 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
>> *chain, uvc_trace(UVC_TRACE_CONTROL, "Unsupported V4L2 control type "
>> "%u.\n", xmap->v4l2_type);
>> ret = -ENOTTY;
>> - goto done;
>> + goto free_map;
>> }
>>
>> ret = uvc_ctrl_add_mapping(chain, map);
>>
>> -done:
>> kfree(map->menu_info);
>> +free_map:
>> kfree(map);
>>
>> return ret;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
