Re: Linux 3.3-rc4

[Linus Torvalds]

On Sun, Feb 19, 2012 at 5:42 AM, Thomas Meyer <thomas@xxxxxxxx> wrote:
>
> Theoretically yes, practically (at least with Fedora 15/16) no. I did hit these two bugs:
>
> 1.) autofs4 interface is broken between x86 and x86_64. as systemd uses autofs, this bug hangs the boot process as e.g. binfmt is mounted via autofs. see also http://lists.freedesktop.org/archives/systemd-devel/2011-September/003396.html

Duh.

That is just broken.

The code even *talks* about how the packet layout is the same on
32-bit and 64-bit architectures, and that's largely true.

However, while true, x86-64 has 8-byte alignment for 'long', and
x86-32 has 4-byte alignment. Which means that even though the
structure layout is exactly the same, on x86-64 the *alignment* issue
will push it out to 304 bytes.

That's just stupid. We've had that problem before. It's easy to
overlook, but that packet is just mis-designed.

The attached patch isn't pretty, but this is definitely a kernel bug.
Binary compatibility is *important*, dammit.

Does this fix it?

Sadly, the *right* fix would have been to just mark the structure
packed, or select a maximum name length that padded things out to 8
bytes on all architectures, but we can't change that any more, because
changing the size on native 32-bit would break binaries there.

> 2.) while debugging above issue: I did find an minor bug in sys_poll() - nobody did take care of my proposed patch: https://lkml.org/lkml/2011/9/24/35

Looks correct, and searching my lkml archives shows that it was acked
by Eric. But it never went any further.

Mind re-sending that patch updated for the x86 system call
re-organization? Sure, I could do it myself, but it would be much
better if somebody who then actually *tests* the end result did it
(hpa cc'd, just because he's the one that touched the x86 compat layer
system call table thing. "Tag, you're it")

Anyway, if that, together with the sizeof hack in the attached patch,
makes everything work for you, I'll happily apply both of them.

                          Linus
 fs/autofs4/waitq.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index da8876d38a7b..f6fcca58e0e3 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -15,6 +15,7 @@
 #include <linux/time.h>
 #include <linux/signal.h>
 #include <linux/file.h>
+#include <linux/compat.h>
 #include "autofs_i.h"
 
 /* We make this a static variable rather than a part of the superblock; it
@@ -91,6 +92,23 @@ static int autofs4_write(struct autofs_sb_info *sbi,
 
 	return (bytes > 0);
 }
+
+/*
+ * The autofs_v5 packet was misdesigned.
+ *
+ * The packets are identical on x86-32 and x86-64, but have different
+ * alignment. Which means that 'sizeof()' will give different results.
+ * Fix it up for the case of running 32-bit user mode on a 64-bit kernel.
+ */
+static noinline size_t autofs_v5_packet_size(void)
+{
+	size_t pktsz = sizeof(struct autofs_v5_packet);
+#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
+	if (is_compat_task())
+		pktsz -= 4;
+#endif
+	return pktsz;
+}
 	
 static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
 				 struct autofs_wait_queue *wq,
@@ -155,8 +173,7 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
 	{
 		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
 
-		pktsz = sizeof(*packet);
-
+		pktsz = autofs_v5_packet_size();
 		packet->wait_queue_token = wq->wait_queue_token;
 		packet->len = wq->name.len;
 		memcpy(packet->name, wq->name.name, wq->name.len);