On 8/12/2023 12:21 AM, Bryan O'Donoghue wrote:
On 11/08/2023 17:02, Vikash Garodia wrote:
On 8/11/2023 4:11 PM, Bryan O'Donoghue wrote:
On 11/08/2023 09:49, Vikash Garodia wrote:
On 8/11/2023 2:12 PM, Bryan O'Donoghue wrote:
On 11/08/2023 07:04, Vikash Garodia wrote:
On 8/10/2023 5:03 PM, Bryan O'Donoghue wrote: > On 10/08/2023 03:25, Vikash Garodia wrote: >> + if (hweight_long(core->dec_codecs) + hweight_long(core->enc_codecs) > >> MAX_CODEC_NUM) >> + return; >> + > > Shouldn't this be >= ? Not needed. Lets take a hypothetical case when core->dec_codecs has initial 16 (0-15) bits set and core->enc_codecs has next 16 bits (16-31) set. The bit count would be 32. The codec loop after this check would run on caps array index 0-31. I do not see a possibility for OOB access in this case.
> > struct hfi_plat_caps caps[MAX_CODEC_NUM]; > > --- > bod >
Are you not doing a general defensive coding pass in this series ie
"[PATCH v2 2/4] venus: hfi: fix the check to handle session buffer requirement"
In "PATCH v2 2/4", there is a possibility if the check does not consider "=". Here in this patch, I do not see a possibility.
bod
But surely hweight_long(core->dec_codecs) + hweight_long(core->enc_codecs) == MAX_CODEC_NUM is an invalid offset ?
No, it isn't. Please run through the loop with the bitmasks added upto 32 and see if there is a possibility of OOB.
IDK Vikash, the logic here seems suspect.
We have two loops that check for up to 32 indexes per loop. Why not have a capabilities index that can accommodate all 64 bits ?
Max codecs supported can be 32, which is also a very high number. At max the hardware supports 5-6 codecs, including both decoder and encoder. 64 indices is would not be needed.
Why is it valid to have 16 encoder bits and 16 decoder bits but invalid to have 16 encoder bits with 17 decoder bits ? While at the same time valid to have 0 encoder bits but 17 decoder bits ?
The addition of the encoder and decoder should be 32. Any combination which adds to it, would go through. For ex, (17 dec + 15 enc) OR (32 dec + 0 enc) OR (0 dec + 32 enc) etc are valid combination theoretically, though there are only few decoders and encoders actually supported by hardware.
Regards, Vikash