get_user_pages() and EXEC_ONLY mapping.
Hello,
Some architectures can now support EXEC_ONLY mappings and I am wondering what get_user_pages() on those addresses should return. Earlier PROT_EXEC implied PROT_READ and pte_access_permitted() returned true for that. But arm64 does have this explicit comment that says
/*
* p??_access_permitted() is true for valid user mappings (PTE_USER
* bit set, subject to the write permission check). For execute-only
* mappings, like PROT_EXEC with EPAN (both PTE_USER and PTE_UXN bits
* not set) must return false. PROT_NONE mappings do not have the
* PTE_VALID bit set.
*/
Is that correct? We should be able to get struct page for PROT_EXEC mappings?
-aneesh
Quoted 5 lines · Aneesh Kumar K.V
Hello,
Some architectures can now support
EXEC_ONLYmappings and I am wondering whatget_user_pages()on those addresses should return.
-EPERM
Quoted 12 lines
Earlier
PROT_EXECimpliedPROT_READandpte_access_permitted()returned true for that. But arm64 does have this explicit comment that says/*
* p??_access_permitted() is true for valid user mappings (PTE_USER
* bit set, subject to the write permission check). For execute-only
* mappings, likePROT_EXECwith EPAN (bothPTE_USERandPTE_UXNbits
* not set) must return false.PROT_NONEmappings do not have the
*PTE_VALIDbit set.
*/
Quoted 2 lines
Is that correct? We should be able to get struct page for
PROT_EXECmappings?
If the memory is unreadable then providing a back door through O_DIRECT and everthing else to read it sounds wrong to me.
If there is some case where a get_user_pages caller is exec-only compatible then a new FOLL_EXEC flag to permit it would make sense.
Jason
Quoted 21 lines · Jason Gunthorpe
Quoted 5 lines · Aneesh Kumar K.V
Hello,
Some architectures can now support
EXEC_ONLYmappings and I am wondering whatget_user_pages()on those addresses should return.
-EPERMQuoted 12 lines
Earlier
PROT_EXECimpliedPROT_READandpte_access_permitted()returned true for that. But arm64 does have this explicit comment that says/*
* p??_access_permitted() is true for valid user mappings (PTE_USER
* bit set, subject to the write permission check). For execute-only
* mappings, likePROT_EXECwith EPAN (bothPTE_USERandPTE_UXNbits
* not set) must return false.PROT_NONEmappings do not have the
*PTE_VALIDbit set.
*/
Quoted 9 lines
Quoted 2 lines
Is that correct? We should be able to get struct page for
PROT_EXECmappings?If the memory is unreadable then providing a back door through
O_DIRECTand everthing else to read it sounds wrong to me.If there is some case where a
get_user_pagescaller is exec-only compatible then a newFOLL_EXECflag to permit it would make sense.
I was expecting pin_user_pages() to return -EPERM and get_user_pages() return struct page. This was based on Documentation/core-api/pin_user_pages.rst
"Another way of thinking about these flags is as a progression of restrictions:FOLL_GET is for struct page manipulation, without affecting the data that the struct page refers to. FOLL_PIN is a *replacement* for FOLL_GET, and is for short term pins on pages whose data *will* get accessed. "
May be we can clarify PROT_EXEC details in the documentation?
-aneesh
Quoted 28 lines · Aneesh Kumar K V
Quoted 27 lines · Jason Gunthorpe
Quoted 5 lines · Aneesh Kumar K.V
Hello,
Some architectures can now support
EXEC_ONLYmappings and I am wondering whatget_user_pages()on those addresses should return.
-EPERMQuoted 12 lines
Earlier
PROT_EXECimpliedPROT_READandpte_access_permitted()returned true for that. But arm64 does have this explicit comment that says/*
* p??_access_permitted() is true for valid user mappings (PTE_USER
* bit set, subject to the write permission check). For execute-only
* mappings, likePROT_EXECwith EPAN (bothPTE_USERandPTE_UXNbits
* not set) must return false.PROT_NONEmappings do not have the
*PTE_VALIDbit set.
*/Quoted 2 lines
Is that correct? We should be able to get struct page for
PROT_EXECmappings?If the memory is unreadable then providing a back door through
O_DIRECTand everthing else to read it sounds wrong to me.
Quoted 6 lines
Quoted 3 lines
If there is some case where a
get_user_pagescaller is exec-only compatible then a newFOLL_EXECflag to permit it would make sense.I was expecting
pin_user_pages()to return-EPERMandget_user_pages()return struct page. This was based onDocumentation/core-api/pin_user_pages.rst
Not unconditionally but you could argue that FOLL_GET should succeed. It depends how much do you care about absolute security of unreadable memory vs compatability.
Quoted 4 lines
"Another way of thinking about these flags is as a progression of restrictions:
FOLL_GETis for struct page manipulation, without affecting the data that the struct page refers to.FOLL_PINis a *replacement* forFOLL_GET, and is for short term pins on pages whose data *will* get accessed. "
This was sort of aspirational - have we got rid of all the FOLL_GET users that are touching the data? Looks like no from a quick check..
Jason
Quoted 13 lines · Aneesh Kumar K.V
Some architectures can now support
EXEC_ONLYmappings and I am wondering whatget_user_pages()on those addresses should return. EarlierPROT_EXECimpliedPROT_READandpte_access_permitted()returned true for that. But arm64 does have this explicit comment that says/*
* p??_access_permitted() is true for valid user mappings (PTE_USER
* bit set, subject to the write permission check). For execute-only
* mappings, likePROT_EXECwith EPAN (bothPTE_USERandPTE_UXNbits
* not set) must return false.PROT_NONEmappings do not have the
*PTE_VALIDbit set.
*/
Quoted 2 lines
Is that correct? We should be able to get struct page for
PROT_EXECmappings?
I don't remember why we ended up with this briefly looking at the code, pte_access_permitted() is only used on the fast GUP path. On the slow path, there is a check_vma_flags() call which returns -EFAULT if the vma is not readable. So the pte_access_permitted() on the fast path matches the semantics of the slow path.
If one wants the page structure, FOLL_FORCE ignores the read check (on the slow path), though I think it still fails if VM_MAYREAD is not set. Unless you have a real use-case where this is not sufficient, I'd leave the behaviour as is on arm64 (and maybe update other architectures that support exec-only to do the same).
Signature · 1 line
Catalin