Excerpts from Christophe Leroy's message of September 9, 2020 4:20 pm:
>
>
> Le 09/09/2020 à 08:04, Aneesh Kumar K.V a écrit :
>> Christophe Leroy <christophe.le...@csgroup.eu> writes:
>>
>>> search_exception_tables() is an heavy operation, we have to avoid it.
>>> When KUAP is selected, we'll know the fault has been blocked by KUAP.
>>> Otherwise, it behaves just as if the address was already in the TLBs
>>> and no fault was generated.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu>
>>> ---
>>> arch/powerpc/mm/fault.c | 20 +++++---------------
>>> 1 file changed, 5 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>> index 525e0c2b5406..edde169ba3a6 100644
>>> --- a/arch/powerpc/mm/fault.c
>>> +++ b/arch/powerpc/mm/fault.c
>>> @@ -214,24 +214,14 @@ static bool bad_kernel_fault(struct pt_regs *regs,
>>> unsigned long error_code,
>>> if (address >= TASK_SIZE)
>>> return true;
>>>
>>> - if (!is_exec && (error_code & DSISR_PROTFAULT) &&
>>> - !search_exception_tables(regs->nip)) {
>>> + // Read/write fault blocked by KUAP is bad, it can never succeed.
>>> + if (bad_kuap_fault(regs, address, is_write)) {
>>> pr_crit_ratelimited("Kernel attempted to access user page (%lx)
>>> - exploit attempt? (uid: %d)\n",
>>> - address,
>>> - from_kuid(&init_user_ns, current_uid()));
>>> - }
>>> -
>>> - // Fault on user outside of certain regions (eg. copy_tofrom_user()) is
>>> bad
>>> - if (!search_exception_tables(regs->nip))
>>> - return true;
>>
>> We still need to keep this ? Without that we detect the lack of
>> exception tables pretty late.
>
> Is that a problem at all to detect the lack of exception tables late ?
> That case is very unlikely and will lead to failure anyway. So, is it
> worth impacting performance of the likely case which will always have an
> exception table and where we expect the exception to run as fast as
> possible ?
>
> The other architectures I have looked at (arm64 and x86) only have the
> exception table search together with the down_read_trylock(&mm->mmap_sem).
Yeah I don't see how it'd be a problem. User could arrange for page
table to already be at this address and avoid the fault so it's not the
right way to stop an attacker, KUAP is.
Thanks,
Nick
