On 3/5/19 8:39 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Tue, Mar 05, 2019 at 10:58:42AM +0100, hverkuil-cisco@xxxxxxxxx wrote:
>> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>>
>> Ensure that this pointer is set to NULL after it is freed.
>> The vimc driver has a static media_entity and after
>> unbinding and rebinding the vimc device the media code will
>> try to free this pointer again since it wasn't set to NULL.
>
> I still think the problem lies in the vimc driver. Reusing static
> structures is really asking for trouble. I'm however not opposed to
> merging this patch, as you mentioned the problem will be fixed in vimc
> too. I still wonder, though, if merging this couldn't make it easier for
> drivers to do the wrong thing.
I'm keeping this patch :-)
I don't think that what vimc is doing is wrong in principle, just very unusual.
Also I think it makes the mc framework more robust by properly zeroing this
pointer.
Regards,
Hans
>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>> ---
>> drivers/media/media-entity.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>> index 0b1cb3559140..7b2a2cc95530 100644
>> --- a/drivers/media/media-entity.c
>> +++ b/drivers/media/media-entity.c
>> @@ -88,6 +88,7 @@ EXPORT_SYMBOL_GPL(__media_entity_enum_init);
>> void media_entity_enum_cleanup(struct media_entity_enum *ent_enum)
>> {
>> kfree(ent_enum->bmap);
>> + ent_enum->bmap = NULL;
>> }
>> EXPORT_SYMBOL_GPL(media_entity_enum_cleanup);
>>
>
