[RFC][PATCH 0/3] Process events biarch bug: Intro

[RFC][PATCH 0/3] Process events biarch bug: Intro

[Matt Helsley]

The events sent by Process Events from a 64-bit kernel are not binary compatible
with what a 32-bit userspace program would expect to recieve because the timespec
struct (used to send a timestamp) is not the same. This means that fields stored
after the timestamp are offset and programs that don't take this into account
break under these circumstances.

This is a problem for 32-bit userspace programs running with 64-bit kernels on
ppc64, s390, x86-64.. any "biarch" system.

This series:

1 - Gives a name to the union of the process events structure so it may be used
	to work around the problem from userspace.
2 - Comments on the bug and describes a userspace workaround in
	Documentation/connector/process_events.txt
3 - Implements a second connector interface without the problem
	(Removing the old interface or changing the definition would break
	 binary compatibility)

Compiled, booted, and lightly tested.

Comments or suggestions on alternate approaches would be appreciated.

Cheers,
	-Matt Helsley

Re: [RFC][PATCH 0/3] Process events biarch bug: Intro

[Evgeniy Polyakov]

On Tue, Jun 27, 2006 at 04:47:01AM -0700, Matt Helsley (matthltc@us.ibm.com) wrote:
> The events sent by Process Events from a 64-bit kernel are not binary compatible
> with what a 32-bit userspace program would expect to recieve because the timespec
> struct (used to send a timestamp) is not the same. This means that fields stored
> after the timestamp are offset and programs that don't take this into account
> break under these circumstances.
> 
> This is a problem for 32-bit userspace programs running with 64-bit kernels on
> ppc64, s390, x86-64.. any "biarch" system.
> 
> This series:
> 
> 1 - Gives a name to the union of the process events structure so it may be used
> 	to work around the problem from userspace.
> 2 - Comments on the bug and describes a userspace workaround in
> 	Documentation/connector/process_events.txt
> 3 - Implements a second connector interface without the problem
> 	(Removing the old interface or changing the definition would break
> 	 binary compatibility)

If you are sure binary compatibility on this stage of process
notification connector is really major issue, then I agree that above is
correct, otherwise I would recommend to just replace broken code with fixed size objects.

Btw, __u64 is not the best choice for some arches too due to it's
alignment (64bit code requires u64 to be aligned to 64 bit, while 32bit
code will only align it to 32 bits), so you will need 
attribute ((aligned(8)))) for your own ___u64.

-- 
	Evgeniy Polyakov

Re: [RFC][PATCH 0/3] Process events biarch bug: Intro

[Matt Helsley]

On Tue, 2006-06-27 at 16:33 +0400, Evgeniy Polyakov wrote:
> On Tue, Jun 27, 2006 at 04:47:01AM -0700, Matt Helsley (matthltc@us.ibm.com) wrote:
> > The events sent by Process Events from a 64-bit kernel are not binary compatible
> > with what a 32-bit userspace program would expect to recieve because the timespec
> > struct (used to send a timestamp) is not the same. This means that fields stored
> > after the timestamp are offset and programs that don't take this into account
> > break under these circumstances.
> > 
> > This is a problem for 32-bit userspace programs running with 64-bit kernels on
> > ppc64, s390, x86-64.. any "biarch" system.
> > 
> > This series:
> > 
> > 1 - Gives a name to the union of the process events structure so it may be used
> > 	to work around the problem from userspace.
> > 2 - Comments on the bug and describes a userspace workaround in
> > 	Documentation/connector/process_events.txt
> > 3 - Implements a second connector interface without the problem
> > 	(Removing the old interface or changing the definition would break
> > 	 binary compatibility)
> 
> If you are sure binary compatibility on this stage of process
> notification connector is really major issue, then I agree that above is
> correct, otherwise I would recommend to just replace broken code with fixed size objects.

	It's not clear whether event binary compatibility is a major issue. I
chose to assume it was and presented the best option (that I could think
of) for preserving binary compatibility.

> Btw, __u64 is not the best choice for some arches too due to it's
> alignment (64bit code requires u64 to be aligned to 64 bit, while 32bit
> code will only align it to 32 bits), so you will need 
> attribute ((aligned(8)))) for your own ___u64.

	Fixing the alignment would be a good idea. Though setting it to 8 would
introduce 4 unused bytes at the end of the structure.

Cheers,
	-Matt Helsley

Re: [RFC][PATCH 0/3] Process events biarch bug: Intro

[Evgeniy Polyakov]

On Tue, Jun 27, 2006 at 02:39:51PM -0700, Matt Helsley (matthltc@us.ibm.com) wrote:
> > Btw, __u64 is not the best choice for some arches too due to it's
> > alignment (64bit code requires u64 to be aligned to 64 bit, while 32bit
> > code will only align it to 32 bits), so you will need 
> > attribute ((aligned(8)))) for your own ___u64.
> 
> 	Fixing the alignment would be a good idea. Though setting it to 8 would
> introduce 4 unused bytes at the end of the structure.

Otherwise your code will not work, although u64 is supposed to be fixed
size, due to it's alignment problems it can not be used as is.
Split timestamp into two 32bit values and everything will be ok.

> Cheers,
> 	-Matt Helsley

-- 
	Evgeniy Polyakov

Re: [RFC][PATCH 0/3] Process events biarch bug: Intro

[Chandra Seetharaman]

On Tue, 2006-06-27 at 04:47 -0700, Matt Helsley wrote:
> The events sent by Process Events from a 64-bit kernel are not binary compatible
> with what a 32-bit userspace program would expect to recieve because the timespec
> struct (used to send a timestamp) is not the same. This means that fields stored
> after the timestamp are offset and programs that don't take this into account
> break under these circumstances.
> 
> This is a problem for 32-bit userspace programs running with 64-bit kernels on
> ppc64, s390, x86-64.. any "biarch" system.
> 
> This series:
> 
> 1 - Gives a name to the union of the process events structure so it may be used
> 	to work around the problem from userspace.
> 2 - Comments on the bug and describes a userspace workaround in
> 	Documentation/connector/process_events.txt

Above two options need the user space program to workaround the issue,
while assuming that the size of the data structure will never change (if
it ever changes, you break compatibility), which IMO is not very
appealing.

> 3 - Implements a second connector interface without the problem
> 	(Removing the old interface or changing the definition would break
> 	 binary compatibility)

I think this is a better option if we want to fix the problem while
maintaining compatibility.

> 
> Compiled, booted, and lightly tested.
> 
> Comments or suggestions on alternate approaches would be appreciated.
> 
> Cheers,
> 	-Matt Helsley
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------