| Message ID | 20180603201832.GA10109@ram.oc3035372033.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/03/2018 10:18 PM, Ram Pai wrote: > On Mon, May 21, 2018 at 01:29:11PM +0200, Florian Weimer wrote: >> On 05/20/2018 09:11 PM, Ram Pai wrote: >>> Florian, >>> >>> Does the following patch fix the problem for you? Just like x86 >>> I am enabling all keys in the UAMOR register during >>> initialization itself. Hence any key created by any thread at >>> any time, will get activated on all threads. So any thread >>> can change the permission on that key. Smoke tested it >>> with your test program. >> >> I think this goes in the right direction, but the AMR value after >> fork is still strange: >> >> AMR (PID 34912): 0x0000000000000000 >> AMR after fork (PID 34913): 0x0000000000000000 >> AMR (PID 34913): 0x0000000000000000 >> Allocated key in subprocess (PID 34913): 2 >> Allocated key (PID 34912): 2 >> Setting AMR: 0xffffffffffffffff >> New AMR value (PID 34912): 0x0fffffffffffffff >> About to call execl (PID 34912) ... >> AMR (PID 34912): 0x0fffffffffffffff >> AMR after fork (PID 34914): 0x0000000000000003 >> AMR (PID 34914): 0x0000000000000003 >> Allocated key in subprocess (PID 34914): 2 >> Allocated key (PID 34912): 2 >> Setting AMR: 0xffffffffffffffff >> New AMR value (PID 34912): 0x0fffffffffffffff >> >> I mean this line: >> >> AMR after fork (PID 34914): 0x0000000000000003 >> >> Shouldn't it be the same as in the parent process? > > Fixed it. Please try this patch. If it all works to your satisfaction, I > will clean it up further and send to Michael Ellermen(ppc maintainer). > > > commit 51f4208ed5baeab1edb9b0f8b68d7144449b3527 > Author: Ram Pai <linuxram@us.ibm.com> > Date: Sun Jun 3 14:44:32 2018 -0500 > > Fix for the fork bug. > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> Is this on top of the previous patch, or a separate fix? Thanks, Florian
On Mon, Jun 04, 2018 at 12:12:07PM +0200, Florian Weimer wrote: > On 06/03/2018 10:18 PM, Ram Pai wrote: > >On Mon, May 21, 2018 at 01:29:11PM +0200, Florian Weimer wrote: > >>On 05/20/2018 09:11 PM, Ram Pai wrote: > >>>Florian, > >>> > >>> Does the following patch fix the problem for you? Just like x86 > >>> I am enabling all keys in the UAMOR register during > >>> initialization itself. Hence any key created by any thread at > >>> any time, will get activated on all threads. So any thread > >>> can change the permission on that key. Smoke tested it > >>> with your test program. > >> > >>I think this goes in the right direction, but the AMR value after > >>fork is still strange: > >> > >>AMR (PID 34912): 0x0000000000000000 > >>AMR after fork (PID 34913): 0x0000000000000000 > >>AMR (PID 34913): 0x0000000000000000 > >>Allocated key in subprocess (PID 34913): 2 > >>Allocated key (PID 34912): 2 > >>Setting AMR: 0xffffffffffffffff > >>New AMR value (PID 34912): 0x0fffffffffffffff > >>About to call execl (PID 34912) ... > >>AMR (PID 34912): 0x0fffffffffffffff > >>AMR after fork (PID 34914): 0x0000000000000003 > >>AMR (PID 34914): 0x0000000000000003 > >>Allocated key in subprocess (PID 34914): 2 > >>Allocated key (PID 34912): 2 > >>Setting AMR: 0xffffffffffffffff > >>New AMR value (PID 34912): 0x0fffffffffffffff > >> > >>I mean this line: > >> > >>AMR after fork (PID 34914): 0x0000000000000003 > >> > >>Shouldn't it be the same as in the parent process? > > > >Fixed it. Please try this patch. If it all works to your satisfaction, I > >will clean it up further and send to Michael Ellermen(ppc maintainer). > > > > > >commit 51f4208ed5baeab1edb9b0f8b68d7144449b3527 > >Author: Ram Pai <linuxram@us.ibm.com> > >Date: Sun Jun 3 14:44:32 2018 -0500 > > > > Fix for the fork bug. > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > > Is this on top of the previous patch, or a separate fix? top of previous patch. RP
On 06/04/2018 04:01 PM, Ram Pai wrote: > On Mon, Jun 04, 2018 at 12:12:07PM +0200, Florian Weimer wrote: >> On 06/03/2018 10:18 PM, Ram Pai wrote: >>> On Mon, May 21, 2018 at 01:29:11PM +0200, Florian Weimer wrote: >>>> On 05/20/2018 09:11 PM, Ram Pai wrote: >>>>> Florian, >>>>> >>>>> Does the following patch fix the problem for you? Just like x86 >>>>> I am enabling all keys in the UAMOR register during >>>>> initialization itself. Hence any key created by any thread at >>>>> any time, will get activated on all threads. So any thread >>>>> can change the permission on that key. Smoke tested it >>>>> with your test program. >>>> >>>> I think this goes in the right direction, but the AMR value after >>>> fork is still strange: >>>> >>>> AMR (PID 34912): 0x0000000000000000 >>>> AMR after fork (PID 34913): 0x0000000000000000 >>>> AMR (PID 34913): 0x0000000000000000 >>>> Allocated key in subprocess (PID 34913): 2 >>>> Allocated key (PID 34912): 2 >>>> Setting AMR: 0xffffffffffffffff >>>> New AMR value (PID 34912): 0x0fffffffffffffff >>>> About to call execl (PID 34912) ... >>>> AMR (PID 34912): 0x0fffffffffffffff >>>> AMR after fork (PID 34914): 0x0000000000000003 >>>> AMR (PID 34914): 0x0000000000000003 >>>> Allocated key in subprocess (PID 34914): 2 >>>> Allocated key (PID 34912): 2 >>>> Setting AMR: 0xffffffffffffffff >>>> New AMR value (PID 34912): 0x0fffffffffffffff >>>> >>>> I mean this line: >>>> >>>> AMR after fork (PID 34914): 0x0000000000000003 >>>> >>>> Shouldn't it be the same as in the parent process? >>> >>> Fixed it. Please try this patch. If it all works to your satisfaction, I >>> will clean it up further and send to Michael Ellermen(ppc maintainer). >>> >>> >>> commit 51f4208ed5baeab1edb9b0f8b68d7144449b3527 >>> Author: Ram Pai <linuxram@us.ibm.com> >>> Date: Sun Jun 3 14:44:32 2018 -0500 >>> >>> Fix for the fork bug. >>> Signed-off-by: Ram Pai <linuxram@us.ibm.com> >> >> Is this on top of the previous patch, or a separate fix? > > top of previous patch. Thanks. With this patch, I get this on an LPAR: AMR (PID 1876): 0x0000000000000003 AMR after fork (PID 1877): 0x0000000000000003 AMR (PID 1877): 0x0000000000000003 Allocated key in subprocess (PID 1877): 2 Allocated key (PID 1876): 2 Setting AMR: 0xffffffffffffffff New AMR value (PID 1876): 0x0fffffffffffffff About to call execl (PID 1876) ... AMR (PID 1876): 0x0000000000000003 AMR after fork (PID 1878): 0x0000000000000003 AMR (PID 1878): 0x0000000000000003 Allocated key in subprocess (PID 1878): 2 Allocated key (PID 1876): 2 Setting AMR: 0xffffffffffffffff New AMR value (PID 1876): 0x0fffffffffffffff Test program is still this one: <https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-May/173198.html> So the process starts out with a different AMR value for some reason. That could be a pre-existing bug that was just hidden by the reset-to-zero on fork, or it could be intentional. But the kernel code does not indicate that key 63 is reserved (POWER numbers keys from the MSB to the LSB). But it looks like we are finally getting somewhere. 8-) Thanks, Florian
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 1237f13..999dd08 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -582,6 +582,7 @@ static void save_all(struct task_struct *tsk) __giveup_spe(tsk); msr_check_and_clear(msr_all_available); + thread_pkey_regs_save(&tsk->thread); } void flush_all_to_thread(struct task_struct *tsk) diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index ab4519a..af6aa4a 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -294,6 +294,7 @@ void thread_pkey_regs_save(struct thread_struct *thread) */ thread->amr = read_amr(); thread->iamr = read_iamr(); + thread->uamor = read_uamor(); } void thread_pkey_regs_restore(struct thread_struct *new_thread, @@ -315,9 +316,13 @@ void thread_pkey_regs_init(struct thread_struct *thread) if (static_branch_likely(&pkey_disabled)) return; - thread->amr = read_amr() & pkey_amr_mask; - thread->iamr = read_iamr() & pkey_iamr_mask; + thread->amr = pkey_amr_mask; + thread->iamr = pkey_iamr_mask; thread->uamor = pkey_uamor_mask; + + write_uamor(pkey_uamor_mask); + write_amr(pkey_amr_mask); + write_iamr(pkey_iamr_mask); } static inline bool pkey_allows_readwrite(int pkey)