Re: [PATCH v2] kbuild: treat char as always unsigned

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Jason A. Donenfeld" <Jason@xxxxxxxxx> writes:

> Hi Linus,
>
> On Mon, Oct 24, 2022 at 7:11 PM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>> IOW, I don't think these are 6.1 material as some kind of obvious
>> fixes, at least not without driver author acks.
>
> Right, these are posted to the authors and maintainers to look at.
> Maybe they punt them until 6.2 which would be fine too.
>
>> On Mon, Oct 24, 2022 at 9:34 AM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
>> Some of those may need more thought. For example, that first one:
>>
>> > https://lore.kernel.org/all/20221024163005.536097-1-Jason@xxxxxxxxx
>>
>> looks just *strange*. As far as I can tell, no other wireless drivers
>> do any sign checks at all.
>>
>> Now, I didn't really look around a lot, but looking at a few other
>> SIOCSIWESSID users, most don't even seem to treat it as a string at
>> all, but as just a byte dump (so memcpy() instead of strncpy())

Yes, SSID should be handled as a byte array with a specified length.
Back in the day some badly written code treated it as string but luckily
it's rare now.

>> As far as I know, there are no actual rules for SSID character sets,
>> and while using utf-8 or something else might cause interoperability
>> problems, this driver seems to be just confused. If you want to check
>> for "printable characters", that check is still wrong.
>>
>> So I don't think this is a "assume char is signed" issue. I think this
>> is a "driver is confused" issue.
>
> Yea I had a few versions of this. In one of them, I changed `char
> *extra` throughout the wireless stack into `s8 *extra` and in another
> `u8 *extra`, after realizing they're mostly just bags of bits. But
> that seemed pretty invasive when, indeed, this staging driver is just
> a little screwy.
>
> So perhaps the right fix is to just kill that whole snippet? Kalle - opinions?

I would also remove the whole 'extra[i] < 0', seems like a pointless
check to me. And I see that you already submitted v2, good.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux