[FFmpeg-devel] [PATCH] avcodec/hevc_ps: fix the problem of memcmp losing effectiveness

[Andreas Rheinhardt]

James Almer:
> On 3/29/2024 10:10 AM, Mark Thompson wrote:
>> On 28/03/2024 13:15, tong1.wu-at-intel.com at ffmpeg.org wrote:
>>> From: Tong Wu <tong1.wu at intel.com>
>>>
>>> HEVCHdrParams* receives a pointer which points to a dynamically
>>> allocated memory block. It causes the memcmp always returning 1.
>>> Add a function to do the comparision. A condition is also added to
>>> avoid malloc(0).
>>>
>>> Signed-off-by: Tong Wu <tong1.wu at intel.com>
>>> ---
>>>   libavcodec/hevc_ps.c | 20 ++++++++++++++++----
>>>   libavcodec/hevc_ps.h |  4 +++-
>>>   2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> It doesn't seem like this method works at all, even before the recent
>> change with the pointer.
>>
>> Structs can contain arbitrary padding, and any write to the struct
>> makes the padding unspecified.  memcmp() is therefore never valid as a
>> method of comparing after writing some fields, as done here.  (It
>> could only be valid if the structs compared were made by memcpy() with
>> no fields written directly.)
> 
> The struct is zero allocated, so shouldn't the padding be exactly the
> same for two equal VPSs after parsing?
> 

In practice it is (and the current code already relied on this); yet as
has already been said padding bytes take unspecified values at any store
(to any member). In practice, if the compiler uses instructions that
clobber the padding, the padding in both structs is clobbered in the
same way.

- Andreas