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]
Date: Wed, 20 Mar 2024 12:24:54 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Yamahata, Isaku" <isaku.yamahata@...el.com>
CC: "jgross@...e.com" <jgross@...e.com>, "Hansen, Dave"
	<dave.hansen@...el.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
	"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
	"peterz@...radead.org" <peterz@...radead.org>, "hpa@...or.com"
	<hpa@...or.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "kirill.shutemov@...ux.intel.com"
	<kirill.shutemov@...ux.intel.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "isaku.yamahata@...ux.intel.com"
	<isaku.yamahata@...ux.intel.com>, "mingo@...hat.com" <mingo@...hat.com>
Subject: Re: [PATCH 4/5] x86/virt/tdx: Support global metadata read for all
 element sizes


> > @@ -295,11 +307,30 @@ static int read_sys_metadata_field16(u64 field_id,
> >  struct field_mapping {
> >  	u64 field_id;
> >  	int offset;
> > +	int size;
> >  };
> >  
> >  #define TD_SYSINFO_MAP(_field_id, _struct, _member)	\
> >  	{ .field_id = MD_FIELD_ID_##_field_id,		\
> > -	  .offset   = offsetof(_struct, _member) }
> > +	  .offset   = offsetof(_struct, _member),	\
> > +	  .size     = sizeof(typeof(((_struct *)0)->_member)) }
> 
> Because we use compile time constant for _field_id mostly, can we add build
> time check? Something like this.
> 
> static inline metadata_size_check(u64 field_id, size_t size)
> {
>         BUILD_BUG_ON(get_metadata_field_bytes(field_id) != size);
> }
> 
> #define TD_SYSINFO_MAP(_field_id, _struct, _member)	\
> 	{ .field_id = MD_FIELD_ID_##_field_id,		\
> 	  .offset   = offsetof(_struct, _member),	\
> 	  .size     = \
> 		({ size_t s = sizeof(typeof(((_struct *)0)->_member)); \
>                 metadata_size_check(MD_FIELD_ID_##_field_id, s); \
>                 s; }) }
> 

Hmm.. The problem is "mostly" as you mentioned?

My understanding is BUILD_BUG_ON() relies on the "condition" to be compile-time
constant.  In your KVM TDX patchset there's code to do ...

	for (i = 0; i < NR_WHATEVER; i++) {
		const struct tdx_metadata_field_mapping fields = {
			TD_SYSINFO_MAP(FIELD_WHATEVERE + i, ...),
			...
		};

		...
	}

To be honest I am not exactly sure whether the compiler can determine
"FIELD_WHATEVER + i" as compile-time constant.

Btw, if there's any mismatch, the current code can already catch during runtime.
I think one purpose (perhaps the most important purpose) of BUILD_BUG_ON() is to
catch bug early if someone changed the constant (macros etc) in the "condition".
But in our case, once written no one is going to change the structure or the
metadata fields.  So I am not sure whether it's worth to do.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ