linux-kernel - Re: [PATCH] Make PNP IDs all uppercase

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]

Message-Id: <200901211009.48191.bjorn.helgaas@hp.com>
Date:	Wed, 21 Jan 2009 10:09:46 -0700
From:	Bjorn Helgaas <bjorn.helgaas@...com>
To:	Philipp Kohlbecher <xt28@....de>
Cc:	Alessandro Zummo <a.zummo@...ertech.it>,
	rtc-linux@...glegroups.com, Vojtech Pavlik <vojtech@....cz>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>, linux-input@...r.kernel.org,
	Pau Oliva Fora <pof@...ack.org>,
	Hans Verkuil <hverkuil@...all.nl>,
	"David S. Miller" <davem@...emloft.net>,
	Jaroslav Kysela <perex@...ex.cz>, Adam Belay <abelay@....edu>,
	Len Brown <lenb@...nel.org>,
	Jonathan Woithe <jwoithe@...sics.adelaide.edu.au>,
	Carlos Corbacho <carlos@...angeworlds.co.uk>,
	linux-acpi@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
	Kay Sievers <kay.sievers@...y.org>
Subject: Re: [PATCH] Make PNP IDs all uppercase

On Wednesday 21 January 2009 12:56:07 am Philipp Kohlbecher wrote:
> Bjorn Helgaas wrote:
> > On Sunday 18 January 2009 07:02:32 am Philipp Kohlbecher wrote:
> >> If I understand correctly, ACPI _HIDs (including PNP IDs) should be all 
> >> uppercase, including the hex digits, cf. ACPI Specification 3.0b [1], 
> >> pp. 162-3
> > 
> > Section 6.1.4, "_HID (Hardware ID)" says:
> > 
> >     A valid PNP ID must be of the form “AAA####” where A is an uppercase
> >     letter and # is a hex digit.  A valid ACPI ID must be of the form
> >     “ACPI####” where # is a hex digit.
> > 
> > I don't see the part about requiring the hex digits to be uppercase.
> > Did I miss it, or is it somewhere else?
> 
> It doesn't say explicitly, hence my reservation. The ACPI specification 
> does, however, use uppercase hex digits in all examples of _HIDs.

The PNPBIOS spec doesn't seem to say explicitly either, although the
examples there also use uppercase.  So it'd probably be nicer if we
used uppercase everywhere in Linux.  Unfortunately, we have the legacy
of the kernel using lowercase in PNP IDs, and there are userspace things
like modprobe aliases that depend on that.

> I do. Thanks for noticing that. Now, how do I make sure this mix-up 
> doesn't go into the commit message? Re-submit or ask the committer to 
> correct it?

I think reposting the patch will be better.  But I would split this
up into a patch per file, so they can go through the usual maintainers,
and I think we should wait a bit (see below).

> > There are definitely some inconsistencies that it would be nice to
> > fix, if we can do it safely.  For example, for ISAPNP and PNPBIOS
> > devices, I think we always generate lowercase hex digits in the PNP
> > IDs.  For PNPACPI, we generate uppercase digits for numeric _HIDs,
> > but we use string _HIDs unchanged.
> > 
> > Did you trip over an actual problem that is fixed by this patch?  If
> > so, can you give any more details?
> 
> Yes. After a package update, hwclock stopped using the "--directisa" 
> switch and could not access the RTC anymore. My RTC is handled by 
> rtc-cmos, which is compiled as a module. However, udev did not load this 
> module, as the device's modalias "acpi:PNP0B00:" did not match any of 
> the module's aliasas, including "acpi*:PNP0b00:*". Thus, the system 
> clock was not set correctly at startup (my hardware clock is set to 
> local time). Changing the PNP IDs in drivers/rtc/rtc-cmos.c to use 
> uppercase hex digits solved that problem for me.

Nice work debugging that problem!  I would include that synopsis in
the changelog, because it helps people who trip over the same problem
and are looking for a solution, and it helps people who want to
understand why the change was made.

IMO, part of the problem here is that there is a single PNP ID
namespace, but it's shared between the PNP core and the ACPI core.
And PNP doesn't emit uevents, so in this case, we're using ACPI
uevents to load a PNP driver.  The ACPI uevents will be uppercase
(which matches the IDs in ACPI drivers), but the IDs in PNP drivers
are typically lowercase (which matches the strings in
/sys/devices/pnp*/*/id).

I started trying to untangle this last year, but I didn't get far
enough to be comfortable that I wouldn't break existing userspace
stuff.  Maybe it's time to resurrect that work.

The RTC problem is not really a regression; it sounds like this is
a long-standing problem.  Given that, I'd like to have a clearer
idea of how to clean up the namespace and uevent issues before we
go changing all the PNP drivers.

Bjorn

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/