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] [day] [month] [year] [list]
Message-ID: <87y1j9sf9r.fsf@toke.dk>
Date:   Fri, 21 Jul 2023 12:57:52 +0200
From:   Toke Høiland-Jørgensen <toke@...e.dk>
To:     Dongliang Mu <dzm91@...t.edu.cn>,
        Jonas Gorski <jonas.gorski@...il.com>
Cc:     Kalle Valo <kvalo@...nel.org>, linux-wireless@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] wifi: ath9k: fix printk specifier

Dongliang Mu <dzm91@...t.edu.cn> writes:

> On 2023/7/7 21:44, Toke Høiland-Jørgensen wrote:
>> Jonas Gorski <jonas.gorski@...il.com> writes:
>>
>>> On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@...e.dk> wrote:
>>>> Dongliang Mu <dzm91@...t.edu.cn> writes:
>>>>
>>>>> Smatch reports:
>>>>>
>>>>> ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer
>>>>> ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer
>>>>>
>>>>> Fix it by modifying %lx to %p in printk.
>>>>>
>>>>> Signed-off-by: Dongliang Mu <dzm91@...t.edu.cn>
>>>> Third time is the charm! :)
>>>>
>>>> Acked-by: Toke Høiland-Jørgensen <toke@...e.dk>
>>> But do we *want* to replace %lx with %p? Because this now causes a
>>> change in behavior, as the kernel will omit the pointer's address,
>>> while before it printed the actual memory address. Using %px would
>>> have kept the original behavior.
>> So this is basically an informational log message, which really
>> shouldn't be leaking kernel pointer addresses. So I think %p is the
>> right choice here, and if someone wants to see the real address for
>> debugging they should be booting with no_hash_pointers.
>>
>>> So if the change is desired, it should be noted in the commit message.
>> That is a fair point, documenting this in the commit message would be
>> good...
>
> Hi Toke,
>
> What's the next? Patch v4 with a note at the end, like the following,
>
> "Note that %p would by default print a hashed value without leaking the 
> kernel pointer to user space."

Yes, please send a new version with the explanation added. However, this
should include not just *what* is changed, but *why* as well. So maybe
something like:

"Note that with this change, the pointer address will be printed as a
hashed value by default. This is appropriate because the kernel
shouldn't leak kernel pointers to user space in an informational
message. If someone wants to see the real address for debugging
purposes, this can be achieved using the no_hash_pointers kernel option"


-Toke

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ