Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Mar 2023 12:15:21 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     "Dae R. Jeong" <threeearcat@gmail.com>
Cc:     bryantan@vmware.com, vdasa@vmware.com, pv-drivers@vmware.com,
        arnd@arndb.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vmci_host: fix a race condition in vmci_host_poll()
 causing GPF
Message-ID: <ZCQPuaps5rCtGAb2@kroah.com>
References: <ZCGFsdBAU4cYww5l@dragonet>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZCGFsdBAU4cYww5l@dragonet>
Precedence: bulk

On Mon, Mar 27, 2023 at 09:01:53PM +0900, Dae R. Jeong wrote:
> During fuzzing, a general protection fault is observed in
> vmci_host_poll().
> 
> general protection fault, probably for non-canonical address 0xdffffc0000000019: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x00000000000000c8-0x00000000000000cf]
> RIP: 0010:__lock_acquire+0xf3/0x5e00 kernel/locking/lockdep.c:4926
> <- omitting registers ->
> Call Trace:
>  <TASK>
>  lock_acquire+0x1a4/0x4a0 kernel/locking/lockdep.c:5672
>  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>  _raw_spin_lock_irqsave+0xb3/0x100 kernel/locking/spinlock.c:162
>  add_wait_queue+0x3d/0x260 kernel/sched/wait.c:22
>  poll_wait include/linux/poll.h:49 [inline]
>  vmci_host_poll+0xf8/0x2b0 drivers/misc/vmw_vmci/vmci_host.c:174
>  vfs_poll include/linux/poll.h:88 [inline]
>  do_pollfd fs/select.c:873 [inline]
>  do_poll fs/select.c:921 [inline]
>  do_sys_poll+0xc7c/0x1aa0 fs/select.c:1015
>  __do_sys_ppoll fs/select.c:1121 [inline]
>  __se_sys_ppoll+0x2cc/0x330 fs/select.c:1101
>  do_syscall_x64 arch/x86/entry/common.c:51 [inline]
>  do_syscall_64+0x4e/0xa0 arch/x86/entry/common.c:82
>  entry_SYSCALL_64_after_hwframe+0x46/0xb0
> 
> Example thread interleaving that causes the general protection fault
> is as follows:
> 
> CPU1 (vmci_host_poll)               CPU2 (vmci_host_do_init_context)
> -----                               -----
> // Read uninitialized context
> context = vmci_host_dev->context;
>                                     // Initialize context
>                                     vmci_host_dev->context = vmci_ctx_create();
>                                     vmci_host_dev->ct_type = VMCIOBJ_CONTEXT;
> 
> if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) {
>     // Dereferencing the wrong pointer
>     poll_wait(..., &context->host_context);
> }
> 
> In this scenario, vmci_host_poll() reads vmci_host_dev->context first,
> and then reads vmci_host_dev->ct_type to check that
> vmci_host_dev->context is initialized. However, since these two reads
> are not atomically executed, there is a chance of a race condition as
> described above.
> 
> To fix this race condition, read vmci_host_dev->context after checking
> the value of vmci_host_dev->ct_type so that vmci_host_poll() always
> reads an initialized context.
> 
> Reported-by: Dae R. Jeong <threeearcat@gmail.com>
> Fixes: 8bf503991f87 ("VMCI: host side driver implementation.")
> Signed-off-by: Dae R. Jeong <threeearcat@gmail.com>

If you author and sign-off on the patch, no need for a Reported-by: as
that is obvious :)

And how did you test this change?

thanks,

greg k-h