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
