[PATCH v3 3/5] treewide: use get_random_u32() when possible

Jason A. Donenfeld Jason at zx2c4.com
Thu Oct 6 16:36:48 PDT 2022


On 10/6/22, Christophe Leroy <christophe.leroy at csgroup.eu> wrote:
>
>
> Le 06/10/2022 à 19:31, Christophe Leroy a écrit :
>>
>>
>> Le 06/10/2022 à 19:24, Jason A. Donenfeld a écrit :
>>> Hi Christophe,
>>>
>>> On Thu, Oct 6, 2022 at 11:21 AM Christophe Leroy
>>> <christophe.leroy at csgroup.eu> wrote:
>>>> Le 06/10/2022 à 18:53, Jason A. Donenfeld a écrit :
>>>>> The prandom_u32() function has been a deprecated inline wrapper around
>>>>> get_random_u32() for several releases now, and compiles down to the
>>>>> exact same code. Replace the deprecated wrapper with a direct call to
>>>>> the real function. The same also applies to get_random_int(), which is
>>>>> just a wrapper around get_random_u32().
>>>>>
>>>>> Reviewed-by: Kees Cook <keescook at chromium.org>
>>>>> Acked-by: Toke Høiland-Jørgensen <toke at toke.dk> # for sch_cake
>>>>> Acked-by: Chuck Lever <chuck.lever at oracle.com> # for nfsd
>>>>> Reviewed-by: Jan Kara <jack at suse.cz> # for ext4
>>>>> Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
>>>>> ---
>>>>
>>>>> diff --git a/arch/powerpc/kernel/process.c
>>>>> b/arch/powerpc/kernel/process.c
>>>>> index 0fbda89cd1bb..9c4c15afbbe8 100644
>>>>> --- a/arch/powerpc/kernel/process.c
>>>>> +++ b/arch/powerpc/kernel/process.c
>>>>> @@ -2308,6 +2308,6 @@ void notrace __ppc64_runlatch_off(void)
>>>>>    unsigned long arch_align_stack(unsigned long sp)
>>>>>    {
>>>>>        if (!(current->personality & ADDR_NO_RANDOMIZE) &&
>>>>> randomize_va_space)
>>>>> -             sp -= get_random_int() & ~PAGE_MASK;
>>>>> +             sp -= get_random_u32() & ~PAGE_MASK;
>>>>>        return sp & ~0xf;
>>>>
>>>> Isn't that a candidate for prandom_u32_max() ?
>>>>
>>>> Note that sp is deemed to be 16 bytes aligned at all time.
>>>
>>> Yes, probably. It seemed non-trivial to think about, so I didn't. But
>>> let's see here... maybe it's not too bad:
>>>
>>> If PAGE_MASK is always ~(PAGE_SIZE-1), then ~PAGE_MASK is
>>> (PAGE_SIZE-1), so prandom_u32_max(PAGE_SIZE) should yield the same
>>> thing? Is that accurate? And holds across platforms (this comes up a
>>> few places)? If so, I'll do that for a v4.
>>>
>>
>> On powerpc it is always (from arch/powerpc/include/asm/page.h) :
>>
>> /*
>>   * Subtle: (1 << PAGE_SHIFT) is an int, not an unsigned long. So if we
>>   * assign PAGE_MASK to a larger type it gets extended the way we want
>>   * (i.e. with 1s in the high bits)
>>   */
>> #define PAGE_MASK      (~((1 << PAGE_SHIFT) - 1))
>>
>> #define PAGE_SIZE        (1UL << PAGE_SHIFT)
>>
>>
>> So it would work I guess.
>
> But taking into account that sp must remain 16 bytes aligned, would it
> be better to do something like ?
>
> 	sp -= prandom_u32_max(PAGE_SIZE >> 4) << 4;
>
> 	return sp;

Does this assume that sp is already aligned at the beginning of the
function? I'd assume from the function's name that this isn't the
case?

Jason



More information about the linux-um mailing list