Messages in this thread | | | Date | Mon, 7 Oct 2019 18:25:51 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison |
| |
On Mon, Oct 07, 2019 at 02:38:18PM +0200, Ralf Ramsauer wrote: > We will soon introduce a new setup_data version and extend the
Who is "We"?
There a couple more "we" below. Can you please rewrite that commit message in passive voice and thus dispense my confusion about who's "we"? :)
> structure. This requires some preparational work for the sanity check of > the header and the check of the version. > > Use the following strategy: > > 1. Ensure that the header declares at least enough space for the version > and the compatible_version as we must hold that fields for any > version. Furthermore, the location and semantics of those fields will > never change. > > 2. Copy over data -- as much as we can. The length is either limited by > the header length, or the length of setup_data. > > 3. Things are now in place -- sanity check if the header length complies > the actual version. > > For future versions of the setup_data, only step 3 requires alignment. > > Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> > Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/x86/include/uapi/asm/bootparam.h | 22 +++++++----- > arch/x86/kernel/jailhouse.c | 50 +++++++++++++++++---------- > 2 files changed, 44 insertions(+), 28 deletions(-)
...
> @@ -180,13 +175,26 @@ static void __init jailhouse_init_platform(void) > if (!pa_data) > panic("Jailhouse: No valid setup data found"); > > - if (setup_data.compatible_version > JAILHOUSE_SETUP_REQUIRED_VERSION) > - panic("Jailhouse: Unsupported setup data structure"); > + /* setup data must at least contain the header */ > + if (header.len < sizeof(setup_data.hdr)) > + goto unsupported; > > - pmtmr_ioport = setup_data.pm_timer_address; > + pa_data += offsetof(struct setup_data, data); > + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len);
Checkpatch makes sense here:
WARNING: min() should probably be min_t(unsigned long, sizeof(setup_data), header.len) #165: FILE: arch/x86/kernel/jailhouse.c:183: + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len);
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |