linux-kernel - Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 5 May 2020 17:38:36 +0200
From:   Neil Armstrong <narmstrong@...libre.com>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc:     linux-media@...r.kernel.org, hverkuil-cisco@...all.nl,
        linux-amlogic@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Maxime Jourdan <mjourdan@...libre.com>
Subject: Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer
 shortage

On 05/05/2020 17:18, Mauro Carvalho Chehab wrote:
> Em Tue, 28 Apr 2020 14:50:35 +0200
> Neil Armstrong <narmstrong@...libre.com> escreveu:
> 
>> From: Maxime Jourdan <mjourdan@...libre.com>
>>
>> - Redo the logic where VP9 gets fresh CAPTURE buffers. The previous code
>>   could lead to a hardlock.
>> - Reserve 4 margin buffers instead of 3, as apparently there are corner
>>   cases where 3 is not enough.
>>
>> Fixes: e9a3eb4819ca ("media: meson: vdec: add VP9 input support")
>> Fixes: 00c43088aa68 ("media: meson: vdec: add VP9 decoder support")
>> Signed-off-by: Maxime Jourdan <mjourdan@...libre.com>
>> Signed-off-by: Neil Armstrong <narmstrong@...libre.com>
> 
>> +static struct vb2_v4l2_buffer *get_free_vbuf(struct amvdec_session *sess)
>> +{
>> +	struct codec_vp9 *vp9 = sess->priv;
>> +	struct vb2_v4l2_buffer *vbuf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx);
>> +	struct vb2_v4l2_buffer *vbuf2;
>> +
>> +	if (!vbuf)
>> +		return NULL;
>> +
>> +	if (!codec_vp9_get_frame_by_idx(vp9, vbuf->vb2_buf.index))
>> +		return vbuf;
>> +
>> +	vbuf2 = get_free_vbuf(sess);
> 
> Huh!!!!
> 
> Never use recursive functions inside the Kernel! Kernel stack is too
> limited.
> 
> Also, even if Kernel stack would be unlimited, the above logic
> would endlessly be calling get_free_vbuf(sess).

Will rework this correctly, thanks for the review.

Neil

> 
> Thanks,
> Mauro
>