lkml.org 
[lkml]   [2019]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2019-10-07 18:27    [W:0.112 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site