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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <474d3c62-5747-45b9-b5c3-253607b0c17a@linaro.org>
Date: Tue, 5 Nov 2024 11:15:19 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Vikash Garodia <quic_vgarodia@...cinc.com>,
 Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 2/4] media: venus: hfi_parser: avoid OOB access beyond
 payload word count

On 05/11/2024 08:54, Vikash Garodia wrote:
> words_count denotes the number of words in total payload, while data
> points to payload of various property within it. When words_count
> reaches last word, data can access memory beyond the total payload.
> Avoid this case by not allowing the loop for the last word count.
> 
> Cc: stable@...r.kernel.org
> Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser")
> Signed-off-by: Vikash Garodia <quic_vgarodia@...cinc.com>
> ---
>   drivers/media/platform/qcom/venus/hfi_parser.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
> index 27d0172294d5154f4839e8cef172f9a619dfa305..20d9ea3626e9c4468d5f7dbd678743135f027c86 100644
> --- a/drivers/media/platform/qcom/venus/hfi_parser.c
> +++ b/drivers/media/platform/qcom/venus/hfi_parser.c
> @@ -303,7 +303,7 @@ u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf,
>   		memset(core->caps, 0, sizeof(core->caps));
>   	}
>   
> -	while (words_count) {
> +	while (words_count > 1) {
>   		data = word + 1;
>   
>   		switch (*word) {
> 

How is it the right thing to do to _not_ process the last u32 ?

How does this overrun ? while (words_count) should be fine because it 
decrements at the bottom of the loop...

assuming your buffer is word aligned obvs

=>

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

char somebuf[64];

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;

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

         while(word_count) {
                 printf("Myword %d == 0x%08x\n", word_count, *my_word);
                 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
10 = 10
11 = 11
12 = 12
13 = 13
14 = 14
15 = 15
16 = 16
17 = 17
18 = 18
19 = 19
1a = 1a
1b = 1b
1c = 1c
1d = 1d
1e = 1e
1f = 1f
20 = 20
21 = 21
22 = 22
23 = 23
24 = 24
25 = 25
26 = 26
27 = 27
28 = 28
29 = 29
2a = 2a
2b = 2b
2c = 2c
2d = 2d
2e = 2e
2f = 2f
30 = 30
31 = 31
32 = 32
33 = 33
34 = 34
35 = 35
36 = 36
37 = 37
38 = 38
39 = 39
3a = 3a
3b = 3b
3c = 3c
3d = 3d
3e = 3e
3f = 3f
Size 64 word_count 16
Myword 16 == 0x03020100
Myword 15 == 0x07060504
Myword 14 == 0x0b0a0908
Myword 13 == 0x0f0e0d0c
Myword 12 == 0x13121110
Myword 11 == 0x17161514
Myword 10 == 0x1b1a1918
Myword 9 == 0x1f1e1d1c
Myword 8 == 0x23222120
Myword 7 == 0x27262524
Myword 6 == 0x2b2a2928
Myword 5 == 0x2f2e2d2c
Myword 4 == 0x33323130
Myword 3 == 0x37363534
Myword 2 == 0x3b3a3938
Myword 1 == 0x3f3e3d3c

---
bod