linux-kernel - Re: Linux 5.18-rc1

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]

Date:   Sun, 3 Apr 2022 21:23:07 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Larry Finger <Larry.Finger@...inger.net>,
        Oded Gabbay <ogabbay@...nel.org>, Jiri Slaby <jslaby@...e.cz>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: Linux 5.18-rc1

On 4/3/22 20:29, Linus Torvalds wrote:
> On Sun, Apr 3, 2022 at 7:22 PM Guenter Roeck <linux@...ck-us.net> wrote:
>>
>> In function '__nat25_add_pppoe_tag',
>>      inlined from 'nat25_db_handle' at drivers/staging/r8188eu/core/rtw_br_ext.c:479:11:
>> arch/alpha/include/asm/string.h:22:16: error: '__builtin_memcpy' forming offset [40, 2051] is out of the bounds [0, 40] of object 'tag_buf' with type 'unsigned char[40]'
>>
>> Exposed by commit e6148767825c ("Makefile: Enable -Warray-bounds").
>> Fix at https://lore.kernel.org/lkml/20220403123628.3113382-1-linux@roeck-us.net/
> 
> Funky. Apparently nobody else does that pppoe_tag thing, and this
> driver does it wrong on little-endian, which is the common thing to
> test.
> 
> Your email that you point to is a bit confused, though, in how it says
> "when building the driver on a big endian system such as alpha".
> 
> Alpha is little-endian, not big-endian.
> 

Oops. Sorry, I thought it was big endian. No idea why. I'll update
subject and description and resend.

> Now, why it apparently only warns on alpha, I have absolutely no idea.
> It should warn on other things too afaik, since that
> 
>          tag->tag_len = htons(MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN+old_tag_len);
> 
> should be visible not just on alpha.
> 
Maybe htons() and ntohs() are modeled differently on other architectures,
and the compiler doesn't see the context ?

> Weird. But your patch looks correct.
> 
>> Building arm:allmodconfig ... failed
>> Building csky:allmodconfig ... failed
>> Building i386:allyesconfig ... failed
>> Building mips:allmodconfig ... failed
>> Building parisc:allmodconfig ... failed
>> Building powerpc:ppc32_allmodconfig ... failed
>> Building xtensa:allmodconfig ... failed
>> --------------
>> Error log:
>> drivers/misc/habanalabs/common/memory.c: In function 'alloc_device_memory':
>> drivers/misc/habanalabs/common/memory.c:153:49: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>>    153 |                                                 (u64) gen_pool_dma_alloc_align(vm->dram_pg_pool,
>>
>> Fix at https://lore.kernel.org/lkml/20220401151450.3414694-1-linux@roeck-us.net/
> 
> Gaah - both of those "(u64)" look pointless.
> 
> Either the thing is a pointer, in which case that (uinptr_t) - or just
> (unsigned long) - is the right thing to do, or it's already an integer
> type, in which case castring it to (u64) just do do that
> 
>          phys_pg_pack->pages[i] =  ...
> 
> assignment seems entirely pointless.
> 
> So I think the patch should also remove those pointless (u64) casts.
> 
> Yes, yes, the pages[] array in 'struct hl_vm_phys_pg_pack' 'pages[]'
> is of u64's, but casting integers like that is just silly.
> 

Double casts are quite common, though. Try 'git grep "(u64)(uintptr_t)"'.
But I think you are right, a cast to (uintptr_t) should be sufficient here.
I'll resend that patch as well.

Guenter