On Mar 17, 2020, at 5:31 AM, Linus Walleij <linus.walleij@linaro.org> 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


> 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 <fw@deneb.enyo.de>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> 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 <linus.walleij@linaro.org>
> ---
> fs/ext4/dir.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 9aa1f75409b0..3faf9edf3e92 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -27,6 +27,7 @@
> #include <linux/slab.h>
> #include <linux/iversion.h>
> #include <linux/unicode.h>
> +#include <linux/personality.h>
> #include "ext4.h"
> #include "xattr.h"
> 
> @@ -618,6 +619,14 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
> 
> 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
> 


Cheers, Andreas