* pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-08 12:05 Florian Weimer 2018-11-08 14:57 ` Dave Hansen 2018-11-08 19:22 ` Ram Pai 0 siblings, 2 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-08 12:05 UTC (permalink / raw) To: linux-api, linux-mm; +Cc: dave.hansen, linuxram Would it be possible to reserve a bit for PKEY_DISABLE_READ? I think the POWER implementation can disable read access at the hardware level, but not write access, and that cannot be expressed with the current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. My upcoming POWER implementation of pkey_set and pkey_get in glibc would benefit from this. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 12:05 pkeys: Reserve PKEY_DISABLE_READ Florian Weimer @ 2018-11-08 14:57 ` Dave Hansen 2018-11-08 15:01 ` Florian Weimer 2018-11-08 19:22 ` Ram Pai 1 sibling, 1 reply; 42+ messages in thread From: Dave Hansen @ 2018-11-08 14:57 UTC (permalink / raw) To: Florian Weimer, linux-api, linux-mm; +Cc: linuxram On 11/8/18 4:05 AM, Florian Weimer wrote: > Would it be possible to reserve a bit for PKEY_DISABLE_READ? > > I think the POWER implementation can disable read access at the hardware > level, but not write access, and that cannot be expressed with the > current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. Do you just mean in the syscall interfaces? What would we need to do on x86 if we see the bit? Would we just say it's invalid on x86, or would we make sure that PKEY_DISABLE_ACCESS==PKEY_DISABLE_READ? ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 14:57 ` Dave Hansen @ 2018-11-08 15:01 ` Florian Weimer 2018-11-08 17:14 ` Dave Hansen 0 siblings, 1 reply; 42+ messages in thread From: Florian Weimer @ 2018-11-08 15:01 UTC (permalink / raw) To: Dave Hansen; +Cc: linux-api, linux-mm, linuxram * Dave Hansen: > On 11/8/18 4:05 AM, Florian Weimer wrote: >> Would it be possible to reserve a bit for PKEY_DISABLE_READ? >> >> I think the POWER implementation can disable read access at the hardware >> level, but not write access, and that cannot be expressed with the >> current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. > > Do you just mean in the syscall interfaces? What would we need to do on > x86 if we see the bit? Would we just say it's invalid on x86, or would > we make sure that PKEY_DISABLE_ACCESS==PKEY_DISABLE_READ? Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a line PKEY_DISABLE_READ would result in an EINVAL failure. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 15:01 ` Florian Weimer @ 2018-11-08 17:14 ` Dave Hansen 2018-11-08 17:37 ` Florian Weimer 2018-11-08 20:08 ` Ram Pai 0 siblings, 2 replies; 42+ messages in thread From: Dave Hansen @ 2018-11-08 17:14 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-api, linux-mm, linuxram On 11/8/18 7:01 AM, Florian Weimer wrote: > Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | > PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a > line PKEY_DISABLE_READ would result in an EINVAL failure. Sounds reasonable to me. I don't see any urgency to do this right now. It could easily go in alongside the ppc patches when those get merged. The only thing I'd suggest is that we make it something slightly higher than 0x4. It'll make the code easier to deal with in the kernel if we have the ABI and the hardware mirror each other, and if we pick 0x4 in the ABI for PKEY_DISABLE_READ, it might get messy if the harware choose 0x4 for PKEY_DISABLE_EXECUTE or something. So, let's make it 0x80 or something on x86 at least. Also, I'll be happy to review and ack the patch to do this, but I'd expect the ppc guys (hi Ram!) to actually put it together. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 17:14 ` Dave Hansen @ 2018-11-08 17:37 ` Florian Weimer 2018-11-08 20:12 ` Ram Pai 2018-11-08 20:08 ` Ram Pai 1 sibling, 1 reply; 42+ messages in thread From: Florian Weimer @ 2018-11-08 17:37 UTC (permalink / raw) To: Dave Hansen; +Cc: linux-api, linux-mm, linuxram * Dave Hansen: > On 11/8/18 7:01 AM, Florian Weimer wrote: >> Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | >> PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a >> line PKEY_DISABLE_READ would result in an EINVAL failure. > > Sounds reasonable to me. > > I don't see any urgency to do this right now. It could easily go in > alongside the ppc patches when those get merged. POWER support has already been merged, so we need to do something here now, before I can complete the userspace side. > The only thing I'd suggest is that we make it something slightly > higher than 0x4. It'll make the code easier to deal with in the > kernel if we have the ABI and the hardware mirror each other, and if > we pick 0x4 in the ABI for PKEY_DISABLE_READ, it might get messy if > the harware choose 0x4 for PKEY_DISABLE_EXECUTE or something. > > So, let's make it 0x80 or something on x86 at least. I don't have a problem with that if that's what it takes. > Also, I'll be happy to review and ack the patch to do this, but I'd > expect the ppc guys (hi Ram!) to actually put it together. Ram, do you want to write a patch? I'll promise I finish the glibc support for this. 8-) Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 17:37 ` Florian Weimer @ 2018-11-08 20:12 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-08 20:12 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api On Thu, Nov 08, 2018 at 06:37:41PM +0100, Florian Weimer wrote: > * Dave Hansen: > > > On 11/8/18 7:01 AM, Florian Weimer wrote: > >> Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | > >> PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a > >> line PKEY_DISABLE_READ would result in an EINVAL failure. > > > > Sounds reasonable to me. > > > > I don't see any urgency to do this right now. It could easily go in > > alongside the ppc patches when those get merged. > > POWER support has already been merged, so we need to do something here > now, before I can complete the userspace side. > > > The only thing I'd suggest is that we make it something slightly > > higher than 0x4. It'll make the code easier to deal with in the > > kernel if we have the ABI and the hardware mirror each other, and if > > we pick 0x4 in the ABI for PKEY_DISABLE_READ, it might get messy if > > the harware choose 0x4 for PKEY_DISABLE_EXECUTE or something. > > > > So, let's make it 0x80 or something on x86 at least. > > I don't have a problem with that if that's what it takes. > > > Also, I'll be happy to review and ack the patch to do this, but I'd > > expect the ppc guys (hi Ram!) to actually put it together. > > Ram, do you want to write a patch? Florian, I can. But I am struggling to understand the requirement. Why is this needed? Are we proposing a enhancement to the sys_pkey_alloc(), to be able to allocate keys that are initialied to disable-read only? RP > > I'll promise I finish the glibc support for this. 8-) > > Thanks, > Florian -- Ram Pai ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-08 20:12 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-08 20:12 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-api, linux-mm, linuxppc-dev On Thu, Nov 08, 2018 at 06:37:41PM +0100, Florian Weimer wrote: > * Dave Hansen: > > > On 11/8/18 7:01 AM, Florian Weimer wrote: > >> Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | > >> PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a > >> line PKEY_DISABLE_READ would result in an EINVAL failure. > > > > Sounds reasonable to me. > > > > I don't see any urgency to do this right now. It could easily go in > > alongside the ppc patches when those get merged. > > POWER support has already been merged, so we need to do something here > now, before I can complete the userspace side. > > > The only thing I'd suggest is that we make it something slightly > > higher than 0x4. It'll make the code easier to deal with in the > > kernel if we have the ABI and the hardware mirror each other, and if > > we pick 0x4 in the ABI for PKEY_DISABLE_READ, it might get messy if > > the harware choose 0x4 for PKEY_DISABLE_EXECUTE or something. > > > > So, let's make it 0x80 or something on x86 at least. > > I don't have a problem with that if that's what it takes. > > > Also, I'll be happy to review and ack the patch to do this, but I'd > > expect the ppc guys (hi Ram!) to actually put it together. > > Ram, do you want to write a patch? Florian, I can. But I am struggling to understand the requirement. Why is this needed? Are we proposing a enhancement to the sys_pkey_alloc(), to be able to allocate keys that are initialied to disable-read only? RP > > I'll promise I finish the glibc support for this. 8-) > > Thanks, > Florian -- Ram Pai ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 20:12 ` Ram Pai @ 2018-11-08 20:23 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-08 20:23 UTC (permalink / raw) To: Ram Pai; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api * Ram Pai: > Florian, > > I can. But I am struggling to understand the requirement. Why is > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > to be able to allocate keys that are initialied to disable-read > only? Yes, I think that would be a natural consequence. However, my immediate need comes from the fact that the AMR register can contain a flag combination that is not possible to represent with the existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code could write to AMR directly, so I cannot rule out that certain flag combinations exist there. So I came up with this: int pkey_get (int key) { if (key < 0 || key > PKEY_MAX) { __set_errno (EINVAL); return -1; } unsigned int index = pkey_index (key); unsigned long int amr = pkey_read (); unsigned int bits = (amr >> index) & 3; /* Translate from AMR values. PKEY_AMR_READ standing alone is not currently representable. */ if (bits & PKEY_AMR_READ) return PKEY_DISABLE_ACCESS; else if (bits == PKEY_AMR_WRITE) return PKEY_DISABLE_WRITE; return 0; } And this is not ideal. I would prefer something like this instead: switch (bits) { case PKEY_AMR_READ | PKEY_AMR_WRITE: return PKEY_DISABLE_ACCESS; case PKEY_AMR_READ: return PKEY_DISABLE_READ; case PKEY_AMR_WRITE: return PKEY_DISABLE_WRITE; case 0: return 0; } By the way, is the AMR register 64-bit or 32-bit on 32-bit POWER? Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-08 20:23 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-08 20:23 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-api, linux-mm, linuxppc-dev * Ram Pai: > Florian, > > I can. But I am struggling to understand the requirement. Why is > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > to be able to allocate keys that are initialied to disable-read > only? Yes, I think that would be a natural consequence. However, my immediate need comes from the fact that the AMR register can contain a flag combination that is not possible to represent with the existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code could write to AMR directly, so I cannot rule out that certain flag combinations exist there. So I came up with this: int pkey_get (int key) { if (key < 0 || key > PKEY_MAX) { __set_errno (EINVAL); return -1; } unsigned int index = pkey_index (key); unsigned long int amr = pkey_read (); unsigned int bits = (amr >> index) & 3; /* Translate from AMR values. PKEY_AMR_READ standing alone is not currently representable. */ if (bits & PKEY_AMR_READ) return PKEY_DISABLE_ACCESS; else if (bits == PKEY_AMR_WRITE) return PKEY_DISABLE_WRITE; return 0; } And this is not ideal. I would prefer something like this instead: switch (bits) { case PKEY_AMR_READ | PKEY_AMR_WRITE: return PKEY_DISABLE_ACCESS; case PKEY_AMR_READ: return PKEY_DISABLE_READ; case PKEY_AMR_WRITE: return PKEY_DISABLE_WRITE; case 0: return 0; } By the way, is the AMR register 64-bit or 32-bit on 32-bit POWER? Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 20:23 ` Florian Weimer @ 2018-11-09 18:09 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-09 18:09 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: > * Ram Pai: > > > Florian, > > > > I can. But I am struggling to understand the requirement. Why is > > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > > to be able to allocate keys that are initialied to disable-read > > only? > > Yes, I think that would be a natural consequence. > > However, my immediate need comes from the fact that the AMR register can > contain a flag combination that is not possible to represent with the > existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code > could write to AMR directly, so I cannot rule out that certain flag > combinations exist there. > > So I came up with this: > > int > pkey_get (int key) > { > if (key < 0 || key > PKEY_MAX) > { > __set_errno (EINVAL); > return -1; > } > unsigned int index = pkey_index (key); > unsigned long int amr = pkey_read (); > unsigned int bits = (amr >> index) & 3; > > /* Translate from AMR values. PKEY_AMR_READ standing alone is not > currently representable. */ > if (bits & PKEY_AMR_READ) this should be if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) > return PKEY_DISABLE_ACCESS; > else if (bits == PKEY_AMR_WRITE) > return PKEY_DISABLE_WRITE; > return 0; > } > > And this is not ideal. I would prefer something like this instead: > > switch (bits) > { > case PKEY_AMR_READ | PKEY_AMR_WRITE: > return PKEY_DISABLE_ACCESS; > case PKEY_AMR_READ: > return PKEY_DISABLE_READ; > case PKEY_AMR_WRITE: > return PKEY_DISABLE_WRITE; > case 0: > return 0; > } yes. and on x86 it will be something like: switch (bits) { case PKEY_PKRU_ACCESS : return PKEY_DISABLE_ACCESS; case PKEY_AMR_WRITE: return PKEY_DISABLE_WRITE; case 0: return 0; } But for this to work, why do you need to enhance the sys_pkey_alloc() interface? Not that I am against it. Trying to understand if the enhancement is really needed. > > By the way, is the AMR register 64-bit or 32-bit on 32-bit POWER? It is 64-bit. RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-09 18:09 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-09 18:09 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-api, linux-mm, linuxppc-dev On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: > * Ram Pai: > > > Florian, > > > > I can. But I am struggling to understand the requirement. Why is > > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > > to be able to allocate keys that are initialied to disable-read > > only? > > Yes, I think that would be a natural consequence. > > However, my immediate need comes from the fact that the AMR register can > contain a flag combination that is not possible to represent with the > existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code > could write to AMR directly, so I cannot rule out that certain flag > combinations exist there. > > So I came up with this: > > int > pkey_get (int key) > { > if (key < 0 || key > PKEY_MAX) > { > __set_errno (EINVAL); > return -1; > } > unsigned int index = pkey_index (key); > unsigned long int amr = pkey_read (); > unsigned int bits = (amr >> index) & 3; > > /* Translate from AMR values. PKEY_AMR_READ standing alone is not > currently representable. */ > if (bits & PKEY_AMR_READ) this should be if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) > return PKEY_DISABLE_ACCESS; > else if (bits == PKEY_AMR_WRITE) > return PKEY_DISABLE_WRITE; > return 0; > } > > And this is not ideal. I would prefer something like this instead: > > switch (bits) > { > case PKEY_AMR_READ | PKEY_AMR_WRITE: > return PKEY_DISABLE_ACCESS; > case PKEY_AMR_READ: > return PKEY_DISABLE_READ; > case PKEY_AMR_WRITE: > return PKEY_DISABLE_WRITE; > case 0: > return 0; > } yes. and on x86 it will be something like: switch (bits) { case PKEY_PKRU_ACCESS : return PKEY_DISABLE_ACCESS; case PKEY_AMR_WRITE: return PKEY_DISABLE_WRITE; case 0: return 0; } But for this to work, why do you need to enhance the sys_pkey_alloc() interface? Not that I am against it. Trying to understand if the enhancement is really needed. > > By the way, is the AMR register 64-bit or 32-bit on 32-bit POWER? It is 64-bit. RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-09 18:09 ` Ram Pai @ 2018-11-12 12:00 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-12 12:00 UTC (permalink / raw) To: Ram Pai; +Cc: linux-mm, linux-api, linuxppc-dev, Dave Hansen * Ram Pai: > On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: >> * Ram Pai: >> >> > Florian, >> > >> > I can. But I am struggling to understand the requirement. Why is >> > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), >> > to be able to allocate keys that are initialied to disable-read >> > only? >> >> Yes, I think that would be a natural consequence. >> >> However, my immediate need comes from the fact that the AMR register can >> contain a flag combination that is not possible to represent with the >> existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code >> could write to AMR directly, so I cannot rule out that certain flag >> combinations exist there. >> >> So I came up with this: >> >> int >> pkey_get (int key) >> { >> if (key < 0 || key > PKEY_MAX) >> { >> __set_errno (EINVAL); >> return -1; >> } >> unsigned int index = pkey_index (key); >> unsigned long int amr = pkey_read (); >> unsigned int bits = (amr >> index) & 3; >> >> /* Translate from AMR values. PKEY_AMR_READ standing alone is not >> currently representable. */ >> if (bits & PKEY_AMR_READ) > > this should be > if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) This would return zero for PKEY_AMR_READ alone. >> return PKEY_DISABLE_ACCESS; > > >> else if (bits == PKEY_AMR_WRITE) >> return PKEY_DISABLE_WRITE; >> return 0; >> } It's hard to tell whether PKEY_DISABLE_ACCESS is better in this case. Which is why I want PKEY_DISABLE_READ. >> And this is not ideal. I would prefer something like this instead: >> >> switch (bits) >> { >> case PKEY_AMR_READ | PKEY_AMR_WRITE: >> return PKEY_DISABLE_ACCESS; >> case PKEY_AMR_READ: >> return PKEY_DISABLE_READ; >> case PKEY_AMR_WRITE: >> return PKEY_DISABLE_WRITE; >> case 0: >> return 0; >> } > > yes. > and on x86 it will be something like: > switch (bits) > { > case PKEY_PKRU_ACCESS : > return PKEY_DISABLE_ACCESS; > case PKEY_AMR_WRITE: > return PKEY_DISABLE_WRITE; > case 0: > return 0; > } x86 returns the PKRU bits directly, including the nonsensical case (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE). > But for this to work, why do you need to enhance the sys_pkey_alloc() > interface? Not that I am against it. Trying to understand if the > enhancement is really needed. sys_pkey_alloc performs an implicit pkey_set for the newly allocated key (that is, it updates the PKRU/AMR register). It makes sense to match the behavior of the userspace implementation. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-12 12:00 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-12 12:00 UTC (permalink / raw) To: Ram Pai; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api * Ram Pai: > On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: >> * Ram Pai: >> >> > Florian, >> > >> > I can. But I am struggling to understand the requirement. Why is >> > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), >> > to be able to allocate keys that are initialied to disable-read >> > only? >> >> Yes, I think that would be a natural consequence. >> >> However, my immediate need comes from the fact that the AMR register can >> contain a flag combination that is not possible to represent with the >> existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code >> could write to AMR directly, so I cannot rule out that certain flag >> combinations exist there. >> >> So I came up with this: >> >> int >> pkey_get (int key) >> { >> if (key < 0 || key > PKEY_MAX) >> { >> __set_errno (EINVAL); >> return -1; >> } >> unsigned int index = pkey_index (key); >> unsigned long int amr = pkey_read (); >> unsigned int bits = (amr >> index) & 3; >> >> /* Translate from AMR values. PKEY_AMR_READ standing alone is not >> currently representable. */ >> if (bits & PKEY_AMR_READ) > > this should be > if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) This would return zero for PKEY_AMR_READ alone. >> return PKEY_DISABLE_ACCESS; > > >> else if (bits == PKEY_AMR_WRITE) >> return PKEY_DISABLE_WRITE; >> return 0; >> } It's hard to tell whether PKEY_DISABLE_ACCESS is better in this case. Which is why I want PKEY_DISABLE_READ. >> And this is not ideal. I would prefer something like this instead: >> >> switch (bits) >> { >> case PKEY_AMR_READ | PKEY_AMR_WRITE: >> return PKEY_DISABLE_ACCESS; >> case PKEY_AMR_READ: >> return PKEY_DISABLE_READ; >> case PKEY_AMR_WRITE: >> return PKEY_DISABLE_WRITE; >> case 0: >> return 0; >> } > > yes. > and on x86 it will be something like: > switch (bits) > { > case PKEY_PKRU_ACCESS : > return PKEY_DISABLE_ACCESS; > case PKEY_AMR_WRITE: > return PKEY_DISABLE_WRITE; > case 0: > return 0; > } x86 returns the PKRU bits directly, including the nonsensical case (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE). > But for this to work, why do you need to enhance the sys_pkey_alloc() > interface? Not that I am against it. Trying to understand if the > enhancement is really needed. sys_pkey_alloc performs an implicit pkey_set for the newly allocated key (that is, it updates the PKRU/AMR register). It makes sense to match the behavior of the userspace implementation. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-12 12:00 ` Florian Weimer @ 2018-11-27 10:23 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-27 10:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-mm, linuxppc-dev, linux-api On Mon, Nov 12, 2018 at 01:00:19PM +0100, Florian Weimer wrote: > * Ram Pai: > > > On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: > >> * Ram Pai: > >> > >> > Florian, > >> > > >> > I can. But I am struggling to understand the requirement. Why is > >> > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > >> > to be able to allocate keys that are initialied to disable-read > >> > only? > >> > >> Yes, I think that would be a natural consequence. > >> > >> However, my immediate need comes from the fact that the AMR register can > >> contain a flag combination that is not possible to represent with the > >> existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code > >> could write to AMR directly, so I cannot rule out that certain flag > >> combinations exist there. > >> > >> So I came up with this: > >> > >> int > >> pkey_get (int key) > >> { > >> if (key < 0 || key > PKEY_MAX) > >> { > >> __set_errno (EINVAL); > >> return -1; > >> } > >> unsigned int index = pkey_index (key); > >> unsigned long int amr = pkey_read (); > >> unsigned int bits = (amr >> index) & 3; > >> > >> /* Translate from AMR values. PKEY_AMR_READ standing alone is not > >> currently representable. */ > >> if (bits & PKEY_AMR_READ) > > > > this should be > > if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) > > This would return zero for PKEY_AMR_READ alone. > > >> return PKEY_DISABLE_ACCESS; > > > > > >> else if (bits == PKEY_AMR_WRITE) > >> return PKEY_DISABLE_WRITE; > >> return 0; > >> } > > It's hard to tell whether PKEY_DISABLE_ACCESS is better in this case. > Which is why I want PKEY_DISABLE_READ. > > >> And this is not ideal. I would prefer something like this instead: > >> > >> switch (bits) > >> { > >> case PKEY_AMR_READ | PKEY_AMR_WRITE: > >> return PKEY_DISABLE_ACCESS; > >> case PKEY_AMR_READ: > >> return PKEY_DISABLE_READ; > >> case PKEY_AMR_WRITE: > >> return PKEY_DISABLE_WRITE; > >> case 0: > >> return 0; > >> } > > > > yes. > > and on x86 it will be something like: > > switch (bits) > > { > > case PKEY_PKRU_ACCESS : > > return PKEY_DISABLE_ACCESS; > > case PKEY_AMR_WRITE: > > return PKEY_DISABLE_WRITE; > > case 0: > > return 0; > > } > > x86 returns the PKRU bits directly, including the nonsensical case > (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE). > > > But for this to work, why do you need to enhance the sys_pkey_alloc() > > interface? Not that I am against it. Trying to understand if the > > enhancement is really needed. > > sys_pkey_alloc performs an implicit pkey_set for the newly allocated key > (that is, it updates the PKRU/AMR register). It makes sense to match > the behavior of the userspace implementation. Here is a untested patch. Does this meet your needs? It defines the new flags. Each architecture will than define the set of flags it supports through PKEY_ACCESS_MASK. Signed-off-by: Ram Pai <linuxram@us.ibm.com> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 92a9962..724ef43 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -21,11 +21,6 @@ #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) -/* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \ - PKEY_DISABLE_WRITE | \ - PKEY_DISABLE_EXECUTE) static inline u64 pkey_to_vmflag_bits(u16 pkey) { diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h index 65065ce..76237b3 100644 --- a/arch/powerpc/include/uapi/asm/mman.h +++ b/arch/powerpc/include/uapi/asm/mman.h @@ -31,9 +31,9 @@ #define MAP_HUGETLB 0x40000 /* create a huge page mapping */ /* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 #undef PKEY_ACCESS_MASK #define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_READ |\ PKEY_DISABLE_EXECUTE) #endif /* _UAPI_ASM_POWERPC_MMAN_H */ diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index 4860acd..c8b2540 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -62,14 +62,6 @@ int pkey_initialize(void) int os_reserved, i; /* - * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral - * generic defines for PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE. - * Ensure that the bits a distinct. - */ - BUILD_BUG_ON(PKEY_DISABLE_EXECUTE & - (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE)); - - /* * pkey_to_vmflag_bits() assumes that the pkey bits are contiguous * in the vmaflag. Make sure that is really the case. */ @@ -259,6 +251,8 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT; else if (init_val & PKEY_DISABLE_WRITE) new_amr_bits |= AMR_WR_BIT; + else if (init_val & PKEY_DISABLE_READ) + new_amr_bits |= AMR_RD_BIT; init_amr(pkey, new_amr_bits); return 0; diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h index d4a8d04..e9b121b 100644 --- a/arch/x86/include/uapi/asm/mman.h +++ b/arch/x86/include/uapi/asm/mman.h @@ -24,6 +24,11 @@ ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \ ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \ ((key) & 0x8 ? VM_PKEY_BIT3 : 0)) + +/* Override any generic PKEY permission defines */ +#undef PKEY_ACCESS_MASK +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ + PKEY_DISABLE_WRITE) #endif #include <asm-generic/mman.h> diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index e7ee328..61168e4 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -71,7 +71,8 @@ #define PKEY_DISABLE_ACCESS 0x1 #define PKEY_DISABLE_WRITE 0x2 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE) - +#define PKEY_DISABLE_EXECUTE 0x4 +#define PKEY_DISABLE_READ 0x8 +#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own + mask bits */ #endif /* __ASM_GENERIC_MMAN_COMMON_H */ ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-27 10:23 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-27 10:23 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-mm, linux-api, linuxppc-dev, Dave Hansen On Mon, Nov 12, 2018 at 01:00:19PM +0100, Florian Weimer wrote: > * Ram Pai: > > > On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote: > >> * Ram Pai: > >> > >> > Florian, > >> > > >> > I can. But I am struggling to understand the requirement. Why is > >> > this needed? Are we proposing a enhancement to the sys_pkey_alloc(), > >> > to be able to allocate keys that are initialied to disable-read > >> > only? > >> > >> Yes, I think that would be a natural consequence. > >> > >> However, my immediate need comes from the fact that the AMR register can > >> contain a flag combination that is not possible to represent with the > >> existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code > >> could write to AMR directly, so I cannot rule out that certain flag > >> combinations exist there. > >> > >> So I came up with this: > >> > >> int > >> pkey_get (int key) > >> { > >> if (key < 0 || key > PKEY_MAX) > >> { > >> __set_errno (EINVAL); > >> return -1; > >> } > >> unsigned int index = pkey_index (key); > >> unsigned long int amr = pkey_read (); > >> unsigned int bits = (amr >> index) & 3; > >> > >> /* Translate from AMR values. PKEY_AMR_READ standing alone is not > >> currently representable. */ > >> if (bits & PKEY_AMR_READ) > > > > this should be > > if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE)) > > This would return zero for PKEY_AMR_READ alone. > > >> return PKEY_DISABLE_ACCESS; > > > > > >> else if (bits == PKEY_AMR_WRITE) > >> return PKEY_DISABLE_WRITE; > >> return 0; > >> } > > It's hard to tell whether PKEY_DISABLE_ACCESS is better in this case. > Which is why I want PKEY_DISABLE_READ. > > >> And this is not ideal. I would prefer something like this instead: > >> > >> switch (bits) > >> { > >> case PKEY_AMR_READ | PKEY_AMR_WRITE: > >> return PKEY_DISABLE_ACCESS; > >> case PKEY_AMR_READ: > >> return PKEY_DISABLE_READ; > >> case PKEY_AMR_WRITE: > >> return PKEY_DISABLE_WRITE; > >> case 0: > >> return 0; > >> } > > > > yes. > > and on x86 it will be something like: > > switch (bits) > > { > > case PKEY_PKRU_ACCESS : > > return PKEY_DISABLE_ACCESS; > > case PKEY_AMR_WRITE: > > return PKEY_DISABLE_WRITE; > > case 0: > > return 0; > > } > > x86 returns the PKRU bits directly, including the nonsensical case > (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE). > > > But for this to work, why do you need to enhance the sys_pkey_alloc() > > interface? Not that I am against it. Trying to understand if the > > enhancement is really needed. > > sys_pkey_alloc performs an implicit pkey_set for the newly allocated key > (that is, it updates the PKRU/AMR register). It makes sense to match > the behavior of the userspace implementation. Here is a untested patch. Does this meet your needs? It defines the new flags. Each architecture will than define the set of flags it supports through PKEY_ACCESS_MASK. Signed-off-by: Ram Pai <linuxram@us.ibm.com> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 92a9962..724ef43 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -21,11 +21,6 @@ #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) -/* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \ - PKEY_DISABLE_WRITE | \ - PKEY_DISABLE_EXECUTE) static inline u64 pkey_to_vmflag_bits(u16 pkey) { diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h index 65065ce..76237b3 100644 --- a/arch/powerpc/include/uapi/asm/mman.h +++ b/arch/powerpc/include/uapi/asm/mman.h @@ -31,9 +31,9 @@ #define MAP_HUGETLB 0x40000 /* create a huge page mapping */ /* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 #undef PKEY_ACCESS_MASK #define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_READ |\ PKEY_DISABLE_EXECUTE) #endif /* _UAPI_ASM_POWERPC_MMAN_H */ diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index 4860acd..c8b2540 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -62,14 +62,6 @@ int pkey_initialize(void) int os_reserved, i; /* - * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral - * generic defines for PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE. - * Ensure that the bits a distinct. - */ - BUILD_BUG_ON(PKEY_DISABLE_EXECUTE & - (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE)); - - /* * pkey_to_vmflag_bits() assumes that the pkey bits are contiguous * in the vmaflag. Make sure that is really the case. */ @@ -259,6 +251,8 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT; else if (init_val & PKEY_DISABLE_WRITE) new_amr_bits |= AMR_WR_BIT; + else if (init_val & PKEY_DISABLE_READ) + new_amr_bits |= AMR_RD_BIT; init_amr(pkey, new_amr_bits); return 0; diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h index d4a8d04..e9b121b 100644 --- a/arch/x86/include/uapi/asm/mman.h +++ b/arch/x86/include/uapi/asm/mman.h @@ -24,6 +24,11 @@ ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \ ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \ ((key) & 0x8 ? VM_PKEY_BIT3 : 0)) + +/* Override any generic PKEY permission defines */ +#undef PKEY_ACCESS_MASK +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ + PKEY_DISABLE_WRITE) #endif #include <asm-generic/mman.h> diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index e7ee328..61168e4 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -71,7 +71,8 @@ #define PKEY_DISABLE_ACCESS 0x1 #define PKEY_DISABLE_WRITE 0x2 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE) - +#define PKEY_DISABLE_EXECUTE 0x4 +#define PKEY_DISABLE_READ 0x8 +#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own + mask bits */ #endif /* __ASM_GENERIC_MMAN_COMMON_H */ ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-27 10:23 ` Ram Pai @ 2018-11-27 11:57 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-27 11:57 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-mm, linuxppc-dev, linux-api * Ram Pai: > diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h > index d4a8d04..e9b121b 100644 > --- a/arch/x86/include/uapi/asm/mman.h > +++ b/arch/x86/include/uapi/asm/mman.h > @@ -24,6 +24,11 @@ > ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \ > ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \ > ((key) & 0x8 ? VM_PKEY_BIT3 : 0)) > + > +/* Override any generic PKEY permission defines */ > +#undef PKEY_ACCESS_MASK > +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > + PKEY_DISABLE_WRITE) > #endif I would have expected something that translates PKEY_DISABLE_WRITE | PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. (My understanding is that PKEY_DISABLE_ACCESS does not disable all access, but produces execute-only memory.) > diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h > index e7ee328..61168e4 100644 > --- a/include/uapi/asm-generic/mman-common.h > +++ b/include/uapi/asm-generic/mman-common.h > @@ -71,7 +71,8 @@ > > #define PKEY_DISABLE_ACCESS 0x1 > #define PKEY_DISABLE_WRITE 0x2 > -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > - PKEY_DISABLE_WRITE) > - > +#define PKEY_DISABLE_EXECUTE 0x4 > +#define PKEY_DISABLE_READ 0x8 > +#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own > + mask bits */ > #endif /* __ASM_GENERIC_MMAN_COMMON_H */ I think Dave requested a value for PKEY_DISABLE_READ which is further away from the existing bits. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-27 11:57 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-27 11:57 UTC (permalink / raw) To: Ram Pai; +Cc: linux-mm, linux-api, linuxppc-dev, Dave Hansen * Ram Pai: > diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h > index d4a8d04..e9b121b 100644 > --- a/arch/x86/include/uapi/asm/mman.h > +++ b/arch/x86/include/uapi/asm/mman.h > @@ -24,6 +24,11 @@ > ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \ > ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \ > ((key) & 0x8 ? VM_PKEY_BIT3 : 0)) > + > +/* Override any generic PKEY permission defines */ > +#undef PKEY_ACCESS_MASK > +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > + PKEY_DISABLE_WRITE) > #endif I would have expected something that translates PKEY_DISABLE_WRITE | PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. (My understanding is that PKEY_DISABLE_ACCESS does not disable all access, but produces execute-only memory.) > diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h > index e7ee328..61168e4 100644 > --- a/include/uapi/asm-generic/mman-common.h > +++ b/include/uapi/asm-generic/mman-common.h > @@ -71,7 +71,8 @@ > > #define PKEY_DISABLE_ACCESS 0x1 > #define PKEY_DISABLE_WRITE 0x2 > -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > - PKEY_DISABLE_WRITE) > - > +#define PKEY_DISABLE_EXECUTE 0x4 > +#define PKEY_DISABLE_READ 0x8 > +#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own > + mask bits */ > #endif /* __ASM_GENERIC_MMAN_COMMON_H */ I think Dave requested a value for PKEY_DISABLE_READ which is further away from the existing bits. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-27 11:57 ` Florian Weimer @ 2018-11-27 15:31 ` Dave Hansen -1 siblings, 0 replies; 42+ messages in thread From: Dave Hansen @ 2018-11-27 15:31 UTC (permalink / raw) To: Florian Weimer, Ram Pai; +Cc: linux-mm, linuxppc-dev, linux-api On 11/27/18 3:57 AM, Florian Weimer wrote: > I would have expected something that translates PKEY_DISABLE_WRITE | > PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > > (My understanding is that PKEY_DISABLE_ACCESS does not disable all > access, but produces execute-only memory.) Correct, it disables all data access, but not execution. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-27 15:31 ` Dave Hansen 0 siblings, 0 replies; 42+ messages in thread From: Dave Hansen @ 2018-11-27 15:31 UTC (permalink / raw) To: Florian Weimer, Ram Pai; +Cc: linux-mm, linux-api, linuxppc-dev On 11/27/18 3:57 AM, Florian Weimer wrote: > I would have expected something that translates PKEY_DISABLE_WRITE | > PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > > (My understanding is that PKEY_DISABLE_ACCESS does not disable all > access, but produces execute-only memory.) Correct, it disables all data access, but not execution. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-27 15:31 ` Dave Hansen @ 2018-11-29 11:37 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-29 11:37 UTC (permalink / raw) To: Dave Hansen; +Cc: linux-mm, Ram Pai, linuxppc-dev, linux-api * Dave Hansen: > On 11/27/18 3:57 AM, Florian Weimer wrote: >> I would have expected something that translates PKEY_DISABLE_WRITE | >> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts >> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. >> >> (My understanding is that PKEY_DISABLE_ACCESS does not disable all >> access, but produces execute-only memory.) > > Correct, it disables all data access, but not execution. So I would expect something like this (completely untested, I did not even compile this): diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 20ebf153c871..bed23f9e8336 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -199,6 +199,11 @@ static inline bool arch_pkeys_enabled(void) return !static_branch_likely(&pkey_disabled); } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return (rights & ~(unsigned long)PKEY_ACCESS_MASK) == 0; +} + extern void pkey_mm_init(struct mm_struct *mm); extern bool arch_supports_pkeys(int cap); extern unsigned int arch_usable_pkeys(void); diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h index 19b137f1b3be..e3e1d5a316e8 100644 --- a/arch/x86/include/asm/pkeys.h +++ b/arch/x86/include/asm/pkeys.h @@ -14,6 +14,17 @@ static inline bool arch_pkeys_enabled(void) return boot_cpu_has(X86_FEATURE_OSPKE); } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (rights & ~(unsigned long)PKEY_ACCESS_MASK) + return false; + if (rights & PKEY_DISABLE_READ) { + /* x86 can only disable read access along with write access. */ + return rights & (PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS); + } + return true; +} + /* * Try to dedicate one of the protection keys to be used as an * execute-only protection key. diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 87a57b7642d3..b9b78145017f 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -928,7 +928,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, return -EINVAL; /* Set the bits we need in PKRU: */ - if (init_val & PKEY_DISABLE_ACCESS) + if (init_val & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ)) + /* + * arch_pkey_access_rights_valid checked that + * PKEY_DISABLE_READ is actually representable on x86 + * (that is, it comes with PKEY_DISABLE_ACCESS or + * PKEY_DISABLE_WRITE). + */ new_pkru_bits |= PKRU_AD_BIT; if (init_val & PKEY_DISABLE_WRITE) new_pkru_bits |= PKRU_WD_BIT; diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h index 2955ba976048..2c330fabbe55 100644 --- a/include/linux/pkeys.h +++ b/include/linux/pkeys.h @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) { } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return false; +} + #endif /* ! CONFIG_ARCH_HAS_PKEYS */ #endif /* _LINUX_PKEYS_H */ diff --git a/mm/mprotect.c b/mm/mprotect.c index 6d331620b9e5..f4cefc3540df 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -597,7 +597,7 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) if (flags) return -EINVAL; /* check for unsupported init values */ - if (init_val & ~PKEY_ACCESS_MASK) + if (!arch_pkey_access_rights_valid(init_val)) return -EINVAL; down_write(¤t->mm->mmap_sem); Thanks, Florian ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-29 11:37 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-29 11:37 UTC (permalink / raw) To: Dave Hansen; +Cc: Ram Pai, linux-mm, linux-api, linuxppc-dev * Dave Hansen: > On 11/27/18 3:57 AM, Florian Weimer wrote: >> I would have expected something that translates PKEY_DISABLE_WRITE | >> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts >> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. >> >> (My understanding is that PKEY_DISABLE_ACCESS does not disable all >> access, but produces execute-only memory.) > > Correct, it disables all data access, but not execution. So I would expect something like this (completely untested, I did not even compile this): diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 20ebf153c871..bed23f9e8336 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -199,6 +199,11 @@ static inline bool arch_pkeys_enabled(void) return !static_branch_likely(&pkey_disabled); } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return (rights & ~(unsigned long)PKEY_ACCESS_MASK) == 0; +} + extern void pkey_mm_init(struct mm_struct *mm); extern bool arch_supports_pkeys(int cap); extern unsigned int arch_usable_pkeys(void); diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h index 19b137f1b3be..e3e1d5a316e8 100644 --- a/arch/x86/include/asm/pkeys.h +++ b/arch/x86/include/asm/pkeys.h @@ -14,6 +14,17 @@ static inline bool arch_pkeys_enabled(void) return boot_cpu_has(X86_FEATURE_OSPKE); } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (rights & ~(unsigned long)PKEY_ACCESS_MASK) + return false; + if (rights & PKEY_DISABLE_READ) { + /* x86 can only disable read access along with write access. */ + return rights & (PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS); + } + return true; +} + /* * Try to dedicate one of the protection keys to be used as an * execute-only protection key. diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 87a57b7642d3..b9b78145017f 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -928,7 +928,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, return -EINVAL; /* Set the bits we need in PKRU: */ - if (init_val & PKEY_DISABLE_ACCESS) + if (init_val & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ)) + /* + * arch_pkey_access_rights_valid checked that + * PKEY_DISABLE_READ is actually representable on x86 + * (that is, it comes with PKEY_DISABLE_ACCESS or + * PKEY_DISABLE_WRITE). + */ new_pkru_bits |= PKRU_AD_BIT; if (init_val & PKEY_DISABLE_WRITE) new_pkru_bits |= PKRU_WD_BIT; diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h index 2955ba976048..2c330fabbe55 100644 --- a/include/linux/pkeys.h +++ b/include/linux/pkeys.h @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) { } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return false; +} + #endif /* ! CONFIG_ARCH_HAS_PKEYS */ #endif /* _LINUX_PKEYS_H */ diff --git a/mm/mprotect.c b/mm/mprotect.c index 6d331620b9e5..f4cefc3540df 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -597,7 +597,7 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) if (flags) return -EINVAL; /* check for unsupported init values */ - if (init_val & ~PKEY_ACCESS_MASK) + if (!arch_pkey_access_rights_valid(init_val)) return -EINVAL; down_write(¤t->mm->mmap_sem); Thanks, Florian ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-29 11:37 ` Florian Weimer @ 2018-12-03 4:02 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-03 4:02 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-api, linuxppc-dev, linux-mm On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: > * Dave Hansen: > > > On 11/27/18 3:57 AM, Florian Weimer wrote: > >> I would have expected something that translates PKEY_DISABLE_WRITE | > >> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > >> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > >> > >> (My understanding is that PKEY_DISABLE_ACCESS does not disable all > >> access, but produces execute-only memory.) > > > > Correct, it disables all data access, but not execution. > > So I would expect something like this (completely untested, I did not > even compile this): Ok. I re-read through the entire email thread to understand the problem and the proposed solution. Let me summarize it below. Lets see if we are on the same plate. So the problem is as follows: Currently the kernel supports 'disable-write' and 'disable-access'. On x86, cpu supports 'disable-write' and 'disable-access'. This matches with what the kernel supports. All good. However on power, cpu supports 'disable-read' too. Since userspace can program the cpu directly, userspace has the ability to set 'disable-read' too. This can lead to inconsistency between the kernel and the userspace. We want the kernel to match userspace on all architectures. Proposed Solution: Enhance the kernel to understand 'disable-read', and facilitate architectures that understand 'disable-read' to allow it. Also explicitly define the semantics of disable-access as 'disable-read and disable-write' Did I get this right? Assuming I did, the implementation has to do the following -- On power, sys_pkey_alloc() should succeed if the init_val is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS or any combination of the three. On x86, sys_pkey_alloc() should succeed if the init_val is PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ or any combination of the three, except PKEY_DISABLE_READ specified all by itself. On all other arches, none of the flags are supported. Are we on the same plate? RP > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > index 20ebf153c871..bed23f9e8336 100644 > --- a/arch/powerpc/include/asm/pkeys.h > +++ b/arch/powerpc/include/asm/pkeys.h > @@ -199,6 +199,11 @@ static inline bool arch_pkeys_enabled(void) > return !static_branch_likely(&pkey_disabled); > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + return (rights & ~(unsigned long)PKEY_ACCESS_MASK) == 0; > +} > + > extern void pkey_mm_init(struct mm_struct *mm); > extern bool arch_supports_pkeys(int cap); > extern unsigned int arch_usable_pkeys(void); > diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h > index 19b137f1b3be..e3e1d5a316e8 100644 > --- a/arch/x86/include/asm/pkeys.h > +++ b/arch/x86/include/asm/pkeys.h > @@ -14,6 +14,17 @@ static inline bool arch_pkeys_enabled(void) > return boot_cpu_has(X86_FEATURE_OSPKE); > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + if (rights & ~(unsigned long)PKEY_ACCESS_MASK) > + return false; > + if (rights & PKEY_DISABLE_READ) { > + /* x86 can only disable read access along with write access. */ > + return rights & (PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS); > + } > + return true; > +} > + > /* > * Try to dedicate one of the protection keys to be used as an > * execute-only protection key. > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 87a57b7642d3..b9b78145017f 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -928,7 +928,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > return -EINVAL; > > /* Set the bits we need in PKRU: */ > - if (init_val & PKEY_DISABLE_ACCESS) > + if (init_val & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ)) > + /* > + * arch_pkey_access_rights_valid checked that > + * PKEY_DISABLE_READ is actually representable on x86 > + * (that is, it comes with PKEY_DISABLE_ACCESS or > + * PKEY_DISABLE_WRITE). > + */ > new_pkru_bits |= PKRU_AD_BIT; > if (init_val & PKEY_DISABLE_WRITE) > new_pkru_bits |= PKRU_WD_BIT; > diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h > index 2955ba976048..2c330fabbe55 100644 > --- a/include/linux/pkeys.h > +++ b/include/linux/pkeys.h > @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) > { > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + return false; > +} > + > #endif /* ! CONFIG_ARCH_HAS_PKEYS */ > > #endif /* _LINUX_PKEYS_H */ > diff --git a/mm/mprotect.c b/mm/mprotect.c > index 6d331620b9e5..f4cefc3540df 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -597,7 +597,7 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) > if (flags) > return -EINVAL; > /* check for unsupported init values */ > - if (init_val & ~PKEY_ACCESS_MASK) > + if (!arch_pkey_access_rights_valid(init_val)) > return -EINVAL; > > down_write(¤t->mm->mmap_sem); > > Thanks, > Florian -- Ram Pai ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-03 4:02 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-03 4:02 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-mm, linux-api, linuxppc-dev On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: > * Dave Hansen: > > > On 11/27/18 3:57 AM, Florian Weimer wrote: > >> I would have expected something that translates PKEY_DISABLE_WRITE | > >> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > >> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > >> > >> (My understanding is that PKEY_DISABLE_ACCESS does not disable all > >> access, but produces execute-only memory.) > > > > Correct, it disables all data access, but not execution. > > So I would expect something like this (completely untested, I did not > even compile this): Ok. I re-read through the entire email thread to understand the problem and the proposed solution. Let me summarize it below. Lets see if we are on the same plate. So the problem is as follows: Currently the kernel supports 'disable-write' and 'disable-access'. On x86, cpu supports 'disable-write' and 'disable-access'. This matches with what the kernel supports. All good. However on power, cpu supports 'disable-read' too. Since userspace can program the cpu directly, userspace has the ability to set 'disable-read' too. This can lead to inconsistency between the kernel and the userspace. We want the kernel to match userspace on all architectures. Proposed Solution: Enhance the kernel to understand 'disable-read', and facilitate architectures that understand 'disable-read' to allow it. Also explicitly define the semantics of disable-access as 'disable-read and disable-write' Did I get this right? Assuming I did, the implementation has to do the following -- On power, sys_pkey_alloc() should succeed if the init_val is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS or any combination of the three. On x86, sys_pkey_alloc() should succeed if the init_val is PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ or any combination of the three, except PKEY_DISABLE_READ specified all by itself. On all other arches, none of the flags are supported. Are we on the same plate? RP > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > index 20ebf153c871..bed23f9e8336 100644 > --- a/arch/powerpc/include/asm/pkeys.h > +++ b/arch/powerpc/include/asm/pkeys.h > @@ -199,6 +199,11 @@ static inline bool arch_pkeys_enabled(void) > return !static_branch_likely(&pkey_disabled); > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + return (rights & ~(unsigned long)PKEY_ACCESS_MASK) == 0; > +} > + > extern void pkey_mm_init(struct mm_struct *mm); > extern bool arch_supports_pkeys(int cap); > extern unsigned int arch_usable_pkeys(void); > diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h > index 19b137f1b3be..e3e1d5a316e8 100644 > --- a/arch/x86/include/asm/pkeys.h > +++ b/arch/x86/include/asm/pkeys.h > @@ -14,6 +14,17 @@ static inline bool arch_pkeys_enabled(void) > return boot_cpu_has(X86_FEATURE_OSPKE); > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + if (rights & ~(unsigned long)PKEY_ACCESS_MASK) > + return false; > + if (rights & PKEY_DISABLE_READ) { > + /* x86 can only disable read access along with write access. */ > + return rights & (PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS); > + } > + return true; > +} > + > /* > * Try to dedicate one of the protection keys to be used as an > * execute-only protection key. > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 87a57b7642d3..b9b78145017f 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -928,7 +928,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > return -EINVAL; > > /* Set the bits we need in PKRU: */ > - if (init_val & PKEY_DISABLE_ACCESS) > + if (init_val & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ)) > + /* > + * arch_pkey_access_rights_valid checked that > + * PKEY_DISABLE_READ is actually representable on x86 > + * (that is, it comes with PKEY_DISABLE_ACCESS or > + * PKEY_DISABLE_WRITE). > + */ > new_pkru_bits |= PKRU_AD_BIT; > if (init_val & PKEY_DISABLE_WRITE) > new_pkru_bits |= PKRU_WD_BIT; > diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h > index 2955ba976048..2c330fabbe55 100644 > --- a/include/linux/pkeys.h > +++ b/include/linux/pkeys.h > @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) > { > } > > +static inline bool arch_pkey_access_rights_valid(unsigned long rights) > +{ > + return false; > +} > + > #endif /* ! CONFIG_ARCH_HAS_PKEYS */ > > #endif /* _LINUX_PKEYS_H */ > diff --git a/mm/mprotect.c b/mm/mprotect.c > index 6d331620b9e5..f4cefc3540df 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -597,7 +597,7 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) > if (flags) > return -EINVAL; > /* check for unsupported init values */ > - if (init_val & ~PKEY_ACCESS_MASK) > + if (!arch_pkey_access_rights_valid(init_val)) > return -EINVAL; > > down_write(¤t->mm->mmap_sem); > > Thanks, > Florian -- Ram Pai ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-03 4:02 ` Ram Pai @ 2018-12-03 15:52 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-12-03 15:52 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-api, linuxppc-dev, linux-mm * Ram Pai: > So the problem is as follows: > > Currently the kernel supports 'disable-write' and 'disable-access'. > > On x86, cpu supports 'disable-write' and 'disable-access'. This > matches with what the kernel supports. All good. > > However on power, cpu supports 'disable-read' too. Since userspace can > program the cpu directly, userspace has the ability to set > 'disable-read' too. This can lead to inconsistency between the kernel > and the userspace. > > We want the kernel to match userspace on all architectures. Correct. > Proposed Solution: > > Enhance the kernel to understand 'disable-read', and facilitate architectures > that understand 'disable-read' to allow it. > > Also explicitly define the semantics of disable-access as > 'disable-read and disable-write' > > Did I get this right? Assuming I did, the implementation has to do > the following -- > > On power, sys_pkey_alloc() should succeed if the init_val > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > or any combination of the three. Agreed. > On x86, sys_pkey_alloc() should succeed if the init_val is > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > or any combination of the three, except PKEY_DISABLE_READ > specified all by itself. Again agreed. That's a clever way of phrasing it actually. > On all other arches, none of the flags are supported. > > > Are we on the same plate? I think so, thanks. Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-03 15:52 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-12-03 15:52 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-mm, linux-api, linuxppc-dev * Ram Pai: > So the problem is as follows: > > Currently the kernel supports 'disable-write' and 'disable-access'. > > On x86, cpu supports 'disable-write' and 'disable-access'. This > matches with what the kernel supports. All good. > > However on power, cpu supports 'disable-read' too. Since userspace can > program the cpu directly, userspace has the ability to set > 'disable-read' too. This can lead to inconsistency between the kernel > and the userspace. > > We want the kernel to match userspace on all architectures. Correct. > Proposed Solution: > > Enhance the kernel to understand 'disable-read', and facilitate architectures > that understand 'disable-read' to allow it. > > Also explicitly define the semantics of disable-access as > 'disable-read and disable-write' > > Did I get this right? Assuming I did, the implementation has to do > the following -- > > On power, sys_pkey_alloc() should succeed if the init_val > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > or any combination of the three. Agreed. > On x86, sys_pkey_alloc() should succeed if the init_val is > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > or any combination of the three, except PKEY_DISABLE_READ > specified all by itself. Again agreed. That's a clever way of phrasing it actually. > On all other arches, none of the flags are supported. > > > Are we on the same plate? I think so, thanks. Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-03 15:52 ` Florian Weimer @ 2018-12-04 6:23 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-04 6:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-api, linuxppc-dev, linux-mm On Mon, Dec 03, 2018 at 04:52:02PM +0100, Florian Weimer wrote: > * Ram Pai: > > > So the problem is as follows: > > > > Currently the kernel supports 'disable-write' and 'disable-access'. > > > > On x86, cpu supports 'disable-write' and 'disable-access'. This > > matches with what the kernel supports. All good. > > > > However on power, cpu supports 'disable-read' too. Since userspace can > > program the cpu directly, userspace has the ability to set > > 'disable-read' too. This can lead to inconsistency between the kernel > > and the userspace. > > > > We want the kernel to match userspace on all architectures. > > Correct. > > > Proposed Solution: > > > > Enhance the kernel to understand 'disable-read', and facilitate architectures > > that understand 'disable-read' to allow it. > > > > Also explicitly define the semantics of disable-access as > > 'disable-read and disable-write' > > > > Did I get this right? Assuming I did, the implementation has to do > > the following -- > > > > On power, sys_pkey_alloc() should succeed if the init_val > > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > > or any combination of the three. > > Agreed. > > > On x86, sys_pkey_alloc() should succeed if the init_val is > > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > > or any combination of the three, except PKEY_DISABLE_READ > > specified all by itself. > > Again agreed. That's a clever way of phrasing it actually. > > > On all other arches, none of the flags are supported. > > > > > > Are we on the same plate? > > I think so, thanks. > > Florian Ok. here is a patch, compiled but not tested. See if this meets the specifications. ----------------------------------------------------------------------------------- commit 3dc06e73f3795921265d5d1d935e428deab01616 Author: Ram Pai <linuxram@us.ibm.com> Date: Tue Dec 4 00:04:11 2018 -0500 pkeys: add support of PKEY_DISABLE_READ Kernel supports 'disable-write' and 'disable-access'. x86 cpu supports 'disable-write' and 'disable-access'. This matches with the kernel support. However POWER cpu supports 'disable-read' too. Since userspace can program the cpu directly, userspace has the ability to set 'disable-read' too. This can lead to inconsistency between the kernel and the userspace. Make kernel match userspace on all architectures. Enhance the kernel to understand 'disable-read', and facilitate architectures that understand 'disable-read' to allow it. Define the semantics of disable-access as 'disable-read and disable-write' Signed-off-by: Ram Pai <linuxram@us.ibm.com> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 20ebf15..4bd09d0 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -19,11 +19,7 @@ #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) -/* Override any generic PKEY permission defines */ #define PKEY_DISABLE_EXECUTE 0x4 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \ - PKEY_DISABLE_WRITE | \ - PKEY_DISABLE_EXECUTE) static inline u64 pkey_to_vmflag_bits(u16 pkey) { @@ -199,6 +195,16 @@ static inline bool arch_pkeys_enabled(void) return !static_branch_likely(&pkey_disabled); } +extern bool __arch_pkey_access_rights_valid(unsigned long rights); + +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (static_branch_likely(&pkey_disabled)) + return false; + + return __arch_pkey_access_rights_valid(rights); +} + extern void pkey_mm_init(struct mm_struct *mm); extern bool arch_supports_pkeys(int cap); extern unsigned int arch_usable_pkeys(void); diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h index 65065ce..e63bc37 100644 --- a/arch/powerpc/include/uapi/asm/mman.h +++ b/arch/powerpc/include/uapi/asm/mman.h @@ -30,10 +30,4 @@ #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */ #define MAP_HUGETLB 0x40000 /* create a huge page mapping */ -/* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 -#undef PKEY_ACCESS_MASK -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE |\ - PKEY_DISABLE_EXECUTE) #endif /* _UAPI_ASM_POWERPC_MMAN_H */ diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index 5d65c47..a4f288b 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -67,7 +67,7 @@ int pkey_initialize(void) * Ensure that the bits a distinct. */ BUILD_BUG_ON(PKEY_DISABLE_EXECUTE & - (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE)); + (PKEY_DISABLE_READ | PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS)); /* * pkey_to_vmflag_bits() assumes that the pkey bits are contiguous @@ -259,11 +259,20 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT; else if (init_val & PKEY_DISABLE_WRITE) new_amr_bits |= AMR_WR_BIT; + else if (init_val & PKEY_DISABLE_READ) + new_amr_bits |= AMR_RD_BIT; init_amr(pkey, new_amr_bits); return 0; } +bool __arch_pkey_access_rights_valid(unsigned long rights) +{ + unsigned long mask = PKEY_DISABLE_READ | PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_ACCESS; + return (rights & mask); +} + void thread_pkey_regs_save(struct thread_struct *thread) { if (static_branch_likely(&pkey_disabled)) diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h index 19b137f..4f36a7e 100644 --- a/arch/x86/include/asm/pkeys.h +++ b/arch/x86/include/asm/pkeys.h @@ -14,6 +14,15 @@ static inline bool arch_pkeys_enabled(void) return boot_cpu_has(X86_FEATURE_OSPKE); } +extern bool __arch_pkey_access_rights_valid(unsigned long rights); +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (!boot_cpu_has(X86_FEATURE_OSPKE)) + return false; + + return __arch_pkey_access_rights_valid(rights); +} + /* * Try to dedicate one of the protection keys to be used as an * execute-only protection key. diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c index 6e98e0a..fcfe1b2 100644 --- a/arch/x86/mm/pkeys.c +++ b/arch/x86/mm/pkeys.c @@ -72,6 +72,17 @@ int __execute_only_pkey(struct mm_struct *mm) return execute_only_pkey; } +bool __arch_pkey_access_rights_valid(unsigned long rights) +{ + unsigned long mask = PKEY_DISABLE_READ | PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_ACCESS; + if (!(rights & mask)) + return false; + + /* return failure if only PKEY_DISABLE_READ is specified */ + return ((rights & mask) != PKEY_DISABLE_READ); +} + static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma) { /* Do this check first since the vm_flags should be hot */ diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h index 2955ba97..2c330fa 100644 --- a/include/linux/pkeys.h +++ b/include/linux/pkeys.h @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) { } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return false; +} + #endif /* ! CONFIG_ARCH_HAS_PKEYS */ #endif /* _LINUX_PKEYS_H */ diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index e7ee328..d2e1a5e 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -71,7 +71,5 @@ #define PKEY_DISABLE_ACCESS 0x1 #define PKEY_DISABLE_WRITE 0x2 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE) - +#define PKEY_DISABLE_READ 0x10 #endif /* __ASM_GENERIC_MMAN_COMMON_H */ diff --git a/mm/mprotect.c b/mm/mprotect.c index 6d33162..f4cefc3 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -597,7 +597,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, if (flags) return -EINVAL; /* check for unsupported init values */ - if (init_val & ~PKEY_ACCESS_MASK) + if (!arch_pkey_access_rights_valid(init_val)) return -EINVAL; down_write(¤t->mm->mmap_sem); ----------------------------------------------------------------------------- ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-04 6:23 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-04 6:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-mm, linux-api, linuxppc-dev On Mon, Dec 03, 2018 at 04:52:02PM +0100, Florian Weimer wrote: > * Ram Pai: > > > So the problem is as follows: > > > > Currently the kernel supports 'disable-write' and 'disable-access'. > > > > On x86, cpu supports 'disable-write' and 'disable-access'. This > > matches with what the kernel supports. All good. > > > > However on power, cpu supports 'disable-read' too. Since userspace can > > program the cpu directly, userspace has the ability to set > > 'disable-read' too. This can lead to inconsistency between the kernel > > and the userspace. > > > > We want the kernel to match userspace on all architectures. > > Correct. > > > Proposed Solution: > > > > Enhance the kernel to understand 'disable-read', and facilitate architectures > > that understand 'disable-read' to allow it. > > > > Also explicitly define the semantics of disable-access as > > 'disable-read and disable-write' > > > > Did I get this right? Assuming I did, the implementation has to do > > the following -- > > > > On power, sys_pkey_alloc() should succeed if the init_val > > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > > or any combination of the three. > > Agreed. > > > On x86, sys_pkey_alloc() should succeed if the init_val is > > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > > or any combination of the three, except PKEY_DISABLE_READ > > specified all by itself. > > Again agreed. That's a clever way of phrasing it actually. > > > On all other arches, none of the flags are supported. > > > > > > Are we on the same plate? > > I think so, thanks. > > Florian Ok. here is a patch, compiled but not tested. See if this meets the specifications. ----------------------------------------------------------------------------------- commit 3dc06e73f3795921265d5d1d935e428deab01616 Author: Ram Pai <linuxram@us.ibm.com> Date: Tue Dec 4 00:04:11 2018 -0500 pkeys: add support of PKEY_DISABLE_READ Kernel supports 'disable-write' and 'disable-access'. x86 cpu supports 'disable-write' and 'disable-access'. This matches with the kernel support. However POWER cpu supports 'disable-read' too. Since userspace can program the cpu directly, userspace has the ability to set 'disable-read' too. This can lead to inconsistency between the kernel and the userspace. Make kernel match userspace on all architectures. Enhance the kernel to understand 'disable-read', and facilitate architectures that understand 'disable-read' to allow it. Define the semantics of disable-access as 'disable-read and disable-write' Signed-off-by: Ram Pai <linuxram@us.ibm.com> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 20ebf15..4bd09d0 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -19,11 +19,7 @@ #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) -/* Override any generic PKEY permission defines */ #define PKEY_DISABLE_EXECUTE 0x4 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \ - PKEY_DISABLE_WRITE | \ - PKEY_DISABLE_EXECUTE) static inline u64 pkey_to_vmflag_bits(u16 pkey) { @@ -199,6 +195,16 @@ static inline bool arch_pkeys_enabled(void) return !static_branch_likely(&pkey_disabled); } +extern bool __arch_pkey_access_rights_valid(unsigned long rights); + +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (static_branch_likely(&pkey_disabled)) + return false; + + return __arch_pkey_access_rights_valid(rights); +} + extern void pkey_mm_init(struct mm_struct *mm); extern bool arch_supports_pkeys(int cap); extern unsigned int arch_usable_pkeys(void); diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h index 65065ce..e63bc37 100644 --- a/arch/powerpc/include/uapi/asm/mman.h +++ b/arch/powerpc/include/uapi/asm/mman.h @@ -30,10 +30,4 @@ #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */ #define MAP_HUGETLB 0x40000 /* create a huge page mapping */ -/* Override any generic PKEY permission defines */ -#define PKEY_DISABLE_EXECUTE 0x4 -#undef PKEY_ACCESS_MASK -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE |\ - PKEY_DISABLE_EXECUTE) #endif /* _UAPI_ASM_POWERPC_MMAN_H */ diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index 5d65c47..a4f288b 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -67,7 +67,7 @@ int pkey_initialize(void) * Ensure that the bits a distinct. */ BUILD_BUG_ON(PKEY_DISABLE_EXECUTE & - (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE)); + (PKEY_DISABLE_READ | PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS)); /* * pkey_to_vmflag_bits() assumes that the pkey bits are contiguous @@ -259,11 +259,20 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT; else if (init_val & PKEY_DISABLE_WRITE) new_amr_bits |= AMR_WR_BIT; + else if (init_val & PKEY_DISABLE_READ) + new_amr_bits |= AMR_RD_BIT; init_amr(pkey, new_amr_bits); return 0; } +bool __arch_pkey_access_rights_valid(unsigned long rights) +{ + unsigned long mask = PKEY_DISABLE_READ | PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_ACCESS; + return (rights & mask); +} + void thread_pkey_regs_save(struct thread_struct *thread) { if (static_branch_likely(&pkey_disabled)) diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h index 19b137f..4f36a7e 100644 --- a/arch/x86/include/asm/pkeys.h +++ b/arch/x86/include/asm/pkeys.h @@ -14,6 +14,15 @@ static inline bool arch_pkeys_enabled(void) return boot_cpu_has(X86_FEATURE_OSPKE); } +extern bool __arch_pkey_access_rights_valid(unsigned long rights); +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + if (!boot_cpu_has(X86_FEATURE_OSPKE)) + return false; + + return __arch_pkey_access_rights_valid(rights); +} + /* * Try to dedicate one of the protection keys to be used as an * execute-only protection key. diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c index 6e98e0a..fcfe1b2 100644 --- a/arch/x86/mm/pkeys.c +++ b/arch/x86/mm/pkeys.c @@ -72,6 +72,17 @@ int __execute_only_pkey(struct mm_struct *mm) return execute_only_pkey; } +bool __arch_pkey_access_rights_valid(unsigned long rights) +{ + unsigned long mask = PKEY_DISABLE_READ | PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_ACCESS; + if (!(rights & mask)) + return false; + + /* return failure if only PKEY_DISABLE_READ is specified */ + return ((rights & mask) != PKEY_DISABLE_READ); +} + static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma) { /* Do this check first since the vm_flags should be hot */ diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h index 2955ba97..2c330fa 100644 --- a/include/linux/pkeys.h +++ b/include/linux/pkeys.h @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void) { } +static inline bool arch_pkey_access_rights_valid(unsigned long rights) +{ + return false; +} + #endif /* ! CONFIG_ARCH_HAS_PKEYS */ #endif /* _LINUX_PKEYS_H */ diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index e7ee328..d2e1a5e 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -71,7 +71,5 @@ #define PKEY_DISABLE_ACCESS 0x1 #define PKEY_DISABLE_WRITE 0x2 -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE) - +#define PKEY_DISABLE_READ 0x10 #endif /* __ASM_GENERIC_MMAN_COMMON_H */ diff --git a/mm/mprotect.c b/mm/mprotect.c index 6d33162..f4cefc3 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -597,7 +597,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, if (flags) return -EINVAL; /* check for unsupported init values */ - if (init_val & ~PKEY_ACCESS_MASK) + if (!arch_pkey_access_rights_valid(init_val)) return -EINVAL; down_write(¤t->mm->mmap_sem); ----------------------------------------------------------------------------- ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-04 6:23 ` Ram Pai @ 2018-12-05 13:00 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-12-05 13:00 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-api, linuxppc-dev, linux-mm * Ram Pai: > Ok. here is a patch, compiled but not tested. See if this meets the > specifications. > > ----------------------------------------------------------------------------------- > > commit 3dc06e73f3795921265d5d1d935e428deab01616 > Author: Ram Pai <linuxram@us.ibm.com> > Date: Tue Dec 4 00:04:11 2018 -0500 > > pkeys: add support of PKEY_DISABLE_READ Thanks. In the x86 code, the translation of PKEY_DISABLE_READ | PKEY_DISABLE_WRITE to PKEY_DISABLE_ACCESS appears to be missing. I believe the existing code produces PKEY_DISABLE_WRITE, which is wrong. Rest looks okay to me (again not tested). Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-05 13:00 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-12-05 13:00 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-mm, linux-api, linuxppc-dev * Ram Pai: > Ok. here is a patch, compiled but not tested. See if this meets the > specifications. > > ----------------------------------------------------------------------------------- > > commit 3dc06e73f3795921265d5d1d935e428deab01616 > Author: Ram Pai <linuxram@us.ibm.com> > Date: Tue Dec 4 00:04:11 2018 -0500 > > pkeys: add support of PKEY_DISABLE_READ Thanks. In the x86 code, the translation of PKEY_DISABLE_READ | PKEY_DISABLE_WRITE to PKEY_DISABLE_ACCESS appears to be missing. I believe the existing code produces PKEY_DISABLE_WRITE, which is wrong. Rest looks okay to me (again not tested). Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-05 13:00 ` Florian Weimer @ 2018-12-05 20:23 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-05 20:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-api, linuxppc-dev, linux-mm On Wed, Dec 05, 2018 at 02:00:59PM +0100, Florian Weimer wrote: > * Ram Pai: > > > Ok. here is a patch, compiled but not tested. See if this meets the > > specifications. > > > > ----------------------------------------------------------------------------------- > > > > commit 3dc06e73f3795921265d5d1d935e428deab01616 > > Author: Ram Pai <linuxram@us.ibm.com> > > Date: Tue Dec 4 00:04:11 2018 -0500 > > > > pkeys: add support of PKEY_DISABLE_READ > > Thanks. In the x86 code, the translation of PKEY_DISABLE_READ | > PKEY_DISABLE_WRITE to PKEY_DISABLE_ACCESS appears to be missing. I > believe the existing code produces PKEY_DISABLE_WRITE, which is wrong. ah. yes. good point. > > Rest looks okay to me (again not tested). thanks, RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-05 20:23 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-05 20:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Dave Hansen, linux-mm, linux-api, linuxppc-dev On Wed, Dec 05, 2018 at 02:00:59PM +0100, Florian Weimer wrote: > * Ram Pai: > > > Ok. here is a patch, compiled but not tested. See if this meets the > > specifications. > > > > ----------------------------------------------------------------------------------- > > > > commit 3dc06e73f3795921265d5d1d935e428deab01616 > > Author: Ram Pai <linuxram@us.ibm.com> > > Date: Tue Dec 4 00:04:11 2018 -0500 > > > > pkeys: add support of PKEY_DISABLE_READ > > Thanks. In the x86 code, the translation of PKEY_DISABLE_READ | > PKEY_DISABLE_WRITE to PKEY_DISABLE_ACCESS appears to be missing. I > believe the existing code produces PKEY_DISABLE_WRITE, which is wrong. ah. yes. good point. > > Rest looks okay to me (again not tested). thanks, RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-03 4:02 ` Ram Pai @ 2018-12-05 16:21 ` Andy Lutomirski -1 siblings, 0 replies; 42+ messages in thread From: Andy Lutomirski @ 2018-12-05 16:21 UTC (permalink / raw) To: linuxram; +Cc: Florian Weimer, Dave Hansen, Linux API, linuxppc-dev, Linux-MM > On Dec 2, 2018, at 8:02 PM, Ram Pai <linuxram@us.ibm.com> wrote: > >> On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: >> * Dave Hansen: >> >>>> On 11/27/18 3:57 AM, Florian Weimer wrote: >>>> I would have expected something that translates PKEY_DISABLE_WRITE | >>>> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts >>>> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. >>>> >>>> (My understanding is that PKEY_DISABLE_ACCESS does not disable all >>>> access, but produces execute-only memory.) >>> >>> Correct, it disables all data access, but not execution. >> >> So I would expect something like this (completely untested, I did not >> even compile this): > > > Ok. I re-read through the entire email thread to understand the problem and > the proposed solution. Let me summarize it below. Lets see if we are on the same > plate. > > So the problem is as follows: > > Currently the kernel supports 'disable-write' and 'disable-access'. > > On x86, cpu supports 'disable-write' and 'disable-access'. This > matches with what the kernel supports. All good. > > However on power, cpu supports 'disable-read' too. Since userspace can > program the cpu directly, userspace has the ability to set > 'disable-read' too. This can lead to inconsistency between the kernel > and the userspace. > > We want the kernel to match userspace on all architectures. > > Proposed Solution: > > Enhance the kernel to understand 'disable-read', and facilitate architectures > that understand 'disable-read' to allow it. > > Also explicitly define the semantics of disable-access as > 'disable-read and disable-write' > > Did I get this right? Assuming I did, the implementation has to do > the following -- > > On power, sys_pkey_alloc() should succeed if the init_val > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > or any combination of the three. > > On x86, sys_pkey_alloc() should succeed if the init_val is > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > or any combination of the three, except PKEY_DISABLE_READ > specified all by itself. > > On all other arches, none of the flags are supported. I don’t really love having a situation where you can use different flag combinations to refer to the same mode. Also, we should document the effect these flags have on execute permission. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-05 16:21 ` Andy Lutomirski 0 siblings, 0 replies; 42+ messages in thread From: Andy Lutomirski @ 2018-12-05 16:21 UTC (permalink / raw) To: linuxram; +Cc: Florian Weimer, Dave Hansen, Linux-MM, Linux API, linuxppc-dev > On Dec 2, 2018, at 8:02 PM, Ram Pai <linuxram@us.ibm.com> wrote: > >> On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: >> * Dave Hansen: >> >>>> On 11/27/18 3:57 AM, Florian Weimer wrote: >>>> I would have expected something that translates PKEY_DISABLE_WRITE | >>>> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts >>>> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. >>>> >>>> (My understanding is that PKEY_DISABLE_ACCESS does not disable all >>>> access, but produces execute-only memory.) >>> >>> Correct, it disables all data access, but not execution. >> >> So I would expect something like this (completely untested, I did not >> even compile this): > > > Ok. I re-read through the entire email thread to understand the problem and > the proposed solution. Let me summarize it below. Lets see if we are on the same > plate. > > So the problem is as follows: > > Currently the kernel supports 'disable-write' and 'disable-access'. > > On x86, cpu supports 'disable-write' and 'disable-access'. This > matches with what the kernel supports. All good. > > However on power, cpu supports 'disable-read' too. Since userspace can > program the cpu directly, userspace has the ability to set > 'disable-read' too. This can lead to inconsistency between the kernel > and the userspace. > > We want the kernel to match userspace on all architectures. > > Proposed Solution: > > Enhance the kernel to understand 'disable-read', and facilitate architectures > that understand 'disable-read' to allow it. > > Also explicitly define the semantics of disable-access as > 'disable-read and disable-write' > > Did I get this right? Assuming I did, the implementation has to do > the following -- > > On power, sys_pkey_alloc() should succeed if the init_val > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > or any combination of the three. > > On x86, sys_pkey_alloc() should succeed if the init_val is > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > or any combination of the three, except PKEY_DISABLE_READ > specified all by itself. > > On all other arches, none of the flags are supported. I don’t really love having a situation where you can use different flag combinations to refer to the same mode. Also, we should document the effect these flags have on execute permission. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-12-05 16:21 ` Andy Lutomirski @ 2018-12-05 20:36 ` Ram Pai -1 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-05 20:36 UTC (permalink / raw) To: Andy Lutomirski Cc: Florian Weimer, Dave Hansen, Linux API, linuxppc-dev, Linux-MM On Wed, Dec 05, 2018 at 08:21:02AM -0800, Andy Lutomirski wrote: > > On Dec 2, 2018, at 8:02 PM, Ram Pai <linuxram@us.ibm.com> wrote: > > > >> On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: > >> * Dave Hansen: > >> > >>>> On 11/27/18 3:57 AM, Florian Weimer wrote: > >>>> I would have expected something that translates PKEY_DISABLE_WRITE | > >>>> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > >>>> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > >>>> > >>>> (My understanding is that PKEY_DISABLE_ACCESS does not disable all > >>>> access, but produces execute-only memory.) > >>> > >>> Correct, it disables all data access, but not execution. > >> > >> So I would expect something like this (completely untested, I did not > >> even compile this): > > > > > > Ok. I re-read through the entire email thread to understand the problem and > > the proposed solution. Let me summarize it below. Lets see if we are on the same > > plate. > > > > So the problem is as follows: > > > > Currently the kernel supports 'disable-write' and 'disable-access'. > > > > On x86, cpu supports 'disable-write' and 'disable-access'. This > > matches with what the kernel supports. All good. > > > > However on power, cpu supports 'disable-read' too. Since userspace can > > program the cpu directly, userspace has the ability to set > > 'disable-read' too. This can lead to inconsistency between the kernel > > and the userspace. > > > > We want the kernel to match userspace on all architectures. > > > > Proposed Solution: > > > > Enhance the kernel to understand 'disable-read', and facilitate architectures > > that understand 'disable-read' to allow it. > > > > Also explicitly define the semantics of disable-access as > > 'disable-read and disable-write' > > > > Did I get this right? Assuming I did, the implementation has to do > > the following -- > > > > On power, sys_pkey_alloc() should succeed if the init_val > > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > > or any combination of the three. > > > > On x86, sys_pkey_alloc() should succeed if the init_val is > > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > > or any combination of the three, except PKEY_DISABLE_READ > > specified all by itself. > > > > On all other arches, none of the flags are supported. > > I don’t really love having a situation where you can use different > flag combinations to refer to the same mode. true. But it is a side-effect of x86 cpu implicitly defining 'disable-access' as a combination of 'disable-read' and 'disable_write'. In other words, if you disable-access on a pte on x86, you are automatically disabling read and disabling write on that page. The software/kernel just happens to explicitly capture that implicit behavior. > > Also, we should document the effect these flags have on execute permission. Actually none of the above flags, interact with execute permission. They operate independently; both on x86 and on POWER. But yes, this statement needs to be documented somewhere. RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-12-05 20:36 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-12-05 20:36 UTC (permalink / raw) To: Andy Lutomirski Cc: Florian Weimer, Dave Hansen, Linux-MM, Linux API, linuxppc-dev On Wed, Dec 05, 2018 at 08:21:02AM -0800, Andy Lutomirski wrote: > > On Dec 2, 2018, at 8:02 PM, Ram Pai <linuxram@us.ibm.com> wrote: > > > >> On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: > >> * Dave Hansen: > >> > >>>> On 11/27/18 3:57 AM, Florian Weimer wrote: > >>>> I would have expected something that translates PKEY_DISABLE_WRITE | > >>>> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > >>>> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > >>>> > >>>> (My understanding is that PKEY_DISABLE_ACCESS does not disable all > >>>> access, but produces execute-only memory.) > >>> > >>> Correct, it disables all data access, but not execution. > >> > >> So I would expect something like this (completely untested, I did not > >> even compile this): > > > > > > Ok. I re-read through the entire email thread to understand the problem and > > the proposed solution. Let me summarize it below. Lets see if we are on the same > > plate. > > > > So the problem is as follows: > > > > Currently the kernel supports 'disable-write' and 'disable-access'. > > > > On x86, cpu supports 'disable-write' and 'disable-access'. This > > matches with what the kernel supports. All good. > > > > However on power, cpu supports 'disable-read' too. Since userspace can > > program the cpu directly, userspace has the ability to set > > 'disable-read' too. This can lead to inconsistency between the kernel > > and the userspace. > > > > We want the kernel to match userspace on all architectures. > > > > Proposed Solution: > > > > Enhance the kernel to understand 'disable-read', and facilitate architectures > > that understand 'disable-read' to allow it. > > > > Also explicitly define the semantics of disable-access as > > 'disable-read and disable-write' > > > > Did I get this right? Assuming I did, the implementation has to do > > the following -- > > > > On power, sys_pkey_alloc() should succeed if the init_val > > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > > or any combination of the three. > > > > On x86, sys_pkey_alloc() should succeed if the init_val is > > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > > or any combination of the three, except PKEY_DISABLE_READ > > specified all by itself. > > > > On all other arches, none of the flags are supported. > > I don’t really love having a situation where you can use different > flag combinations to refer to the same mode. true. But it is a side-effect of x86 cpu implicitly defining 'disable-access' as a combination of 'disable-read' and 'disable_write'. In other words, if you disable-access on a pte on x86, you are automatically disabling read and disabling write on that page. The software/kernel just happens to explicitly capture that implicit behavior. > > Also, we should document the effect these flags have on execute permission. Actually none of the above flags, interact with execute permission. They operate independently; both on x86 and on POWER. But yes, this statement needs to be documented somewhere. RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 17:14 ` Dave Hansen 2018-11-08 17:37 ` Florian Weimer @ 2018-11-08 20:08 ` Ram Pai 2018-11-08 20:11 ` Dave Hansen 2018-11-08 20:14 ` Florian Weimer 1 sibling, 2 replies; 42+ messages in thread From: Ram Pai @ 2018-11-08 20:08 UTC (permalink / raw) To: Dave Hansen; +Cc: Florian Weimer, linux-api, linux-mm On Thu, Nov 08, 2018 at 09:14:54AM -0800, Dave Hansen wrote: > On 11/8/18 7:01 AM, Florian Weimer wrote: > > Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ | > > PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a > > line PKEY_DISABLE_READ would result in an EINVAL failure. > > Sounds reasonable to me. > > I don't see any urgency to do this right now. It could easily go in > alongside the ppc patches when those get merged. The only thing I'd > suggest is that we make it something slightly higher than 0x4. It'll > make the code easier to deal with in the kernel if we have the ABI and > the hardware mirror each other, and if we pick 0x4 in the ABI for > PKEY_DISABLE_READ, it might get messy if the harware choose 0x4 for > PKEY_DISABLE_EXECUTE or something. The hardware bits have to be decoupled from the software bits. Otherwise we will get too constrainted and will conflict with the bit configuration of some hardware. Powerpc implementation can deal with 0x4 or any other value. > > So, let's make it 0x80 or something on x86 at least. > > Also, I'll be happy to review and ack the patch to do this, but I'd > expect the ppc guys (hi Ram!) to actually put it together. Hi Dave! :) So what is needed? Support a new flag PKEY_DISABLE_READ, and make it return error for all architectures? Or are we enhancing the symantics of pkey_alloc() to allocate keys with just disable-read permissions.? And if so, will x86 be able to support that semantics? -- Ram Pai ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 20:08 ` Ram Pai @ 2018-11-08 20:11 ` Dave Hansen 2018-11-08 20:14 ` Florian Weimer 1 sibling, 0 replies; 42+ messages in thread From: Dave Hansen @ 2018-11-08 20:11 UTC (permalink / raw) To: Ram Pai; +Cc: Florian Weimer, linux-api, linux-mm On 11/8/18 12:08 PM, Ram Pai wrote: >> Also, I'll be happy to review and ack the patch to do this, but I'd >> expect the ppc guys (hi Ram!) to actually put it together. > Hi Dave! :) So what is needed? Support a new flag PKEY_DISABLE_READ, and make it > return error for all architectures? Florian proposed some semantics further up in the thread. Basically if someone asks for PKEY_DISABLE_READ, allow it to be accepted as long as its compatible with the other flags. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 20:08 ` Ram Pai 2018-11-08 20:11 ` Dave Hansen @ 2018-11-08 20:14 ` Florian Weimer 1 sibling, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-08 20:14 UTC (permalink / raw) To: Ram Pai; +Cc: Dave Hansen, linux-api, linux-mm * Ram Pai: > Hi Dave! :) So what is needed? Support a new flag PKEY_DISABLE_READ, > and make it return error for all architectures? PKEY_DISABLE_READ | PKEY_DISABLE_WRITE should be equivalent to PKEY_DISABLE_ACCESS. PKEY_DISABLE_READ without any other flag on x86 should return EINVAL (as for other invalid access rights specified for pkey_alloc). > Or are we enhancing the symantics of pkey_alloc() to allocate keys with > just disable-read permissions.? And if so, will x86 be able to support > that semantics? I think x86 cannot do this, but POWER can, but it's currently not possible to express this via pkey_alloc. That could be fixed, too. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 12:05 pkeys: Reserve PKEY_DISABLE_READ Florian Weimer @ 2018-11-08 19:22 ` Ram Pai 2018-11-08 19:22 ` Ram Pai 1 sibling, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-08 19:22 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-api, dave.hansen, linuxppc-dev, linux-mm On Thu, Nov 08, 2018 at 01:05:09PM +0100, Florian Weimer wrote: > Would it be possible to reserve a bit for PKEY_DISABLE_READ? > > I think the POWER implementation can disable read access at the hardware > level, but not write access, and that cannot be expressed with the > current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. POWER hardware can disable-read and can **also disable-write** at the hardware level. It can disable-execute aswell at the hardware level. For example if the key bits for a given key in the AMR register is 0b01 it is read-disable 0b10 it is write-disable To support access-disable, we make the key value 0b11. So in case if you want to know if the key is read-disable 'bitwise-and' it against 0x1. i.e (x & 0x1) RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-08 19:22 ` Ram Pai 0 siblings, 0 replies; 42+ messages in thread From: Ram Pai @ 2018-11-08 19:22 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-api, linux-mm, dave.hansen, linuxppc-dev On Thu, Nov 08, 2018 at 01:05:09PM +0100, Florian Weimer wrote: > Would it be possible to reserve a bit for PKEY_DISABLE_READ? > > I think the POWER implementation can disable read access at the hardware > level, but not write access, and that cannot be expressed with the > current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. POWER hardware can disable-read and can **also disable-write** at the hardware level. It can disable-execute aswell at the hardware level. For example if the key bits for a given key in the AMR register is 0b01 it is read-disable 0b10 it is write-disable To support access-disable, we make the key value 0b11. So in case if you want to know if the key is read-disable 'bitwise-and' it against 0x1. i.e (x & 0x1) RP ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ 2018-11-08 19:22 ` Ram Pai @ 2018-11-12 10:29 ` Florian Weimer -1 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-12 10:29 UTC (permalink / raw) To: Ram Pai; +Cc: linux-api, dave.hansen, linuxppc-dev, linux-mm * Ram Pai: > On Thu, Nov 08, 2018 at 01:05:09PM +0100, Florian Weimer wrote: >> Would it be possible to reserve a bit for PKEY_DISABLE_READ? >> >> I think the POWER implementation can disable read access at the hardware >> level, but not write access, and that cannot be expressed with the >> current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. > > POWER hardware can disable-read and can **also disable-write** > at the hardware level. It can disable-execute aswell at the > hardware level. For example if the key bits for a given key in the AMR > register is > 0b01 it is read-disable > 0b10 it is write-disable > > To support access-disable, we make the key value 0b11. > > So in case if you want to know if the key is read-disable 'bitwise-and' it > against 0x1. i.e (x & 0x1) Not sure if we covered that alreay, but my problem is that I cannot translate a 0b01 mask to a PKEY_DISABLE_* flag combination with the current flags. 0b10 and 0b11 are fine. POWER also loses the distinction between PKEY_DISABLE_ACCESS and PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE, but that's fine. This breaks the current glibc test case, but I have a patch for that. Arguably, the test is wrong or at least overly strict in what it accepts. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: pkeys: Reserve PKEY_DISABLE_READ @ 2018-11-12 10:29 ` Florian Weimer 0 siblings, 0 replies; 42+ messages in thread From: Florian Weimer @ 2018-11-12 10:29 UTC (permalink / raw) To: Ram Pai; +Cc: linux-api, linux-mm, dave.hansen, linuxppc-dev * Ram Pai: > On Thu, Nov 08, 2018 at 01:05:09PM +0100, Florian Weimer wrote: >> Would it be possible to reserve a bit for PKEY_DISABLE_READ? >> >> I think the POWER implementation can disable read access at the hardware >> level, but not write access, and that cannot be expressed with the >> current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits. > > POWER hardware can disable-read and can **also disable-write** > at the hardware level. It can disable-execute aswell at the > hardware level. For example if the key bits for a given key in the AMR > register is > 0b01 it is read-disable > 0b10 it is write-disable > > To support access-disable, we make the key value 0b11. > > So in case if you want to know if the key is read-disable 'bitwise-and' it > against 0x1. i.e (x & 0x1) Not sure if we covered that alreay, but my problem is that I cannot translate a 0b01 mask to a PKEY_DISABLE_* flag combination with the current flags. 0b10 and 0b11 are fine. POWER also loses the distinction between PKEY_DISABLE_ACCESS and PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE, but that's fine. This breaks the current glibc test case, but I have a patch for that. Arguably, the test is wrong or at least overly strict in what it accepts. Thanks, Florian ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2018-12-05 20:36 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-08 12:05 pkeys: Reserve PKEY_DISABLE_READ Florian Weimer 2018-11-08 14:57 ` Dave Hansen 2018-11-08 15:01 ` Florian Weimer 2018-11-08 17:14 ` Dave Hansen 2018-11-08 17:37 ` Florian Weimer 2018-11-08 20:12 ` Ram Pai 2018-11-08 20:12 ` Ram Pai 2018-11-08 20:23 ` Florian Weimer 2018-11-08 20:23 ` Florian Weimer 2018-11-09 18:09 ` Ram Pai 2018-11-09 18:09 ` Ram Pai 2018-11-12 12:00 ` Florian Weimer 2018-11-12 12:00 ` Florian Weimer 2018-11-27 10:23 ` Ram Pai 2018-11-27 10:23 ` Ram Pai 2018-11-27 11:57 ` Florian Weimer 2018-11-27 11:57 ` Florian Weimer 2018-11-27 15:31 ` Dave Hansen 2018-11-27 15:31 ` Dave Hansen 2018-11-29 11:37 ` Florian Weimer 2018-11-29 11:37 ` Florian Weimer 2018-12-03 4:02 ` Ram Pai 2018-12-03 4:02 ` Ram Pai 2018-12-03 15:52 ` Florian Weimer 2018-12-03 15:52 ` Florian Weimer 2018-12-04 6:23 ` Ram Pai 2018-12-04 6:23 ` Ram Pai 2018-12-05 13:00 ` Florian Weimer 2018-12-05 13:00 ` Florian Weimer 2018-12-05 20:23 ` Ram Pai 2018-12-05 20:23 ` Ram Pai 2018-12-05 16:21 ` Andy Lutomirski 2018-12-05 16:21 ` Andy Lutomirski 2018-12-05 20:36 ` Ram Pai 2018-12-05 20:36 ` Ram Pai 2018-11-08 20:08 ` Ram Pai 2018-11-08 20:11 ` Dave Hansen 2018-11-08 20:14 ` Florian Weimer 2018-11-08 19:22 ` Ram Pai 2018-11-08 19:22 ` Ram Pai 2018-11-12 10:29 ` Florian Weimer 2018-11-12 10:29 ` Florian Weimer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.