linux-kernel - Re: [PATCH] media: cedrus: hevc: Add check for invalid timestamp

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d58e8624e9703e5dbbc54908aa142d10ef7f5a22.camel@collabora.com>
Date:   Mon, 18 Jul 2022 13:41:48 -0400
From:   Nicolas Dufresne <nicolas.dufresne@...labora.com>
To:     Jernej Skrabec <jernej.skrabec@...il.com>, mripard@...nel.org,
        paul.kocialkowski@...tlin.com
Cc:     mchehab@...nel.org, gregkh@...uxfoundation.org, wens@...e.org,
        samuel@...lland.org, hverkuil-cisco@...all.nl,
        ezequiel@...guardiasur.com.ar, linux-media@...r.kernel.org,
        linux-staging@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: cedrus: hevc: Add check for invalid timestamp

Le lundi 18 juillet 2022 à 18:56 +0200, Jernej Skrabec a écrit :
> Not all DPB entries will be used most of the time. Unused entries will
> thus have invalid timestamps. They will produce negative buffer index
> which is not specifically handled. This works just by chance in current
> code. It will even produce bogus pointer, but since it's not used, it
> won't do any harm.
> 
> Let's fix that brittle design by skipping writing DPB entry altogether
> if timestamp is invalid.
> 
> Fixes: 86caab29da78 ("media: cedrus: Add HEVC/H.265 decoding support")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@...il.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> index 1afc6797d806..687f87598f78 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -147,6 +147,9 @@ static void cedrus_h265_frame_info_write_dpb(struct cedrus_ctx *ctx,
>  			dpb[i].pic_order_cnt_val
>  		};
>  
> +		if (buffer_index < 0)
> +			continue;

When I compare to other codecs, when the buffer_index does not exist, the addr 0
is being programmed into the HW. With this implementation is is left to whatever
it was set for the previous decode operation. I think its is nicer done the
other way.

> +
>  		cedrus_h265_frame_info_write_single(ctx, i, dpb[i].field_pic,
>  						    pic_order_cnt,
>  						    buffer_index);