linux-kernel - Re: [PATCH] drivers/x86: add thinkpad-wmi

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: <20171115100252.GA8412@amd>
Date:   Wed, 15 Nov 2017 11:02:52 +0100
From:   Pavel Machek <pavel@....cz>
To:     Corentin Chary <corentin.chary@...il.com>
Cc:     Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers/x86: add thinkpad-wmi

Hi!

> diff --git a/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi b/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi
> new file mode 100644
> index 000000000000..c3673876c5b3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi
> @@ -0,0 +1,50 @@
> +What:		/sys/devices/platform/thinkpad-wmi/password
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		BIOS password needs to be written in this file if set
> +		to be able to change BIOS settings.

Should this go under platform/thinkpad-wmi? Seems like similar
interface is potentially useful on different machines?

> +What:		/sys/devices/platform/thinkpad-wmi/password_encoding
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Password encoding ('ascii' or 'scanmode').
> +
> +What:		/sys/devices/platform/thinkpad-wmi/password_kbd_lang
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Keyboard language used for password. One of 'us', 'fr' and 'gr'.
> +
> +What:		/sys/devices/platform/thinkpad-wmi/password_type
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Password type to be changed when password_change is written to, e.g. 'pap'.

> +What:		/sys/devices/platform/thinkpad-wmi/password_change
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Writing to this file will set the password specified in password_type.
> +		The new password will not take effect until the next reboot.

With the different "encoding" and "keyboard language" fields, this
looks like great way to lock user out of his own machine ;-(.

> +What:		/sys/devices/platform/thinkpad-wmi/password_settings
> +Date:		Oct 2015
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Display various password settings.

Umm. We have one value per file in sysfs?

> +What:		/sys/devices/platform/thinkpad-wmi/load_default_settings
> +Date:		Oct 2015
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@...il.com>
> +Description:
> +		Write anything to this file to load default BIOS
> settings.

Is that reasonable?

> +### password_type
> +
> +Specify the password type to be changed when password_change is written to.
> +Can be:
> +* 'pap': supervisor password
> +* 'pop': power-on-password
> +
> +Other types may be valid, e.g. for user and master disk passwords.

This is extremely weird for /sysfs interface. Sounds like you should
have supervisor_password_change and power_on_password_change, etc...

> +### password_settings
> +
> +Display password related settings. This includes:
> +
> +* password_state: which passwords are set, if any
> +  * bit 0: user password (power on password) is installed / 'pop'
> +  * bit 1: admin/supervisor password is installed / 'pap'
> +  * bit 2: hdd password(s) installed
> +* supported_encodings: supported keyboard encoding(s)
> +  * bit 0: ASCII
> +  * bit 1: scancode
> +* supported_keyboard: support keyboard language(s)
> +  * bit 0: us
> +  * bit 1: fr
> +  * bit 2: gr
> +

If this belongs to the kernel (I'm not convinced), it certainly needs
different/better interface.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)