linux-kernel - Re: [PATCH v8 06/16] virt: sev-guest: Move SNP Guest command mutex

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240423102829.GCZieNTcHyuAYMcRf5@fat_crate.local>
Date: Tue, 23 Apr 2024 12:28:29 +0200
From: Borislav Petkov <bp@...en8.de>
To: "Nikunj A. Dadhania" <nikunj@....com>
Cc: linux-kernel@...r.kernel.org, thomas.lendacky@....com, x86@...nel.org,
	kvm@...r.kernel.org, mingo@...hat.com, tglx@...utronix.de,
	dave.hansen@...ux.intel.com, pgonda@...gle.com, seanjc@...gle.com,
	pbonzini@...hat.com
Subject: Re: [PATCH v8 06/16] virt: sev-guest: Move SNP Guest command mutex

On Tue, Apr 23, 2024 at 09:52:41AM +0530, Nikunj A. Dadhania wrote:
> SNP guest messaging will be moving as part of sev.c, and Secure TSC code
> will use this mutex.

No, this is all backwards.

You have a *static* function in sev-guest - snp_guest_ioctl- which takes
an exported lock - snp_guest_cmd_lock - in order to synchronize with
other callers which are only in that same sev-guest driver.

Why do you even need the guest messaging in sev.c?

I guess this: "Many of the required functions are implemented in the
sev-guest driver and therefore not available at early boot."

But then your API is misdesigned: the lock should be private to sev.c
and none of the callers should pay attention to grabbing it - the
callers simply call the functions and underneath the locking works
automatically for them - they don't care. Just like any other shared
resource, users see only the API they call and the actual
synchronization is done behind the scenes.

Sounds like you need to go back to the drawing board and think how this
thing should look like.

And when you have it, make sure to explain the commit messages *why* it
is done this way.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette