linux-kernel - Re: [PATCH] iommu/amd: Fix extended features logging

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.20.13.2104111410340.11104@monopod.intra.ispras.ru>
Date:   Sun, 11 Apr 2021 14:22:14 +0300 (MSK)
From:   Alexander Monakov <amonakov@...ras.ru>
To:     Paul Menzel <pmenzel@...gen.mpg.de>
cc:     Joerg Roedel <jroedel@...e.de>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        iommu@...ts.linux-foundation.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iommu/amd: Fix extended features logging

On Sun, 11 Apr 2021, Paul Menzel wrote:

> > The second line is emitted via 'pr_cont', which causes it to have a
> > different ('warn') loglevel compared to the previous line ('info').
> > 
> > Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
> > from the pci_info format string, but this doesn't work, as pci_info
> > calls implicitly append a newline anyway.
> 
> Hmm, did I really screw that up during my testing? I am sorry about that.
> 
> I tried to wrap my head around, where the newline is implicitly appended, and
> only found the definitions below.
> 
>     include/linux/pci.h:#define pci_info(pdev, fmt, arg...)
> dev_info(&(pdev)->dev, fmt, ##arg)
> 
>     include/linux/dev_printk.h:#define dev_info(dev, fmt, ...)
> \
>     include/linux/dev_printk.h:     _dev_info(dev, dev_fmt(fmt),
> ##__VA_ARGS__)
> 
>     include/linux/dev_printk.h:__printf(2, 3) __cold
>     include/linux/dev_printk.h:void _dev_info(const struct device *dev, const
> char *fmt, ...);
> 
>     include/linux/compiler_attributes.h:#define __printf(a, b)
> __attribute__((__format__(printf, a, b)))

Yeah, it's not obvious: it happens in kernel/printk/printk.c:vprintk_store
where it does

	if (dev_info)
		lflags |= LOG_NEWLINE;

It doesn't seem to be documented; I think it prevents using pr_cont with
"rich" printk facilities that go via _dev_info.

I suspect it quietly changed in commit c313af145b9bc ("printk() - isolate
KERN_CONT users from ordinary complete lines").

> In the discussion *smpboot: CPU numbers printed as warning* [1] John wrote:
> 
> > It is supported to provide loglevels for CONT messages. The loglevel is
> > then only used if the append fails:
> > 
> >     pr_cont(KERN_INFO "message part");
> > 
> > I don't know if we want to go down that path. But it is supported.

Yeah, I saw that, but decided to go with the 'pr_info("")' solution, because
it is less magic, and already used in two other drivers.

pr_info("") will also prepend 'AMD-Vi:' to the feature list, which is nice.

Best regards,
Alexander