On Mon, Jan 18, 2021 at 09:45:30AM +0000, Marc Zyngier wrote:
> Given that the early cpufeature infrastructure has borrowed quite
> a lot of code from the kaslr implementation, let's reimplement
> the matching of the "nokaslr" option with it.
>
> Signed-off-by: Marc Zyngier <[email protected]>
Acked-by: David Brazdil <[email protected]>
> ---
> arch/arm64/kernel/idreg-override.c | 17 ++++++++++++++
> arch/arm64/kernel/kaslr.c | 37 +++---------------------------
> 2 files changed, 20 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> index 1db54878b2c4..143fe7b8e3ce 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -33,8 +33,24 @@ static const struct reg_desc mmfr1 __initdata = {
> },
> };
>
> +extern u64 kaslr_feature_val;
> +extern u64 kaslr_feature_mask;
> +
> +static const struct reg_desc kaslr __initdata = {
> + .name = "kaslr",
> +#ifdef CONFIG_RANDOMIZE_BASE
> + .val = &kaslr_feature_val,
> + .mask = &kaslr_feature_mask,
> +#endif
> + .fields = {
> + { "disabled", 0 },
> + {}
> + },
> +};
> +
> static const struct reg_desc * const regs[] __initdata = {
> &mmfr1,
> + &kaslr,
> };
>
> static const struct {
> @@ -43,6 +59,7 @@ static const struct {
> } aliases[] __initdata = {
> { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" },
> { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" },
> + { "nokaslr", "kaslr.disabled=1" },
> };
>
> static int __init find_field(const char *cmdline, const struct reg_desc *reg,
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 5fc86e7d01a1..dcc4a5aadbe2 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -51,39 +51,8 @@ static __init u64 get_kaslr_seed(void *fdt)
> return ret;
> }
>
> -static __init bool cmdline_contains_nokaslr(const u8 *cmdline)
> -{
> - const u8 *str;
> -
> - str = strstr(cmdline, "nokaslr");
> - return str == cmdline || (str > cmdline && *(str - 1) == ' ');
> -}
> -
> -static __init bool is_kaslr_disabled_cmdline(void *fdt)
> -{
> - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> - int node;
> - const u8 *prop;
> -
> - node = fdt_path_offset(fdt, "/chosen");
> - if (node < 0)
> - goto out;
> -
> - prop = fdt_getprop(fdt, node, "bootargs", NULL);
> - if (!prop)
> - goto out;
> -
> - if (cmdline_contains_nokaslr(prop))
> - return true;
> -
> - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
> - goto out;
> -
> - return false;
> - }
> -out:
> - return cmdline_contains_nokaslr(CONFIG_CMDLINE);
> -}
> +u64 kaslr_feature_val __initdata;
> +u64 kaslr_feature_mask __initdata;
>
> /*
> * This routine will be executed with the kernel mapped at its default virtual
> @@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
> * Check if 'nokaslr' appears on the command line, and
> * return 0 if that is the case.
> */
> - if (is_kaslr_disabled_cmdline(fdt)) {
> + if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
nit: Isn't the 0xf redundant here? You don't re-mask for VH either.
> kaslr_status = KASLR_DISABLED_CMDLINE;
> return 0;
> }
> --
> 2.29.2
>
On Mon, 18 Jan 2021 14:46:36 +0000,
David Brazdil <[email protected]> wrote:
>
> On Mon, Jan 18, 2021 at 09:45:30AM +0000, Marc Zyngier wrote:
> > Given that the early cpufeature infrastructure has borrowed quite
> > a lot of code from the kaslr implementation, let's reimplement
> > the matching of the "nokaslr" option with it.
> >
> > Signed-off-by: Marc Zyngier <[email protected]>
> Acked-by: David Brazdil <[email protected]>
[...]
> > @@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
> > * Check if 'nokaslr' appears on the command line, and
> > * return 0 if that is the case.
> > */
> > - if (is_kaslr_disabled_cmdline(fdt)) {
> > + if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
>
> nit: Isn't the 0xf redundant here? You don't re-mask for VH either.
Actually, I do. See the two back to back ubfx that extract both the
mask and the feature. The "& 0xf" here serves the same purpose.
Is it redundant? At the moment, quite possibly. But since we have
space for 16 "features", this is an indication that we are only using
the first one. I expect that eventually, we'll use it for other
things.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.