[PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Pavel Emelyanov]

From: Konstantin Khlebnikov <khlebnikov@openvz.org>

The struct autofs_v5_packet has only one difference between
32-bit and 64-bit versions - on 64-bit gcc aligns its size and
it is 4 bytes larger that it is on 32-bit kernel. This confuses
32-bit user-space daemon, when talking to 64-bit kernel.

This is very critical for containerized setups, when containers
with <different>-bit tolls are used.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
Acked-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index 1e4a539..9855b6e 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -132,6 +132,14 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
 		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
 
 		pktsz = sizeof(*packet);
+#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)
+		/*
+		 * On x86_64 autofs_v5_packet struct is padded with 4 bytes
+		 * which breaks 32-bit autofs daemon.
+		 */
+		if (test_thread_flag(TIF_IA32))
+			pktsz -= 4;
+#endif
 
 		packet->wait_queue_token = wq->wait_queue_token;
 		packet->len = wq->len;

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Ian Kent]

On Wed, 2008-06-11 at 13:54 +0400, Pavel Emelyanov wrote:
> From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> 
> The struct autofs_v5_packet has only one difference between
> 32-bit and 64-bit versions - on 64-bit gcc aligns its size and
> it is 4 bytes larger that it is on 32-bit kernel. This confuses
> 32-bit user-space daemon, when talking to 64-bit kernel.

No, I don't think that's quite right.

As far as I know this issue arises when a user space program, compiled
as a 32-bit application is executed within 64-bit user space.

> 
> This is very critical for containerized setups, when containers
> with <different>-bit tolls are used.

Have you tested different situations with this change?
Will this affect the existing check and adjustment the version 5
automount daemon does now?

> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Acked-by: Pavel Emelyanov <xemul@openvz.org>
> 
> ---
> 
> diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
> index 1e4a539..9855b6e 100644
> --- a/fs/autofs4/waitq.c
> +++ b/fs/autofs4/waitq.c
> @@ -132,6 +132,14 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
>  		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
>  
>  		pktsz = sizeof(*packet);
> +#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)
> +		/*
> +		 * On x86_64 autofs_v5_packet struct is padded with 4 bytes
> +		 * which breaks 32-bit autofs daemon.
> +		 */
> +		if (test_thread_flag(TIF_IA32))
> +			pktsz -= 4;
> +#endif
>  
>  		packet->wait_queue_token = wq->wait_queue_token;
>  		packet->len = wq->len;
> 

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Pavel Emelyanov]

Ian Kent wrote:
> On Wed, 2008-06-11 at 13:54 +0400, Pavel Emelyanov wrote:
>> From: Konstantin Khlebnikov <khlebnikov@openvz.org>
>>
>> The struct autofs_v5_packet has only one difference between
>> 32-bit and 64-bit versions - on 64-bit gcc aligns its size and
>> it is 4 bytes larger that it is on 32-bit kernel. This confuses
>> 32-bit user-space daemon, when talking to 64-bit kernel.
> 
> No, I don't think that's quite right.
> 
> As far as I know this issue arises when a user space program, compiled
> as a 32-bit application is executed within 64-bit user space.

What program do mean here? The problem arises right on the kernel-daemon
boundary - the latter refuses to accept the message with larger length.
No other software required.

>> This is very critical for containerized setups, when containers
>> with <different>-bit tolls are used.
> 
> Have you tested different situations with this change?
> Will this affect the existing check and adjustment the version 5
> automount daemon does now?

64-bit daemons *still* work OK and 32-bit *start* to after this fix :)

>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
>> Acked-by: Pavel Emelyanov <xemul@openvz.org>
>>
>> ---
>>
>> diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
>> index 1e4a539..9855b6e 100644
>> --- a/fs/autofs4/waitq.c
>> +++ b/fs/autofs4/waitq.c
>> @@ -132,6 +132,14 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
>>  		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
>>  
>>  		pktsz = sizeof(*packet);
>> +#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)
>> +		/*
>> +		 * On x86_64 autofs_v5_packet struct is padded with 4 bytes
>> +		 * which breaks 32-bit autofs daemon.
>> +		 */
>> +		if (test_thread_flag(TIF_IA32))
>> +			pktsz -= 4;
>> +#endif
>>  
>>  		packet->wait_queue_token = wq->wait_queue_token;
>>  		packet->len = wq->len;
>>
> 
> --
> 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/
> 

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Ian Kent]

On Wed, 2008-06-11 at 16:47 +0400, Pavel Emelyanov wrote:
> Ian Kent wrote:
> > On Wed, 2008-06-11 at 13:54 +0400, Pavel Emelyanov wrote:
> >> From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> >>
> >> The struct autofs_v5_packet has only one difference between
> >> 32-bit and 64-bit versions - on 64-bit gcc aligns its size and
> >> it is 4 bytes larger that it is on 32-bit kernel. This confuses
> >> 32-bit user-space daemon, when talking to 64-bit kernel.
> > 
> > No, I don't think that's quite right.
> > 
> > As far as I know this issue arises when a user space program, compiled
> > as a 32-bit application is executed within 64-bit user space.
> 
> What program do mean here? The problem arises right on the kernel-daemon
> boundary - the latter refuses to accept the message with larger length.
> No other software required.

Any program that is compiled to use the autofs4 module with the version
5 communication protocol. As far as I know only autofs version 5 uses
this at the moment.

This isn't a problem for a 32-bit daemon running within a 32-bit user
space environment or a 64-bit daemon running within a 64-bit user space
environment.

> 
> >> This is very critical for containerized setups, when containers
> >> with <different>-bit tolls are used.
> > 
> > Have you tested different situations with this change?
> > Will this affect the existing check and adjustment the version 5
> > automount daemon does now?
> 
> 64-bit daemons *still* work OK and 32-bit *start* to after this fix :)

My point is that I think this change may adversely affect existing
compiled user space applications and I want know if I'm right.

I acknowledge that the struct padding is a problem and I am aware of it
but it is potentially a big problem to just change the kernel structure
size.

Does the test_thread_flag(TIF_IA32) macro only return true for a 32-bit
user-space process running within a 64-bit user space environment
(perhaps I can do away with the check in the autofs daemon, perhaps it
doesn't quite work correctly)?

> 
> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> >> Acked-by: Pavel Emelyanov <xemul@openvz.org>
> >>
> >> ---
> >>
> >> diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
> >> index 1e4a539..9855b6e 100644
> >> --- a/fs/autofs4/waitq.c
> >> +++ b/fs/autofs4/waitq.c
> >> @@ -132,6 +132,14 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
> >>  		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
> >>  
> >>  		pktsz = sizeof(*packet);
> >> +#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)
> >> +		/*
> >> +		 * On x86_64 autofs_v5_packet struct is padded with 4 bytes
> >> +		 * which breaks 32-bit autofs daemon.
> >> +		 */
> >> +		if (test_thread_flag(TIF_IA32))
> >> +			pktsz -= 4;
> >> +#endif
> >>  
> >>  		packet->wait_queue_token = wq->wait_queue_token;
> >>  		packet->len = wq->len;
> >>
> > 
> > --
> > 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/
> > 
> 

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Ian Kent]

On Wed, 2008-06-11 at 16:47 +0400, Pavel Emelyanov wrote:
> Ian Kent wrote:
> > On Wed, 2008-06-11 at 13:54 +0400, Pavel Emelyanov wrote:
> >> From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> >>
> >> The struct autofs_v5_packet has only one difference between
> >> 32-bit and 64-bit versions - on 64-bit gcc aligns its size and
> >> it is 4 bytes larger that it is on 32-bit kernel. This confuses
> >> 32-bit user-space daemon, when talking to 64-bit kernel.
> > 
> > No, I don't think that's quite right.
> > 
> > As far as I know this issue arises when a user space program, compiled
> > as a 32-bit application is executed within 64-bit user space.
> 
> What program do mean here? The problem arises right on the kernel-daemon
> boundary - the latter refuses to accept the message with larger length.
> No other software required.
> 
> >> This is very critical for containerized setups, when containers
> >> with <different>-bit tolls are used.
> > 
> > Have you tested different situations with this change?
> > Will this affect the existing check and adjustment the version 5
> > automount daemon does now?
> 
> 64-bit daemons *still* work OK and 32-bit *start* to after this fix :)
> 
> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> >> Acked-by: Pavel Emelyanov <xemul@openvz.org>
> >>
> >> ---
> >>
> >> diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
> >> index 1e4a539..9855b6e 100644
> >> --- a/fs/autofs4/waitq.c
> >> +++ b/fs/autofs4/waitq.c
> >> @@ -132,6 +132,14 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
> >>  		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
> >>  
> >>  		pktsz = sizeof(*packet);
> >> +#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)

Oh .. so maybe the answer to my question is yes?

What about other arches offer 32-bit within a 64-bit environment (has
this been the case on sparc64 at some point)?
What about the compiler padding on these?

Don't get me wrong, I'm not against fixing this, in fact I'd like to,
but I'm concerned it may end up a bit of a can of worms.

> >> +		/*
> >> +		 * On x86_64 autofs_v5_packet struct is padded with 4 bytes
> >> +		 * which breaks 32-bit autofs daemon.
> >> +		 */
> >> +		if (test_thread_flag(TIF_IA32))
> >> +			pktsz -= 4;
> >> +#endif
> >>  
> >>  		packet->wait_queue_token = wq->wait_queue_token;
> >>  		packet->len = wq->len;
> >>
> > 
> > --
> > 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/
> > 
> 

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Pavel Emelyanov]

> Does the test_thread_flag(TIF_IA32) macro only return true for a 32-bit
> user-space process running within a 64-bit user space environment
> (perhaps I can do away with the check in the autofs daemon, perhaps it
> doesn't quite work correctly)?

Hm... If we fix it in the user level that would also be OK, I
suppose...

> Oh .. so maybe the answer to my question is yes?

It is - we check *only* for the process, that is to receive the 
packet.

> What about other arches offer 32-bit within a 64-bit environment (has
> this been the case on sparc64 at some point)?
> What about the compiler padding on these?

We have no technical ability to check this. First I wanted to
get your opinion about this particular fix for x86 machines.

As far as sparc(64) and other 32-to-64 are concerned - I can
start talking to their users/maintainers to check.

> Don't get me wrong, I'm not against fixing this, in fact I'd like to,
> but I'm concerned it may end up a bit of a can of worms.

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Ian Kent]

On Wed, 2008-06-11 at 17:25 +0400, Pavel Emelyanov wrote:
> > Does the test_thread_flag(TIF_IA32) macro only return true for a 32-bit
> > user-space process running within a 64-bit user space environment
> > (perhaps I can do away with the check in the autofs daemon, perhaps it
> > doesn't quite work correctly)?
> 
> Hm... If we fix it in the user level that would also be OK, I
> suppose...
> 
> > Oh .. so maybe the answer to my question is yes?
> 
> It is - we check *only* for the process, that is to receive the 
> packet.

Right, since the send is done in user context.

> 
> > What about other arches offer 32-bit within a 64-bit environment (has
> > this been the case on sparc64 at some point)?
> > What about the compiler padding on these?
> 
> We have no technical ability to check this. First I wanted to
> get your opinion about this particular fix for x86 machines.
> 
> As far as sparc(64) and other 32-to-64 are concerned - I can
> start talking to their users/maintainers to check.

I investigated only x86_64 and sparc64 but that was a quite a while ago
now. My sparc machine doesn't even have a Linux install atm.

If we can find a general and consistent solution I'm happy to remove the
check from the daemon and cope with the bit of pain it might cause.

> 
> > Don't get me wrong, I'm not against fixing this, in fact I'd like to,
> > but I'm concerned it may end up a bit of a can of worms.

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Mikael Pettersson]

Ian Kent writes:
 > What about other arches offer 32-bit within a 64-bit environment (has
 > this been the case on sparc64 at some point)?
 > What about the compiler padding on these?

Both ppc64 and sparc64 commonly have 32-bit userspaces.
But 64-bit and mixed 32/64-bit userspaces are of course possible.

Re: [PATCH] autofs4: fix 32-bit userspace vs 64-bit kernel communications

[Ian Kent]

On Wed, 2008-06-11 at 16:29 +0200, Mikael Pettersson wrote:
> Ian Kent writes:
>  > What about other arches offer 32-bit within a 64-bit environment (has
>  > this been the case on sparc64 at some point)?
>  > What about the compiler padding on these?
> 
> Both ppc64 and sparc64 commonly have 32-bit userspaces.
> But 64-bit and mixed 32/64-bit userspaces are of course possible.

I believe that both these archs have the same alignment characteristics
a x86_64. Can anyone confirm that?

Ian