Impact: cleanup
'make headers_check' warn us about leaking of kernel private
(mostly compile time vars) data to userspace in headers. Fix it.
ptrace_bts_config struct is wrapped by __KERNEL__ --
not sure if it was ever proposed for userland. On the
other hand branch tracing is really anxious for userland
developers.
Signed-off-by: Cyrill Gorcunov <[email protected]>
---
arch/x86/include/asm/ptrace-abi.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/ptrace-abi.h
+++ linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
@@ -50,7 +50,7 @@
#define RSP 152
#define SS 160
#define ARGOFFSET R11
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLY__ || __FRAME_OFFSETS */
/* top of stack page */
#define FRAME_SIZE 168
@@ -80,6 +80,7 @@
#define PTRACE_SINGLEBLOCK 33 /* resume execution until next branch */
+#ifdef __KERNEL__
#ifdef CONFIG_X86_PTRACE_BTS
#ifndef __ASSEMBLY__
@@ -141,5 +142,6 @@ struct ptrace_bts_config {
Returns number of BTS records drained.
*/
#endif /* CONFIG_X86_PTRACE_BTS */
+#endif /* __KERNEL__ */
#endif /* _ASM_X86_PTRACE_ABI_H */
--
Okay, this one really looks wrong to me.
Markus, could you confirm if this was ever intended to be exported to
userspace? I suspect The Right Thing is simply to remove the #ifdef
CONFIG_X86_TRACE_BTS here...
-hpa
Cyrill Gorcunov wrote:
> Impact: cleanup
>
> 'make headers_check' warn us about leaking of kernel private
> (mostly compile time vars) data to userspace in headers. Fix it.
>
> ptrace_bts_config struct is wrapped by __KERNEL__ --
> not sure if it was ever proposed for userland. On the
> other hand branch tracing is really anxious for userland
> developers.
>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> ---
> arch/x86/include/asm/ptrace-abi.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/include/asm/ptrace-abi.h
> +++ linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
> @@ -50,7 +50,7 @@
> #define RSP 152
> #define SS 160
> #define ARGOFFSET R11
> -#endif /* __ASSEMBLY__ */
> +#endif /* __ASSEMBLY__ || __FRAME_OFFSETS */
>
> /* top of stack page */
> #define FRAME_SIZE 168
> @@ -80,6 +80,7 @@
>
> #define PTRACE_SINGLEBLOCK 33 /* resume execution until next branch */
>
> +#ifdef __KERNEL__
> #ifdef CONFIG_X86_PTRACE_BTS
>
> #ifndef __ASSEMBLY__
> @@ -141,5 +142,6 @@ struct ptrace_bts_config {
> Returns number of BTS records drained.
> */
> #endif /* CONFIG_X86_PTRACE_BTS */
> +#endif /* __KERNEL__ */
>
> #endif /* _ASM_X86_PTRACE_ABI_H */
>
>-----Original Message-----
>From: H. Peter Anvin [mailto:[email protected]]
>Sent: Wednesday, January 14, 2009 11:22 PM
>To: Cyrill Gorcunov; Metzger, Markus T
>Cc: [email protected]; [email protected]; [email protected]; [email protected];
>[email protected]; Cyrill Gorcunov
>Subject: Re: [patch 3/5] x86: headers cleanup - ptrace-abi.h
>
>Okay, this one really looks wrong to me.
>
>Markus, could you confirm if this was ever intended to be exported to
>userspace? I suspect The Right Thing is simply to remove the #ifdef
>CONFIG_X86_TRACE_BTS here...
This is intended for userspace.
I added the #ifdef so the entire feature could be turned off.
This might not be necessary for declarations and macros, though.
Do you want me to remove the #ifdef CONFIG around the declarations in ptrace headers?
regards,
markus.
>
> -hpa
>
>
>Cyrill Gorcunov wrote:
>> Impact: cleanup
>>
>> 'make headers_check' warn us about leaking of kernel private
>> (mostly compile time vars) data to userspace in headers. Fix it.
>>
>> ptrace_bts_config struct is wrapped by __KERNEL__ --
>> not sure if it was ever proposed for userland. On the
>> other hand branch tracing is really anxious for userland
>> developers.
>>
>> Signed-off-by: Cyrill Gorcunov <[email protected]>
>> ---
>> arch/x86/include/asm/ptrace-abi.h | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> Index: linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
>> ===================================================================
>> --- linux-2.6.git.orig/arch/x86/include/asm/ptrace-abi.h
>> +++ linux-2.6.git/arch/x86/include/asm/ptrace-abi.h
>> @@ -50,7 +50,7 @@
>> #define RSP 152
>> #define SS 160
>> #define ARGOFFSET R11
>> -#endif /* __ASSEMBLY__ */
>> +#endif /* __ASSEMBLY__ || __FRAME_OFFSETS */
>>
>> /* top of stack page */
>> #define FRAME_SIZE 168
>> @@ -80,6 +80,7 @@
>>
>> #define PTRACE_SINGLEBLOCK 33 /* resume execution until next branch */
>>
>> +#ifdef __KERNEL__
>> #ifdef CONFIG_X86_PTRACE_BTS
>>
>> #ifndef __ASSEMBLY__
>> @@ -141,5 +142,6 @@ struct ptrace_bts_config {
>> Returns number of BTS records drained.
>> */
>> #endif /* CONFIG_X86_PTRACE_BTS */
>> +#endif /* __KERNEL__ */
>>
>> #endif /* _ASM_X86_PTRACE_ABI_H */
>>
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[Metzger, Markus T - Thu, Jan 15, 2009 at 08:06:31AM +0000]
| >-----Original Message-----
| >From: H. Peter Anvin [mailto:[email protected]]
| >Sent: Wednesday, January 14, 2009 11:22 PM
| >To: Cyrill Gorcunov; Metzger, Markus T
| >Cc: [email protected]; [email protected]; [email protected]; [email protected];
| >[email protected]; Cyrill Gorcunov
| >Subject: Re: [patch 3/5] x86: headers cleanup - ptrace-abi.h
| >
| >Okay, this one really looks wrong to me.
| >
| >Markus, could you confirm if this was ever intended to be exported to
| >userspace? I suspect The Right Thing is simply to remove the #ifdef
| >CONFIG_X86_TRACE_BTS here...
|
| This is intended for userspace.
| I added the #ifdef so the entire feature could be turned off.
| This might not be necessary for declarations and macros, though.
|
| Do you want me to remove the #ifdef CONFIG around the declarations in ptrace headers?
|
| regards,
| markus.
Hi Markus,
since it's supposed to be visible by userspace then we
should just remove this CONFIG_X86_PTRACE_BTS so if userspace
app has a reference to ptrace_bts_config but kernel in turn has
this feature turned off -- the uerspace app shouldn't fail
while being compiling.
| >
| > -hpa
| >
| >
...
- Cyrill -
Cyrill Gorcunov wrote:
>
> Hi Markus,
>
> since it's supposed to be visible by userspace then we
> should just remove this CONFIG_X86_PTRACE_BTS so if userspace
> app has a reference to ptrace_bts_config but kernel in turn has
> this feature turned off -- the uerspace app shouldn't fail
> while being compiling.
>
Well, for userspace, the kernel configuration doesn't matter: the
CONFIG_* macros *never* exist.
This would be yet another good reason why having them be always defined
and 0/1 instead would be such an improvement, but we're not there.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
[H. Peter Anvin - Thu, Jan 15, 2009 at 08:52:24AM -0800]
| Cyrill Gorcunov wrote:
| >
| > Hi Markus,
| >
| > since it's supposed to be visible by userspace then we
| > should just remove this CONFIG_X86_PTRACE_BTS so if userspace
| > app has a reference to ptrace_bts_config but kernel in turn has
| > this feature turned off -- the uerspace app shouldn't fail
| > while being compiling.
| >
|
| Well, for userspace, the kernel configuration doesn't matter: the
| CONFIG_* macros *never* exist.
Let me explain more detailed what I've meant with my prev message.
Markus said that ptrace_bts_config is supposed to be visible in userspace,
right? So I've a program which is built under kernel with
CONFIG_X86_PTRACE_BTS turned on. Then I rebuild my kernel with
CONFIG_X86_PTRACE_BTS turned off. Then I decide to recompile
my program for some reason _and_ compilation shouldn't fail
because of lack of ptrace_bts_config struct. So for userspace
kernel configuration does matter if it touches data being referenced
from user-side. BUT all I said is valid (at least I hop so) if _only_
ptrace_bts_config is supposed to be visible to user-space programs.
|
| This would be yet another good reason why having them be always defined
| and 0/1 instead would be such an improvement, but we're not there.
|
oh, that reminds me autoconf horror :) I don't know if it possible
but we could have some common/base file with all CONFIG_ set to 0/1
which any header being exported to userspace should include, or
we could modify unifdef to process headers in fashion: scan the header,
insert CONFIG_'s refered in the header with value 0/1 at top of the
header. Not sure if it worth it (too many files are to be touched).
| -hpa
|
| --
| H. Peter Anvin, Intel Open Source Technology Center
| I work for Intel. I don't speak on their behalf.
|
- Cyrill -
Cyrill Gorcunov wrote:
>
> Let me explain more detailed what I've meant with my prev message.
> Markus said that ptrace_bts_config is supposed to be visible in userspace,
> right? So I've a program which is built under kernel with
> CONFIG_X86_PTRACE_BTS turned on. Then I rebuild my kernel with
> CONFIG_X86_PTRACE_BTS turned off. Then I decide to recompile
> my program for some reason _and_ compilation shouldn't fail
> because of lack of ptrace_bts_config struct. So for userspace
> kernel configuration does matter if it touches data being referenced
> from user-side. BUT all I said is valid (at least I hop so) if _only_
> ptrace_bts_config is supposed to be visible to user-space programs.
>
CONFIG_* is not visible to userspace. Furthermore, there is (almost) no
point in putting a structure definition under #ifdef unless it uses data
types that somehow depend on the configuration (and those data
structures would be fundamentally ineligible to be exported to
userspace!!) -- if the feature isn't configured the structure definition
just doesn't get used.
> |
> | This would be yet another good reason why having them be always defined
> | and 0/1 instead would be such an improvement, but we're not there.
> |
>
> oh, that reminds me autoconf horror :) I don't know if it possible
> but we could have some common/base file with all CONFIG_ set to 0/1
> which any header being exported to userspace should include, or
> we could modify unifdef to process headers in fashion: scan the header,
> insert CONFIG_'s refered in the header with value 0/1 at top of the
> header. Not sure if it worth it (too many files are to be touched).
>
No, no, no, no, no.
We're not exporting CONFIG_* to userspace.
The point was that if we were using #if instead of #ifdef, then -Wundef
could be used to complain instead of silently ignoring sections.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
[H. Peter Anvin - Thu, Jan 15, 2009 at 09:55:23AM -0800]
...
| The point was that if we were using #if instead of #ifdef, then -Wundef
| could be used to complain instead of silently ignoring sections.
Ah, I see now what you mean.
|
| -hpa
|
| --
| H. Peter Anvin, Intel Open Source Technology Center
| I work for Intel. I don't speak on their behalf.
|
- Cyrill -