Re: [PATCH 2/4] media: venus: hfi_parser: avoid OOB access beyond payload word count

["Bryan O'Donoghue" <bryan.odonoghue@xxxxxxxxxx>]

On 11/11/2024 14:36, Vikash Garodia wrote:
> > int hfi_parser(void *buf, int size)
> > {
> >          int word_count = size >> 2;
> >          uint32_t*my_word = (uint32_t*)buf;
> Make this as below and it should lead to OOB
> uint32_t*my_word = (uint32_t*)buf + 1
>
> Regards,
> Vikash

How does this code make sense ?


        while (words_count) {
                data = word + 1;

                switch (*word) {
                case HFI_PROPERTY_PARAM_CODEC_SUPPORTED:
                        parse_codecs(core, data);
                        init_codecs(core);
                        break;
                case HFI_PROPERTY_PARAM_MAX_SESSIONS_SUPPORTED:
                        parse_max_sessions(core, data);
                        break;
                case HFI_PROPERTY_PARAM_CODEC_MASK_SUPPORTED:
                        parse_codecs_mask(&codecs, &domain, data);
                        break;
                case HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SUPPORTED:
                        parse_raw_formats(core, codecs, domain, data);
                        break;
                case HFI_PROPERTY_PARAM_CAPABILITY_SUPPORTED:
                        parse_caps(core, codecs, domain, data);
                        break;
                case HFI_PROPERTY_PARAM_PROFILE_LEVEL_SUPPORTED:
                        parse_profile_level(core, codecs, domain, data);
                        break;
                case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE_SUPPORTED:
                        parse_alloc_mode(core, codecs, domain, data);
                        break;
                default:
                        break;
                }

                word++;
                words_count--;
        }


word[] = { 0, 1, 2, 3 };

words_count = 4;

while(words_count);

	data = word + 1;

	switch(*word) {
	case WHATEVER:
		do_something(param, data);
	}

	word++;
	words_count--;
}

// iteration 0
data = 1;
*word = 0;

// iteration 1
data = 2;
*word = 1;

????

How can the step size of word be correct ?

Do we ever actually process more than one pair here ?

#include <stdio.h>
#include <stdint.h>

char somebuf[16];

void init(char *buf, int len)
{
        int i;
        char c = 0;

        for (i = 0; i < len; i++)
                buf[i] = c++;
}

int hfi_parser(void *buf, int size)
{
        int word_count = size >> 2;
        uint32_t *my_word = (uint32_t*)buf, *data;

        printf("Size %d word_count %d\n", size, word_count);

        while (word_count > 1) {
                data = my_word + 1;
                printf("Myword %d == 0x%08x data=0x%08x\n", word_count, 
*my_word, *data);
                my_word++;
                word_count--;
        }
}

int main(int argc, char *argv[])
{
        int i;

        init(somebuf, sizeof(somebuf));
        for (i = 0; i < sizeof(somebuf); i++)
                printf("%x = %x\n", i, somebuf[i]);

        hfi_parser(somebuf, sizeof(somebuf));

        return 0;
}

0 = 0
1 = 1
2 = 2
3 = 3
4 = 4
5 = 5
6 = 6
7 = 7
8 = 8
9 = 9
a = a
b = b
c = c
d = d
e = e
f = f
Size 16 word_count 4
Myword 4 == 0x03020100 data=0x07060504
Myword 3 == 0x07060504 data=0x0b0a0908
Myword 2 == 0x0b0a0908 data=0x0f0e0d0c

---
bod