From: Michael Ellerman <mpe@ellerman.id.au>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org,
Christian Brauner <brauner@kernel.org>,
Alexey Kardashevskiy <aik@amd.com>,
Paul Mackerras <paulus@ozlabs.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [RFC] potential UAF in kvm_spapr_tce_attach_iommu_group() (was Re: [PATCH 11/19] switch simple users of fdget() to CLASS(fd, ...))
Date: Thu, 13 Jun 2024 20:56:03 +1000 [thread overview]
Message-ID: <878qz9p0vw.fsf@mail.lhotse> (raw)
In-Reply-To: <CAHk-=wgf4yN4gGsGQOTBR_xE0q-9fB04omufZk2gnBRZ0Ywbiw@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sun, 9 Jun 2024 at 19:45, Al Viro <viro@zeniv.linux.org.uk> wrote:
>>
>> Unless I'm misreading that code (entirely possible), this fdput() shouldn't
>> be done until we are done with stt.
>
> Ack. That looks right to me.
>
> If I follow it right, the lifetime of stt is tied to the lifetime of
> the file (plus RCU), so doing fdput early and then dropping the RCU
> lock means that stt may not be valid any more later.
Yep. I added a sleep after the fdput and was able to get KASAN to catch
it (below).
I'll send a fix patch tomorrow, just using fdput(), and then the CLASS
conversion can go on top later.
cheers
==================================================================
BUG: KASAN: slab-use-after-free in kvm_spapr_tce_attach_iommu_group+0x298/0x720 [kvm]
Read of size 4 at addr c000200027552c30 by task kvm-vfio/2505
CPU: 54 PID: 2505 Comm: kvm-vfio Not tainted 6.10.0-rc3-next-20240612-dirty #1
Hardware name: 8335-GTH POWER9 0x4e1202 opal:skiboot-v6.5.3-35-g1851b2a06 PowerNV
Call Trace:
[c00020008c2a7860] [c0000000027d4d50] dump_stack_lvl+0xb4/0x108 (unreliable)
[c00020008c2a78a0] [c00000000072dfa8] print_report+0x2b4/0x6ec
[c00020008c2a7990] [c00000000072d898] kasan_report+0x118/0x2b0
[c00020008c2a7aa0] [c00000000072ff38] __asan_load4+0xb8/0xd0
[c00020008c2a7ac0] [c00800001b343140] kvm_spapr_tce_attach_iommu_group+0x298/0x720 [kvm]
[c00020008c2a7b90] [c00800001b31d61c] kvm_vfio_set_attr+0x524/0xac0 [kvm]
[c00020008c2a7c60] [c00800001b3083ec] kvm_device_ioctl+0x144/0x240 [kvm]
[c00020008c2a7cd0] [c0000000007e052c] sys_ioctl+0x62c/0x1810
[c00020008c2a7df0] [c000000000038d90] system_call_exception+0x190/0x440
[c00020008c2a7e50] [c00000000000d15c] system_call_vectored_common+0x15c/0x2ec
--- interrupt: 3000 at 0x7fff8af5bedc
NIP: 00007fff8af5bedc LR: 00007fff8af5bedc CTR: 0000000000000000
REGS: c00020008c2a7e80 TRAP: 3000 Not tainted (6.10.0-rc3-next-20240612-dirty)
MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 44002482 XER: 00000000
IRQMASK: 0
GPR00: 0000000000000036 00007fffda53b1f0 00007fff8b066d00 0000000000000006
GPR04: 000000008018aee1 00007fffda53b270 0000000000000008 00007fff8ac0e9e0
GPR08: 0000000000000006 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00007fff8b2ca540 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 00000000100101c0
GPR24: 00007fff8b2bf840 00007fff8b2c0000 00007fffda53b728 0000000000000001
GPR28: 00007fffda53b838 0000000000000006 0000000000000001 0000000000000005
NIP [00007fff8af5bedc] 0x7fff8af5bedc
LR [00007fff8af5bedc] 0x7fff8af5bedc
--- interrupt: 3000
Allocated by task 2505:
kasan_save_stack+0x48/0x80
kasan_save_track+0x2c/0x50
kasan_save_alloc_info+0x44/0x60
__kasan_kmalloc+0xd0/0x120
__kmalloc_noprof+0x214/0x670
kvm_vm_ioctl_create_spapr_tce+0x10c/0x420 [kvm]
kvm_arch_vm_ioctl+0x5fc/0x890 [kvm]
kvm_vm_ioctl+0xa54/0x13d0 [kvm]
sys_ioctl+0x62c/0x1810
system_call_exception+0x190/0x440
system_call_vectored_common+0x15c/0x2ec
Freed by task 0:
kasan_save_stack+0x48/0x80
kasan_save_track+0x2c/0x50
kasan_save_free_info+0xac/0xd0
__kasan_slab_free+0x120/0x210
kfree+0xec/0x3e0
release_spapr_tce_table+0xd4/0x11c [kvm]
rcu_core+0x568/0x16a0
handle_softirqs+0x23c/0x920
do_softirq_own_stack+0x6c/0x90
do_softirq_own_stack+0x58/0x90
__irq_exit_rcu+0x218/0x2d0
irq_exit+0x30/0x80
arch_local_irq_restore+0x128/0x230
arch_local_irq_enable+0x1c/0x30
cpuidle_enter_state+0x134/0x5cc
cpuidle_enter+0x6c/0xb0
call_cpuidle+0x7c/0x100
do_idle+0x394/0x410
cpu_startup_entry+0x60/0x70
start_secondary+0x3fc/0x410
start_secondary_prolog+0x10/0x14
Last potentially related work creation:
kasan_save_stack+0x48/0x80
__kasan_record_aux_stack+0xcc/0x130
__call_rcu_common.constprop.0+0x8c/0x8e0
kvm_spapr_tce_release+0x29c/0xbc10 [kvm]
__fput+0x22c/0x630
sys_close+0x70/0xe0
system_call_exception+0x190/0x440
system_call_vectored_common+0x15c/0x2ec
The buggy address belongs to the object at c000200027552c00
which belongs to the cache kmalloc-256 of size 256
The buggy address is located 48 bytes inside of
freed 256-byte region [c000200027552c00, c000200027552d00)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xc000200027551800 pfn:0x20002755
flags: 0x83ffff800000000(node=8|zone=0|lastcpupid=0x7ffff)
page_type: 0xfdffffff(slab)
raw: 083ffff800000000 c000000007010d80 5deadbeef0000122 0000000000000000
raw: c000200027551800 0000000080800078 00000001fdffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
c000200027552b00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
c000200027552b80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>c000200027552c00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
c000200027552c80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
c000200027552d00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Disabling lock debugging due to kernel taint
next prev parent reply other threads:[~2024-06-13 10:56 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-07 1:56 [PATCHES][RFC] rework of struct fd handling Al Viro
2024-06-07 1:59 ` [PATCH 01/19] powerpc: fix a file leak in kvm_vcpu_ioctl_enable_cap() Al Viro
2024-06-07 1:59 ` [PATCH 02/19] lirc: rc_dev_get_from_fd(): fix file leak Al Viro
2024-06-07 15:17 ` Christian Brauner
2024-06-07 1:59 ` [PATCH 03/19] introduce fd_file(), convert all accessors to it Al Viro
2024-06-07 1:59 ` [PATCH 04/19] struct fd: representation change Al Viro
2024-06-07 5:55 ` Amir Goldstein
2024-06-07 1:59 ` [PATCH 05/19] add struct fd constructors, get rid of __to_fd() Al Viro
2024-06-07 1:59 ` [PATCH 06/19] net/socket.c: switch to CLASS(fd) Al Viro
2024-06-07 1:59 ` [PATCH 07/19] introduce struct fderr, convert overlayfs uses to that Al Viro
2024-06-07 1:59 ` [PATCH 08/19] fdget_raw() users: switch to CLASS(fd_raw, ...) Al Viro
2024-06-07 15:20 ` Christian Brauner
2024-06-07 1:59 ` [PATCH 09/19] css_set_fork(): " Al Viro
2024-06-07 15:21 ` Christian Brauner
2024-06-07 1:59 ` [PATCH 10/19] introduce "fd_pos" class Al Viro
2024-06-07 15:21 ` Christian Brauner
2024-06-07 1:59 ` [PATCH 11/19] switch simple users of fdget() to CLASS(fd, ...) Al Viro
2024-06-07 15:26 ` Christian Brauner
2024-06-07 16:10 ` Al Viro
2024-06-07 16:11 ` Al Viro
2024-06-07 21:08 ` Al Viro
2024-06-10 2:44 ` [RFC] potential UAF in kvm_spapr_tce_attach_iommu_group() (was Re: [PATCH 11/19] switch simple users of fdget() to CLASS(fd, ...)) Al Viro
2024-06-12 16:36 ` Linus Torvalds
2024-06-13 10:56 ` Michael Ellerman [this message]
2024-06-10 5:12 ` [RFC] UAF in acrn_irqfd_assign() and vfio_virqfd_enable() Al Viro
2024-06-10 17:03 ` Al Viro
2024-06-10 20:09 ` Alex Williamson
2024-06-10 20:53 ` Al Viro
2024-06-11 23:04 ` Alex Williamson
2024-06-12 2:16 ` Al Viro
2024-06-07 1:59 ` [PATCH 12/19] bpf: switch to CLASS(fd, ...) Al Viro
2024-06-07 15:27 ` Christian Brauner
2024-06-07 1:59 ` [PATCH 13/19] convert vmsplice() " Al Viro
2024-06-07 1:59 ` [PATCH 14/19] finit_module(): convert " Al Viro
2024-06-07 1:59 ` [PATCH 15/19] timerfd: switch " Al Viro
2024-06-07 1:59 ` [PATCH 16/19] do_mq_notify(): " Al Viro
2024-06-07 1:59 ` [PATCH 17/19] simplify xfs_find_handle() a bit Al Viro
2024-06-07 1:59 ` [PATCH 18/19] convert kernel/events/core.c Al Viro
2024-06-07 1:59 ` [PATCH 19/19] deal with the last remaing boolean uses of fd_file() Al Viro
2024-06-07 15:16 ` [PATCH 01/19] powerpc: fix a file leak in kvm_vcpu_ioctl_enable_cap() Christian Brauner
2024-06-07 15:30 ` [PATCHES][RFC] rework of struct fd handling Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878qz9p0vw.fsf@mail.lhotse \
--to=mpe@ellerman.id.au \
--cc=aik@amd.com \
--cc=brauner@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=paulus@ozlabs.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox