linux-kernel - Re: [PATCH v7 3/7] LoongArch: KVM: Add cpucfg area for kvm hypervisor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b7d91c1a42470821983a811a16f9fc1c55cfe699.camel@xry111.site>
Date: Tue, 02 Apr 2024 10:49:55 +0800
From: Xi Ruoyao <xry111@...111.site>
To: maobibo <maobibo@...ngson.cn>, WANG Xuerui <kernel@...0n.name>, Huacai
 Chen <chenhuacai@...nel.org>, Tianrui Zhao <zhaotianrui@...ngson.cn>,
 Juergen Gross <jgross@...e.com>, Paolo Bonzini <pbonzini@...hat.com>,
 Jonathan Corbet <corbet@....net>
Cc: loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	virtualization@...ts.linux.dev, kvm@...r.kernel.org
Subject: Re: [PATCH v7 3/7] LoongArch: KVM: Add cpucfg area for kvm
 hypervisor

On Tue, 2024-04-02 at 09:43 +0800, maobibo wrote:
> > Sorry for the late reply, but I think it may be a bit non-constructive 
> > to repeatedly submit the same code without due explanation in our 
> > previous review threads. Let me try to recollect some of the details
> > though...
> Because your review comments about hypercall method is wrong, I need not 
> adopt it.

Again it's unfair to say so considering the lack of LVZ documentation.

/* snip */

> 
> 1. T0-T7 are scratch registers during SYSCALL ABI, this is what you 
> suggest, does there exist information leaking to user space from T0-T7
> registers?

It's not a problem.  When syscall returns RESTORE_ALL_AND_RET is invoked
despite T0-T7 are not saved.  So a "junk" value will be read from the
leading PT_SIZE bytes of the kernel stack for this thread.

The leading PT_SIZE bytes of the kernel stack is dedicated for storing
the struct pt_regs representing the reg file of the thread in the
userspace.

Thus we may only read out the userspace T0-T7 value stored when the same
thread was interrupted or trapped last time, or 0 (if the thread was
never interrupted or trapped before).

And it's impossible to read some data used by the kernel internally, or
some data of another thread.

But indeed there is some improvement here.  Zeroing these registers
seems cleaner than reading out the junk values, and also faster (move
$t0, $r0 is faster than ld.d $t0, $sp, PT_R12).  Not sure if it's worthy
to violate Huacai's "keep things simple" aspiration though.

-- 
Xi Ruoyao <xry111@...111.site>
School of Aerospace Science and Technology, Xidian University