linux-kernel - Re: [PATCH 2/2] keys: Fix the use of the C++ keyword "private" in uapi/linux/keyctl.h

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: <alpine.OSX.2.21.1809240941550.2707@mjmartin-mac01.local>
Date:   Mon, 24 Sep 2018 09:52:41 -0700 (PDT)
From:   Mat Martineau <mathew.j.martineau@...ux.intel.com>
To:     David Howells <dhowells@...hat.com>, jmorris@...ei.org
cc:     Randy Dunlap <rdunlap@...radead.org>,
        Lubomir Rintel <lkundrak@...sk>,
        Stephan Mueller <smueller@...onox.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        stable@...r.kernel.org, keyrings@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Lubomir Rintel <lkundrak@...sk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] keys: Fix the use of the C++ keyword "private" in
 uapi/linux/keyctl.h


On Mon, 24 Sep 2018, David Howells wrote:

> The keyctl_dh_params struct in uapi/linux/keyctl.h contains the symbol
> "private" which means that the header file will cause compilation failure
> if #included in to a C++ program.  Further, the patch that added the same
> struct to the keyutils package named the symbol "priv", not "private".
>
> The previous attempt to fix this (commit 8a2336e549d3) did so by simply
> renaming the kernel's copy of the field to dh_private, but this then breaks
> existing userspace and as such has to be reverted (which is done by the
> preceding patch).
>
> [And note, to those who think that wrapping the struct in extern "C" {}
> will work: it won't; that only changes how symbol names are presented to
> the assembler and linker.].
>
> Instead, insert an anonymous union around the "private" member and add a
> second member in there with the name "priv" to match the one in the
> keyutils package.  The "private" member is then wrapped in !__cplusplus
> cpp-conditionals to hide it from C++.
>
> Fixes: ddbb41148724 ("KEYS: Add KEYCTL_DH_COMPUTE command")
> Fixes: 8a2336e549d3 ("uapi/linux/keyctl.h: don't use C++ reserved keyword as a struct member name")
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Randy Dunlap <rdunlap@...radead.org>
> cc: Lubomir Rintel <lkundrak@...sk>
> cc: James Morris <jmorris@...ei.org>
> cc: Mat Martineau <mathew.j.martineau@...ux.intel.com>
> cc: Stephan Mueller <smueller@...onox.de>
> cc: Andrew Morton <akpm@...ux-foundation.org>
> cc: Linus Torvalds <torvalds@...ux-foundation.org>
> cc: stable@...r.kernel.org
> ---
>
> include/uapi/linux/keyctl.h |    7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> index 7b8c9e19bad1..0f3cb13db8e9 100644
> --- a/include/uapi/linux/keyctl.h
> +++ b/include/uapi/linux/keyctl.h
> @@ -65,7 +65,12 @@
>
> /* keyctl structures */
> struct keyctl_dh_params {
> -	__s32 private;
> +	union {
> +#ifndef __cplusplus
> +		__s32 private;
> +#endif
> +		__s32 priv;
> +	};
> 	__s32 prime;
> 	__s32 base;
> };
>
>

Thanks David. This is an important fix for existing C userspace code, it 
would create a headache now and in to the future if the keyctl.h change 
introduced in v4.19-rc3 made it in to an official release. Please merge, 
James!

Reviewed-By: Mat Martineau <mathew.j.martineau@...ux.intel.com>

--
Mat Martineau
Intel OTC