It was brought to my attention that this bug from 2018 was
still unresolved: 32 bit emulators like QEMU were given
64 bit hashes when running 32 bit emulation on 64 bit systems.
The personality(2) system call supports to let processes
indicate that they are 32 bit Linux to the kernel. This
was suggested by Teo in the original thread, so I just wired
it up and it solves the problem.
Programs that need the 32 bit hash only need to issue the
personality(PER_LINUX32) call and things start working.
I made a test program like this:
#include <dirent.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/personality.h>
int main(int argc, char** argv) {
DIR* dir;
personality(PER_LINUX32);
dir = opendir("/boot");
printf("dir=%p\n", dir);
printf("readdir(dir)=%p\n", readdir(dir));
printf("errno=%d: %s\n", errno, strerror(errno));
return 0;
}
This was compiled with an ARM32 toolchain from Bootlin using
glibc 2.28 and thus suffering from the bug.
Before the patch:
$ ./readdir-bug
dir=0x86000
readdir(dir)=(nil)
errno=75: Value too large for defined data type
After the patch:
$ ./readdir-bug
dir=0x86000
readdir(dir)=0x86020
errno=0: Success
Problem solved.
Cc: Florian Weimer <redacted>
Cc: Peter Maydell <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: stable@vger.kernel.org
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Link: https://bugs.launchpad.net/qemu/+bug/1805913
Link: https://lore.kernel.org/lkml/87bm56vqg4.fsf@mid.deneb.enyo.de/
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=205957
Signed-off-by: Linus Walleij <redacted>
---
fs/ext4/dir.c | 9 +++++++++
1 file changed, 9 insertions(+)
From: Peter Maydell <hidden> Date: 2020-03-17 11:58:23
On Tue, 17 Mar 2020 at 11:31, Linus Walleij [off-list ref] wrote:
It was brought to my attention that this bug from 2018 was
still unresolved: 32 bit emulators like QEMU were given
64 bit hashes when running 32 bit emulation on 64 bit systems.
The personality(2) system call supports to let processes
indicate that they are 32 bit Linux to the kernel. This
was suggested by Teo in the original thread, so I just wired
it up and it solves the problem.
Thanks for having a look at this. I'm not sure this is what
QEMU needs, though. When QEMU runs, it is not a 32-bit
process, it's a 64-bit process. Some of the syscalls
it makes are on behalf of the guest and would need 32-bit
semantics (including this one of wanting 32-bit hash sizes
in directory reads). But some syscalls it makes for itself
(either directly, or via libraries it's linked against
including glibc and glib) -- those would still want the
usual 64-bit semantics, I would have thought.
Programs that need the 32 bit hash only need to issue the
personality(PER_LINUX32) call and things start working.
What in particular does this personality setting affect?
My copy of the personality(2) manpage just says:
PER_LINUX32 (since Linux 2.2)
[To be documented.]
which isn't very informative.
thanks
-- PMM
From: Andreas Dilger <hidden> Date: 2020-03-17 22:29:02
On Mar 17, 2020, at 5:31 AM, Linus Walleij [off-list ref] wrote:
It was brought to my attention that this bug from 2018 was
still unresolved: 32 bit emulators like QEMU were given
64 bit hashes when running 32 bit emulation on 64 bit systems.
The personality(2) system call supports to let processes
indicate that they are 32 bit Linux to the kernel. This
was suggested by Teo in the original thread, so I just wired
it up and it solves the problem.
Programs that need the 32 bit hash only need to issue the
personality(PER_LINUX32) call and things start working.
I'm generally with with this from the ext4 point of view.
That said, I'd think it would be preferable for ease of use and
compatibility that applications didn't have to be modified
(e.g. have QEMU or glibc internally set PER_LINUX32 for this
process before the 32-bit syscall is called, given that it knows
whether it is emulating a 32-bit runtime or not).
The other way to handle this would be for ARM32 to check the
PER_LINUX32 flag via is_compat_task() so that there wouldn't
need to be any changes to the ext4 code at all?
Cheers, Andreas
quoted hunk
I made a test program like this:
#include <dirent.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/personality.h>
int main(int argc, char** argv) {
DIR* dir;
personality(PER_LINUX32);
dir = opendir("/boot");
printf("dir=%p\n", dir);
printf("readdir(dir)=%p\n", readdir(dir));
printf("errno=%d: %s\n", errno, strerror(errno));
return 0;
}
This was compiled with an ARM32 toolchain from Bootlin using
glibc 2.28 and thus suffering from the bug.
Before the patch:
$ ./readdir-bug
dir=0x86000
readdir(dir)=(nil)
errno=75: Value too large for defined data type
After the patch:
$ ./readdir-bug
dir=0x86000
readdir(dir)=0x86020
errno=0: Success
Problem solved.
Cc: Florian Weimer <redacted>
Cc: Peter Maydell <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: stable@vger.kernel.org
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Link: https://bugs.launchpad.net/qemu/+bug/1805913
Link: https://lore.kernel.org/lkml/87bm56vqg4.fsf@mid.deneb.enyo.de/
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=205957
Signed-off-by: Linus Walleij <redacted>
---
fs/ext4/dir.c | 9 +++++++++
1 file changed, 9 insertions(+)
static int ext4_dir_open(struct inode * inode, struct file * filp)
{
+ /*
+ * If we are currently running e.g. a 32 bit emulator on
+ * a 64 bit machine, the emulator will indicate that it needs
+ * a 32 bit personality and thus 32 bit hashes from the file
+ * system.
+ */
+ if (personality(current->personality) == PER_LINUX32)
+ filp->f_mode |= FMODE_32BITHASH;
if (IS_ENCRYPTED(inode))
return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
return 0;
--
2.24.1
On Tue, Mar 17, 2020 at 12:58 PM Peter Maydell [off-list ref] wrote:
On Tue, 17 Mar 2020 at 11:31, Linus Walleij [off-list ref] wrote:
quoted
It was brought to my attention that this bug from 2018 was
still unresolved: 32 bit emulators like QEMU were given
64 bit hashes when running 32 bit emulation on 64 bit systems.
The personality(2) system call supports to let processes
indicate that they are 32 bit Linux to the kernel. This
was suggested by Teo in the original thread, so I just wired
it up and it solves the problem.
Thanks for having a look at this. I'm not sure this is what
QEMU needs, though. When QEMU runs, it is not a 32-bit
process, it's a 64-bit process. Some of the syscalls
it makes are on behalf of the guest and would need 32-bit
semantics (including this one of wanting 32-bit hash sizes
in directory reads). But some syscalls it makes for itself
(either directly, or via libraries it's linked against
including glibc and glib) -- those would still want the
usual 64-bit semantics, I would have thought.
The "personality" thing is a largely unused facility that
a process can use to say it has this generic behaviour.
In this case we say we have the PER_LINUX32 personality
so it will make the process evoke 32bit behaviours inside
the kernel when dealing with this process.
Which I (loosely) appreciate that we want to achieve.
quoted
Programs that need the 32 bit hash only need to issue the
personality(PER_LINUX32) call and things start working.
What in particular does this personality setting affect?
My copy of the personality(2) manpage just says:
PER_LINUX32 (since Linux 2.2)
[To be documented.]
which isn't very informative.
It's not a POSIX thing (not part of the Single Unix Specification)
so as with most Linux things it has some fuzzy semantics
defined by the community...
I usually just go to the source.
If you grep the kernel what turns up is a bunch of
architecture-specific behaviors on arm64, ia64, mips, parisc,
powerpc, s390, sparc. They are very minor.
On x86_64 the semantic effect is
none so this would work for any kind of 32bit userspace
on x86_64. It would be the first time this flag has any
effect at all on x86_64, but arguably a useful one.
I would not be surprised if running say i586 on x86_64
has the same problem, just that noone has reported
it yet. But what do I know. If they have the same problem
they can use the same solution. Hm QEMU supports
emulating i586 or even i386 ... maybe you could test?
Or tell me how to test? We might be solving a bigger
issue here.
Yours,
Linus Walleij
On Tue, Mar 17, 2020 at 11:29 PM Andreas Dilger [off-list ref] wrote:
That said, I'd think it would be preferable for ease of use and
compatibility that applications didn't have to be modified
(e.g. have QEMU or glibc internally set PER_LINUX32 for this
process before the 32-bit syscall is called, given that it knows
whether it is emulating a 32-bit runtime or not).
I think the plan is that QEMU set this, either directly when
you run qemu-user or through the binfmt handler.
https://www.kernel.org/doc/html/latest/admin-guide/binfmt-misc.html
IIUC the binfmt handler is invoked when you try to
execute ELF so-and-so-for-arch-so-and-so when you
are not that arch yourself. So that can set up this
personality.
Not that I know how the binfmt handler works, I am just
assuming that thing is some program that can issue
syscalls. It JustWorks for me after installing the QEMU
packages...
The problem still stands that userspace need to somehow
inform kernelspace that 32bit is going on, and this was the
best I could think of.
Yours,
Linus Walleij
From: Peter Maydell <hidden> Date: 2020-03-19 15:25:16
On Thu, 19 Mar 2020 at 15:13, Linus Walleij [off-list ref] wrote:
On Tue, Mar 17, 2020 at 12:58 PM Peter Maydell [off-list ref] wrote:
quoted
What in particular does this personality setting affect?
My copy of the personality(2) manpage just says:
PER_LINUX32 (since Linux 2.2)
[To be documented.]
which isn't very informative.
It's not a POSIX thing (not part of the Single Unix Specification)
so as with most Linux things it has some fuzzy semantics
defined by the community...
I usually just go to the source.
If we're going to decide that this is the way to say
"give me 32-bit semantics" we need to actually document
that and define in at least broad terms what we mean
by it, so that when new things are added that might or
might not check against the setting there is a reference
defining whether they should or not, and so that
userspace knows what it's opting into by setting the flag.
The kernel loves undocumented APIs but userspace
consumers of them are not so enamoured :-)
As a concrete example, should "give me 32-bit semantics
via PER_LINUX32" mean "mmap should always return addresses
within 4GB" ? That would seem like it would make sense --
but on the other hand it would make it absolutely unusable
for QEMU's purposes, because we want to be able to
do full 64-bit mmap() for our own internal allocations.
(This is a specific example of the general case that
I'm dubious about having this be a process-wide switch,
because QEMU is a single process which sometimes
makes syscalls on its own behalf and sometimes makes
syscalls on behalf of the guest program it is emulating.
We want 32-bit semantics for the latter but if we
also get them for the former then there might be
unintended side effects.)
I would not be surprised if running say i586 on x86_64
has the same problem, just that noone has reported
it yet. But what do I know. If they have the same problem
they can use the same solution. Hm QEMU supports
emulating i586 or even i386 ... maybe you could test?
Native i586 code on x86-64 should be fine, because it
will be using the compat syscalls, which ext4 already
ensures get the 32-bit sized hash (see hash2pos() and
friends).
thanks
-- PMM
On Thu, Mar 19, 2020 at 4:25 PM Peter Maydell [off-list ref] wrote:
On Thu, 19 Mar 2020 at 15:13, Linus Walleij [off-list ref] wrote:
quoted
On Tue, Mar 17, 2020 at 12:58 PM Peter Maydell [off-list ref] wrote:
quoted
What in particular does this personality setting affect?
My copy of the personality(2) manpage just says:
PER_LINUX32 (since Linux 2.2)
[To be documented.]
which isn't very informative.
It's not a POSIX thing (not part of the Single Unix Specification)
so as with most Linux things it has some fuzzy semantics
defined by the community...
I usually just go to the source.
If we're going to decide that this is the way to say
"give me 32-bit semantics" we need to actually document
that and define in at least broad terms what we mean
by it, so that when new things are added that might or
might not check against the setting there is a reference
defining whether they should or not, and so that
userspace knows what it's opting into by setting the flag.
The kernel loves undocumented APIs but userspace
consumers of them are not so enamoured :-)
OK I guess we can at least take this opportunity to add
some kerneldoc to the include file.
As a concrete example, should "give me 32-bit semantics
via PER_LINUX32" mean "mmap should always return addresses
within 4GB" ? That would seem like it would make sense --
Incidentally that thing in particular has its own personality
flag (personalities are additive, it's a bit schizophrenic)
so PER_LINUX_32BIT is defined as:
PER_LINUX_32BIT = 0x0000 | ADDR_LIMIT_32BIT,
and that is specifically for limiting the address space to
32bit.
There is also PER_LINUX32_3GB for a 3GB lowmem
limit.
Since the personality is kind of additive, if
we want a flag *specifically* for indicating that we want
32bit hashes from the file system, there are bits left so we
can provide that.
Is this what we want to do? I just think we shouldn't
decide on that lightly as we will be using up personality
bug bits, but sometimes you have to use them.
PER_LINUX32 as it stands means 32bit personality
but very specifically does not include memory range
limitations since that has its own flags.
Yours,
Linus Walleij
From: "Theodore Y. Ts'o" <tytso@mit.edu> Date: 2020-03-24 02:34:49
On Thu, Mar 19, 2020 at 11:23:33PM +0100, Linus Walleij wrote:
OK I guess we can at least take this opportunity to add
some kerneldoc to the include file.
quoted
As a concrete example, should "give me 32-bit semantics
via PER_LINUX32" mean "mmap should always return addresses
within 4GB" ? That would seem like it would make sense --
Incidentally that thing in particular has its own personality
flag (personalities are additive, it's a bit schizophrenic)
so PER_LINUX_32BIT is defined as:
PER_LINUX_32BIT = 0x0000 | ADDR_LIMIT_32BIT,
and that is specifically for limiting the address space to
32bit.
There is also PER_LINUX32_3GB for a 3GB lowmem
limit.
Since the personality is kind of additive, if
we want a flag *specifically* for indicating that we want
32bit hashes from the file system, there are bits left so we
can provide that.
Is this what we want to do? I just think we shouldn't
decide on that lightly as we will be using up personality
bug bits, but sometimes you have to use them.
I've been looking at the personality bug bits more detailed, and it
looks... messy. Do we pick a new personality, or do we grab another
unique flag?
Another possibility, which would be messier for qemu, would be use a
flag set via fcntl. That would require qemu from noticing when the
guest is calling open, openat, or openat2, and then inserting a fcntl
system call to set the 32-bit readdir mode. That's cleaner from the
kernel interface complexity perspective, but it's messier for qemu.
- Ted
From: Peter Maydell <hidden> Date: 2020-03-24 09:30:11
On Tue, 24 Mar 2020 at 02:34, Theodore Y. Ts'o [off-list ref] wrote:
Another possibility, which would be messier for qemu, would be use a
flag set via fcntl. That would require qemu from noticing when the
guest is calling open, openat, or openat2, and then inserting a fcntl
system call to set the 32-bit readdir mode. That's cleaner from the
kernel interface complexity perspective, but it's messier for qemu.
On the contrary, that would be a much better interface for QEMU.
We always know when we're doing an open-syscall on behalf
of the guest, and it would be trivial to make the fcntl() call then.
That would ensure that we don't accidentally get the
'32-bit semantics' on file descriptors QEMU opens for its own
purposes, and wouldn't leave us open to the risk in future that
setting the PER_LINUX32 flag for all of QEMU causes
unexpected extra behaviour in future kernels that would be correct
for the guest binary but wrong/broken for QEMU's own internals.
thanks
-- PMM
From: "Theodore Y. Ts'o" <tytso@mit.edu> Date: 2020-03-24 18:48:11
On Tue, Mar 24, 2020 at 09:29:58AM +0000, Peter Maydell wrote:
On the contrary, that would be a much better interface for QEMU.
We always know when we're doing an open-syscall on behalf
of the guest, and it would be trivial to make the fcntl() call then.
That would ensure that we don't accidentally get the
'32-bit semantics' on file descriptors QEMU opens for its own
purposes, and wouldn't leave us open to the risk in future that
setting the PER_LINUX32 flag for all of QEMU causes
unexpected extra behaviour in future kernels that would be correct
for the guest binary but wrong/broken for QEMU's own internals.
If using a flag set by fcntl is better for qemu, then by all means
let's go with that instead of using a personality flag/number.
Linus, do you have what you need to do a respin of the patch?
- Ted
On Tue, Mar 24, 2020 at 7:48 PM Theodore Y. Ts'o [off-list ref] wrote:
On Tue, Mar 24, 2020 at 09:29:58AM +0000, Peter Maydell wrote:
quoted
On the contrary, that would be a much better interface for QEMU.
We always know when we're doing an open-syscall on behalf
of the guest, and it would be trivial to make the fcntl() call then.
That would ensure that we don't accidentally get the
'32-bit semantics' on file descriptors QEMU opens for its own
purposes, and wouldn't leave us open to the risk in future that
setting the PER_LINUX32 flag for all of QEMU causes
unexpected extra behaviour in future kernels that would be correct
for the guest binary but wrong/broken for QEMU's own internals.
If using a flag set by fcntl is better for qemu, then by all means
let's go with that instead of using a personality flag/number.
Linus, do you have what you need to do a respin of the patch?
Absolutely, I'm a bit occupied this week but I will try to get to it
early next week!
Thanks a lot for the directions here, it's highly valuable.
Yours,
Linus Walleij