On Mon, Jul 17, 2017 at 3:22 PM, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 17 July 2017 at 20:54, Kees Cook <keescook@xxxxxxxxxxxx> wrote: >> On Mon, Jul 17, 2017 at 12:32 PM, Rob Herring <robh@xxxxxxxxxx> wrote: >>> On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote: >>>> Document then /chosen/kaslr-seed property (and its interaction with the >>> >>> s/then/the/ >>> >>>> EFI_RNG_PROTOCOL API). >>> >>> "dt-bindings: chosen: ..." for the subject. >> >> I'll send a v2 with these fixed and the Acks added. >> >>>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> >>>> --- >>>> Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++-- >>>> 1 file changed, 20 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt >>>> index dee3f5d9df26..0cdb43b268e5 100644 >>>> --- a/Documentation/devicetree/bindings/chosen.txt >>>> +++ b/Documentation/devicetree/bindings/chosen.txt >>>> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place >>>> for passing data between firmware and the operating system, like boot >>>> arguments. Data in the chosen node does not represent the hardware. >>>> >>>> +The following properties are recognized: >>>> >>>> -stdout-path property >>>> --------------------- >>>> + >>>> +kaslr-seed >>>> +----------- >>> >>> Is there some reason we would not feed this to other things needing >>> entropy and therefore should have a more generic name? >> >> I'll let Ard answer this better than me, but IIRC, he wanted a narrow use. >> > > The reason is that the seed is not used to feed a DRBG (because it is > way to early for that when we lay out the VA space), but different > slices of the u64 are used for randomizing different parts of the > address space, i.e., the top 16 bits are used to randomize the linear > region, bits 48 - 21 for the kernel itself, and the bits below that > for the module region. (The virtual kernel offset is randomized > further by the physical placement modulo 2 MB, but this is done by the > stub, which calls EFI_RNG_PROTOCOL itself so it does not use > /chosen/kaslr-seed) > > This means any use of this seed beyond its intended purpose may leak > information. Is having this value in /proc/device-tree/chosen/kaslr-seed a leak? Should the kernel remove it? > For UEFI systems, we do pass a random seed via a UEFI configuration > table, but this is deliberately kept separate (and uses a UEFI > specific interface, which seemed more appropriate) So if we wanted a seed for non-UEFI systems, we should define a separate property? >>>> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed >>>> +the entropy used to randomize the kernel image base address location. It >>>> +is parsed as a u64 value, e.g. >>> >>> Why limit the size to 64-bit and why does it matter how the data is >>> interpretted? >> >> IIRC, u64 is sufficient. (And note I'm just documenting what exists...) >> > > See above. Seeding a DRBG typically requires more than that, but for > KASLR it is sufficient. > >>>> + >>>> +/ { >>>> + chosen { >>>> + kaslr-seed = <0xfeedbeef 0xc0def00d>; >>>> + }; >>>> +}; >>>> + >>>> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported, >>>> +this value will be overwritten by the EFI stub. >>> >>> Isn't this always true? The bootloader will overwrite. Just in the EFI >>> case, the EFI stub is part of the bootloader from a functional (and not >>> packaging) standpoint. >> >> I thought it best to call this out so that no one could get confused >> if they wanted to understand how to use kaslr-seed with an EFI system. >> (i.e. just implement EFI_RNG_PROTOCOL instead.) >> > > Well, I guess the point being made is that setting this property from > UEFI (or u-boot in EFI mode) is pointless is EFI_RNG_PROTOCOL is > implemented as well. Okay. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html