linux-kernel - Re: [PATCH 2/2] Bluetooth: Replaces printk with pr_debug in bt

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: <2023110752-laundry-stiffness-9f34@gregkh>
Date:   Tue, 7 Nov 2023 19:13:13 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Yuran Pereira <yuran.pereira@...mail.com>
Cc:     linux-bluetooth@...r.kernel.org, johan.hedberg@...il.com,
        marcel@...tmann.org, linux-kernel@...r.kernel.org,
        luiz.dentz@...il.com,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH 2/2] Bluetooth: Replaces printk with pr_debug in bt_dbg

On Tue, Nov 07, 2023 at 09:32:51PM +0530, Yuran Pereira wrote:
> Hello Greg,
> On Tue, Nov 07, 2023 at 07:31:27AM +0100, Greg KH wrote:
> > 
> > You might have just changed the functionality here, are you SURE this is
> > identical to the original code?  How was it tested?
> > 
> > I'm not saying this is a bad idea to do, just be aware of the
> > consequences for this change and document it properly (hint, the
> > changelog does not document the user-visible change that just happened.)
> > 
> > Note, pr_debug() is NOT identical to printk(), look at the source for
> > the full details.
> > 
> 
> Thank you for the heads-up. 
> Yes, you're right.
> 
> I just took another look and it seems that using pr_debug here
> does defeat the purpose of bt_dbg which was created for situations
> where `DYNAMIC_DEBUG` and `DEBUG` are disabled.
> 
> The likely equivalent would have been `pr_devel` but that also
> depends on `DEBUG`.
> 
> Do you think that a new `pr_devel_uncond` like the one below
> (only to be used in exceptional scenarios) would be a good idea?
> 
> ```
> #define pr_devel_uncond(fmt, ...) \
>     printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> ```
> 
> This would neither depend on `DYNAMIC_DEBUG` nor on `DEBUG`.

No, not at all, the bluetooth subsystem should move to actually use the
proper dynamic debug infrastructure and not have their own "special"
subsystem loging macros/functions.  What you are doing here is the
proper way forward, BUT you need to make everyone aware that it is going
to change how things work from what they do today.

In other words, it's not just a "trivial" change, you need to get
approval to change this type of functionality from the Bluetooth
developers/maintainers.

thanks,

greg k-h