Bug report - GZ magic check always fails in uImage_gz_load

Bug report - GZ magic check always fails in uImage_gz_load

[McLaughlin Amy]

I tried adding -Wextra to Kexec's makefile, while compiling some work by a colleague that built on the kexec source. It spotted the following:

```
kexec/kexec-uImage.c: In function 'uImage_gz_load':
kexec/kexec-uImage.c:160:31: warning: comparison is always true due to limited range of data type [-Wtype-limits]
  160 |  if (buf[0] != 0x1f || buf[1] != 0x8b) {
      |                               ^~
```

With buf being an array of signed char, buf[1] cannot equal 0x8b without a cast, meaning that this function will always return -1 early on. Maybe representing it as an array of uint8_t/unsigned char would have been a better idea?

Re: Bug report - GZ magic check always fails in uImage_gz_load

[Simon Horman]

On Thu, Nov 21, 2024 at 02:24:47PM +0000, McLaughlin Amy wrote:
> I tried adding -Wextra to Kexec's makefile, while compiling some work by a colleague that built on the kexec source. It spotted the following:
> 
> ```
> kexec/kexec-uImage.c: In function 'uImage_gz_load':
> kexec/kexec-uImage.c:160:31: warning: comparison is always true due to limited range of data type [-Wtype-limits]
>   160 |  if (buf[0] != 0x1f || buf[1] != 0x8b) {
>       |                               ^~
> ```
> 
> With buf being an array of signed char, buf[1] cannot equal 0x8b without a cast, meaning that this function will always return -1 early on. Maybe representing it as an array of uint8_t/unsigned char would have been a better idea?

Hi Amy,

Yes, I agree that seems wrong and that your proposed solution sounds good.

But I do wonder if this code has been exercised. Since as you point out,
it always returns early. I'm guessing no-one is using gzipped uImages.
So perhaps we could consider a more radical approach of removing support
for those images?

In any case, could you consider sending a patch to fix the problem
you've found?