| Message ID | 20231221080858.12876-1-abelova@astralinux.ru |
|---|---|
| State | New |
| Headers | show |
| Series | load_elf: fix iterator type in glue | expand |
Hi, On 21/12/23 09:08, Anastasia Belova wrote: > file_size is uint32_t, so j < file_size should be > uint32_t too. file_size is of elf_word type, which is either uint32_t or uint64_t. Your explanation is not very clear... Maybe you want an unsigned type? In that case, does the following makes your sanitizer happier? -- >8 -- diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 0a5c258fe6..03eba78c6e 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -502,4 +502,3 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd, if (data_swab) { - int j; - for (j = 0; j < file_size; j += (1 << data_swab)) { + for (unsigned j = 0; j < file_size; j += (1 << data_swab)) { uint8_t *dp = data + j; --- > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf") > Signed-off-by: Anastasia Belova <abelova@astralinux.ru> > --- > include/hw/elf_ops.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index 0a5c258fe6..1defccaa71 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -500,7 +500,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd, > } > > if (data_swab) { > - int j; > + uint32_t j; > for (j = 0; j < file_size; j += (1 << data_swab)) { > uint8_t *dp = data + j; > switch (data_swab) {
On Tue, 26 Dec 2023 at 12:04, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Hi, > > On 21/12/23 09:08, Anastasia Belova wrote: > > file_size is uint32_t, so j < file_size should be > > uint32_t too. > > file_size is of elf_word type, which is either uint32_t > or uint64_t. > > Your explanation is not very clear... Maybe you want an unsigned type? > In that case, does the following makes your sanitizer happier? Since file_size is type 'elf_word', the iterator 'j' should be the matching type. In practice nobody is loading 2GB ELF files, so we don't really run into this, but it is a bug. I agree with Philippe that it would be helpful if the commit message: * is clear about the problem it is fixing * describes whether there are any real-world consequences of the issue and how severe (or not) they are thanks -- PMM
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 0a5c258fe6..1defccaa71 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -500,7 +500,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd, } if (data_swab) { - int j; + uint32_t j; for (j = 0; j < file_size; j += (1 << data_swab)) { uint8_t *dp = data + j; switch (data_swab) {
file_size is uint32_t, so j < file_size should be uint32_t too. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf") Signed-off-by: Anastasia Belova <abelova@astralinux.ru> --- include/hw/elf_ops.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)