From: Ian Kent <[email protected]>
Date: Wed, 22 Feb 2012 20:45:44 +0800
commits a32744d4abae24572eff7269bc17895c41bd0085,
3c761ea05a8900a907f32b628611873f6bef24b2, and
048cd4e51d24ebf7f3552226d03c769d6ad91658 upstream.
When the autofs protocol version 5 packet type was added in commit
5c0a32fc2cd0 ("autofs4: add new packet type for v5 communications"), it
obvously tried quite hard to be word-size agnostic, and uses explicitly
sized fields that are all correctly aligned.
However, with the final "char name[NAME_MAX+1]" array at the end, the
actual size of the structure ends up being not very well defined:
because the struct isn't marked 'packed', doing a "sizeof()" on it will
align the size of the struct up to the biggest alignment of the members
it has.
And despite all the members being the same, the alignment of them is
different: a "__u64" has 4-byte alignment on x86-32, but native 8-byte
alignment on x86-64. And while 'NAME_MAX+1' ends up being a nice round
number (256), the name[] array starts out a 4-byte aligned.
End result: the "packed" size of the structure is 300 bytes: 4-byte, but
not 8-byte aligned.
As a result, despite all the fields being in the same place on all
architectures, sizeof() will round up that size to 304 bytes on
architectures that have 8-byte alignment for u64.
Note that this is *not* a problem for 32-bit compat mode on POWER, since
there __u64 is 8-byte aligned even in 32-bit mode. But on x86, 32-bit
and 64-bit alignment is different for 64-bit entities, and as a result
the structure that has exactly the same layout has different sizes.
So on x86-64, but no other architecture, we will just subtract 4 from
the size of the structure when running in a compat task. That way we
will write the properly sized packet that user mode expects.
Not pretty. Sadly, this very subtle, and unnecessary, size difference
has been encoded in user space that wants to read packets of *exactly*
the right size, and will refuse to touch anything else.
[jn: with compilation fixes for !COMPAT case and !COMPAT case on s390
by Linus Torvalds and Heiko Carstens squashed in]
Reported-and-tested-by: Thomas Meyer <[email protected]>
Signed-off-by: Ian Kent <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Tested-by: Sven Joachim <[email protected]>
Signed-off-by: Jonathan Nieder <[email protected]>
---
Hi Greg,
Sven Joachim wrote[1]:
> Today I installed and booted (with init=/bin/systemd) systemd for the
> first time. Alas, it does not work for me. :-( The last output on the
> screen was from systemd-fsck which successfully checked the root
> partition, then the boot hung without further output requiring to reboot
> with SysRq-b.
It turned out to be a 32-bit userspace/64-bit kernel compat problem.
The broken interface was introduced in v2.6.17-rc1~444 but I guess
the problem has become more noticeable recently because systemd uses it.
So longterm kernels can use this fix, too.
Luckily the fix is in mainline:
[...]
> 3.3-rc5 works for me, as does 3.2.7 with the following changes
> cherry-picked:
>
> a32744d4abae (autofs: work around unhappy compat problem on x86-64)
> 3c761ea05a89 (Fix autofs compile without CONFIG_COMPAT)
> 048cd4e51d24 (compat: fix compile breakage on s390)
>
> The latter two are necessary to fix FTBFS problems on other
> architectures introduced by a32744d4abae.
arch/s390/include/asm/compat.h | 7 -------
arch/s390/kernel/process.c | 1 -
arch/s390/kernel/ptrace.c | 2 +-
arch/s390/kernel/setup.c | 2 +-
arch/s390/kernel/signal.c | 1 -
arch/s390/mm/fault.c | 1 -
arch/s390/mm/mmap.c | 2 +-
drivers/s390/block/dasd_eckd.c | 2 +-
drivers/s390/block/dasd_ioctl.c | 1 +
drivers/s390/char/fs3270.c | 1 +
drivers/s390/char/vmcp.c | 1 +
drivers/s390/cio/chsc_sch.c | 1 +
drivers/s390/scsi/zfcp_cfdc.c | 1 +
fs/autofs4/autofs_i.h | 1 +
fs/autofs4/dev-ioctl.c | 1 +
fs/autofs4/inode.c | 2 ++
fs/autofs4/waitq.c | 22 +++++++++++++++++++---
include/linux/compat.h | 4 ++++
18 files changed, 36 insertions(+), 17 deletions(-)
The meat of the patch is not so long, though 048cd4e5 involves a lot
of ifdef churn. I suspect it would be less error-prone to move the
fallback definition of is_compat_task to <arch>/include/asm/compat.h
so the same headers are needed on x86 and other arches, but here I've
left the patch alone to match upstream. Linus, Heiko, any thoughts on
that?
[1] http://bugs.debian.org/633423
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 2e49748b27da..234f1d859cea 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -172,13 +172,6 @@ static inline int is_compat_task(void)
return is_32bit_task();
}
-#else
-
-static inline int is_compat_task(void)
-{
- return 0;
-}
-
#endif
static inline void __user *arch_compat_alloc_user_space(long len)
diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c
index 9451b210a1b4..53088e208427 100644
--- a/arch/s390/kernel/process.c
+++ b/arch/s390/kernel/process.c
@@ -29,7 +29,6 @@
#include <asm/irq.h>
#include <asm/timer.h>
#include <asm/nmi.h>
-#include <asm/compat.h>
#include <asm/smp.h>
#include "entry.h"
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 573bc29551ef..afe82bc7b928 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -20,8 +20,8 @@
#include <linux/regset.h>
#include <linux/tracehook.h>
#include <linux/seccomp.h>
+#include <linux/compat.h>
#include <trace/syscall.h>
-#include <asm/compat.h>
#include <asm/segment.h>
#include <asm/page.h>
#include <asm/pgtable.h>
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index e54c4ff8abaa..773f55e52956 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -45,6 +45,7 @@
#include <linux/kexec.h>
#include <linux/crash_dump.h>
#include <linux/memory.h>
+#include <linux/compat.h>
#include <asm/ipl.h>
#include <asm/uaccess.h>
@@ -58,7 +59,6 @@
#include <asm/ptrace.h>
#include <asm/sections.h>
#include <asm/ebcdic.h>
-#include <asm/compat.h>
#include <asm/kvm_virtio.h>
#include <asm/diag.h>
diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
index 7f6f9f354545..508655359b5e 100644
--- a/arch/s390/kernel/signal.c
+++ b/arch/s390/kernel/signal.c
@@ -30,7 +30,6 @@
#include <asm/ucontext.h>
#include <asm/uaccess.h>
#include <asm/lowcore.h>
-#include <asm/compat.h>
#include "entry.h"
#define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index a9a301866b3c..c7f0fbc00fc8 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -36,7 +36,6 @@
#include <asm/pgtable.h>
#include <asm/irq.h>
#include <asm/mmu_context.h>
-#include <asm/compat.h>
#include "../kernel/entry.h"
#ifndef CONFIG_64BIT
diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
index f09c74881b7e..a0155c02e324 100644
--- a/arch/s390/mm/mmap.c
+++ b/arch/s390/mm/mmap.c
@@ -29,8 +29,8 @@
#include <linux/mman.h>
#include <linux/module.h>
#include <linux/random.h>
+#include <linux/compat.h>
#include <asm/pgalloc.h>
-#include <asm/compat.h>
static unsigned long stack_maxrandom_size(void)
{
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 6ab29680586a..fe9dacc37622 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -18,12 +18,12 @@
#include <linux/hdreg.h> /* HDIO_GETGEO */
#include <linux/bio.h>
#include <linux/module.h>
+#include <linux/compat.h>
#include <linux/init.h>
#include <asm/debug.h>
#include <asm/idals.h>
#include <asm/ebcdic.h>
-#include <asm/compat.h>
#include <asm/io.h>
#include <asm/uaccess.h>
#include <asm/cio.h>
diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
index f1a2016829fc..792c69e78fe2 100644
--- a/drivers/s390/block/dasd_ioctl.c
+++ b/drivers/s390/block/dasd_ioctl.c
@@ -13,6 +13,7 @@
#define KMSG_COMPONENT "dasd"
#include <linux/interrupt.h>
+#include <linux/compat.h>
#include <linux/major.h>
#include <linux/fs.h>
#include <linux/blkpg.h>
diff --git a/drivers/s390/char/fs3270.c b/drivers/s390/char/fs3270.c
index e71298158f9e..911704571b9c 100644
--- a/drivers/s390/char/fs3270.c
+++ b/drivers/s390/char/fs3270.c
@@ -11,6 +11,7 @@
#include <linux/console.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/compat.h>
#include <linux/module.h>
#include <linux/list.h>
#include <linux/slab.h>
diff --git a/drivers/s390/char/vmcp.c b/drivers/s390/char/vmcp.c
index 75bde6a8b7dc..89c03e6b1c0c 100644
--- a/drivers/s390/char/vmcp.c
+++ b/drivers/s390/char/vmcp.c
@@ -13,6 +13,7 @@
#include <linux/fs.h>
#include <linux/init.h>
+#include <linux/compat.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/slab.h>
diff --git a/drivers/s390/cio/chsc_sch.c b/drivers/s390/cio/chsc_sch.c
index 0c87b0fc7714..8f9a1a384496 100644
--- a/drivers/s390/cio/chsc_sch.c
+++ b/drivers/s390/cio/chsc_sch.c
@@ -8,6 +8,7 @@
*/
#include <linux/slab.h>
+#include <linux/compat.h>
#include <linux/device.h>
#include <linux/module.h>
#include <linux/uaccess.h>
diff --git a/drivers/s390/scsi/zfcp_cfdc.c b/drivers/s390/scsi/zfcp_cfdc.c
index 303dde09d294..fab2c2592a97 100644
--- a/drivers/s390/scsi/zfcp_cfdc.c
+++ b/drivers/s390/scsi/zfcp_cfdc.c
@@ -11,6 +11,7 @@
#define KMSG_COMPONENT "zfcp"
#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+#include <linux/compat.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/miscdevice.h>
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 326dc08d3e3f..308a98bdb5f7 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -110,6 +110,7 @@ struct autofs_sb_info {
int sub_version;
int min_proto;
int max_proto;
+ int compat_daemon;
unsigned long exp_timeout;
unsigned int type;
int reghost_enabled;
diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index 509fe1eb66ae..56bac702ab09 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -385,6 +385,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
sbi->pipefd = pipefd;
sbi->pipe = pipe;
sbi->catatonic = 0;
+ sbi->compat_daemon = is_compat_task();
}
out:
mutex_unlock(&sbi->wq_mutex);
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 8179f1ab8175..98a56950f687 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -19,6 +19,7 @@
#include <linux/parser.h>
#include <linux/bitops.h>
#include <linux/magic.h>
+#include <linux/compat.h>
#include "autofs_i.h"
#include <linux/module.h>
@@ -224,6 +225,7 @@ int autofs4_fill_super(struct super_block *s, void *data,
int silent)
set_autofs_type_indirect(&sbi->type);
sbi->min_proto = 0;
sbi->max_proto = 0;
+ sbi->compat_daemon = is_compat_task();
mutex_init(&sbi->wq_mutex);
spin_lock_init(&sbi->fs_lock);
sbi->queues = NULL;
diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index e1fbdeef85db..6861f616342f 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -90,7 +90,24 @@ static int autofs4_write(struct file *file, const void
*addr, int bytes)
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(struct autofs_sb_info *sbi)
+{
+ size_t pktsz = sizeof(struct autofs_v5_packet);
+#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
+ if (sbi->compat_daemon > 0)
+ pktsz -= 4;
+#endif
+ return pktsz;
+}
+
static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
struct autofs_wait_queue *wq,
int type)
@@ -147,8 +164,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(sbi);
packet->wait_queue_token = wq->wait_queue_token;
packet->len = wq->name.len;
memcpy(packet->name, wq->name.name, wq->name.len);
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 66ed067fb729..d42bd48ae5d5 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -561,5 +561,9 @@ asmlinkage ssize_t
compat_sys_process_vm_writev(compat_pid_t pid,
unsigned long liovcnt, const struct compat_iovec __user *rvec,
unsigned long riovcnt, unsigned long flags);
+#else
+
+#define is_compat_task() (0)
+
#endif /* CONFIG_COMPAT */
#endif /* _LINUX_COMPAT_H */
--
1.7.9.2
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
