Re: [PATCH v7 24/41] mm: Don't allow write GUPs to shadow stack memory

[Andy Lutomirski]

On Mon, Mar 6, 2023, at 10:33 AM, Edgecombe, Rick P wrote:
> On Mon, 2023-03-06 at 10:15 -0800, Andy Lutomirski wrote:
>> On Mon, Mar 6, 2023 at 5:10 AM Borislav Petkov <bp@xxxxxxxxx> wrote:
>> > 
>> > On Mon, Feb 27, 2023 at 02:29:40PM -0800, Rick Edgecombe wrote:
>> > > The x86 Control-flow Enforcement Technology (CET) feature
>> > > includes a new
>> > > type of memory called shadow stack. This shadow stack memory has
>> > > some
>> > > unusual properties, which requires some core mm changes to
>> > > function
>> > > properly.
>> > > 
>> > > Shadow stack memory is writable only in very specific, controlled
>> > > ways.
>> > > However, since it is writable, the kernel treats it as such. As a
>> > > result
>> > 
>> >                                                                    
>> >         ^
>> >                                                                    
>> >         ,
>> > 
>> > > there remain many ways for userspace to trigger the kernel to
>> > > write to
>> > > shadow stack's via get_user_pages(, FOLL_WRITE) operations. To
>> > > make this a
>> 
>> Is there an alternate mechanism, or do we still want to allow
>> FOLL_FORCE so that debuggers can write it?
>
> Yes, GDB shadow stack support uses it via both ptrace poke and
> /proc/pid/mem apparently. So some ability to write through is needed
> for debuggers. But not CRIU actually. It uses WRSS.
>
> There was also some discussion[0] previously about how apps might
> prefer to block /proc/self/mem for general security reasons. Blocking
> shadow stack writes while you allow text writes is probably not that
> impactful security-wise. So I thought it would be better to leave the
> logic simpler. Then when /proc/self/mem could be locked down per the
> discussion, shadow stack can be locked down the same way.

Ah, I am guilty of reading your changelog but not the code.

You said:

Shadow stack memory is writable only in very specific, controlled ways.
However, since it is writable, the kernel treats it as such. As a result
there remain many ways for userspace to trigger the kernel to write to
shadow stack's via get_user_pages(, FOLL_WRITE) operations. To make this a
little less exposed, block writable GUPs for shadow stack VMAs.

I read that as *denying* FOLL_FORCE.  Maybe clarify the changelog?

>
> [0] 
> https://lore.kernel.org/lkml/E857CF98-EEB2-4F83-8305-0A52B463A661@xxxxxxxxxx/