linux-kernel - Re: [PATCH] crypto: ccp - Have it depend on AMD

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: <20240211201831.GBZckrl84xVo-ILnOJ@fat_crate.local>
Date: Sun, 11 Feb 2024 21:18:31 +0100
From: Borislav Petkov <bp@...en8.de>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: Ashish Kalra <ashish.kalra@....com>,
	Michael Roth <michael.roth@....com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: ccp - Have it depend on AMD_IOMMU

On Fri, Feb 09, 2024 at 11:51:37AM -0600, Tom Lendacky wrote:
> Or should the ifdef around amd_iommu_snp_disable() in
> include/linux/amd-iommu.h instead be:
> 
> #if defined(CONFIG_AMD_IOMMU) && defined(CONFIG_KVM_AMD_SEV)

Pretty much, except the thunk should say that it is the thunk that gets
called:

#if defined(CONFIG_AMD_IOMMU) && defined(CONFIG_KVM_AMD_SEV)
int amd_iommu_snp_disable(void);
#else
static inline int amd_iommu_snp_disable(void) { return -ENODEV; }
#endif

and return -ENODEV to denote that so that the caller doesn't wonder
what's going on.

> I think this would be better in case these should be referenced
> elsewhere in the future.

However, looking at all that code, its design still looks iffy to me.

iommu_page_make_shared() and amd_iommu_snp_disable() is functionality
which the *IOMMU* provides in order to deal with SEV* guests. So that
functionality should be behind

CONFIG_AMD_IOMMU_SEV

or so, on which all its users depend or select. I'd prefer depend tho.

Putting IOMMU facilities around a KVM Kconfig symbol CONFIG_KVM_AMD_SEV
is simply bad design. There are providers of functionality which is
behind Kconfig symbols and users which depend on those. The current
thing is a hack IMNSVHO.

Thx.

-- 
Regards/Gruss,
    Boris.

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