Re: [PATCH v3 1/2] media: venus: fix TOCTOU vulnerability when reading packets from shared memory

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 15/05/2025 14:23, Vikash Garodia wrote:
>> Re-reading to see if the firmware wrote new bad data to the transmitted packet
>> in-memory is not a fix before or after the memcpy() because the time you do that
>> re-read is not fixed - locked wrt the freerunning firmware.
> It would be more meaningful if you can suggest the vulnerability you see with
> the changes suggested i.e adding the check in local packet against the size read
> from shared queue. Based on that we can see how to fix it, otherwise this
> discussion in not leading to any conclusion.

So to re-iterate.

TOCTOU is this

if (*ptr_val >> 2 >= MAX)
     return -EBAD;

memcpy(dst, src, *ptr_val >> 2);

Here a malicious actor can change *ptr_val between our check and our use.

not

data_value = *ptr_val >> 2;

if (data_value >= MAX)
     return -EBAD;

memcpy(dst, src, data_value);

Here the taking a copy of the value and subsequently relying on that 
value mitigates TOCTOU, because the value *ptr_val is latched - read 
into a local variable which cannot be manipulated from an outside agent 
i.e. venus firmware.

The example in the commit log is not a TOCTOU for that reason.

Adding an additional check _after_ the memcpy() seems silly to me because

data_value = *ptr_val >> 2;

if (data_value >= MAX)
     return -EBAD;

memcpy(dst, src, data_value);

// This statement could be false
if (data_value != *ptr_value >> 2)
     return -EBAD;

// while this subsequent statement is true
if (data_value != *ptr_value >> 2)
     return -EBAD;

And in any case this is a post-use sanity check not a mitigation for 
TOCTOU which we don't have.

---
bod