* [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations @ 2023-01-25 16:43 Jens Axboe 2023-01-26 14:00 ` Will Deacon 0 siblings, 1 reply; 6+ messages in thread From: Jens Axboe @ 2023-01-25 16:43 UTC (permalink / raw) To: Catalin Marinas, Will Deacon; +Cc: linux-arm-kernel Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist entirely within the kernel, and hence don't need any task mitigations applied. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c index fca9cc6f5581..25a21c3d446c 100644 --- a/arch/arm64/kernel/proton-pack.c +++ b/arch/arm64/kernel/proton-pack.c @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) void spectre_v4_enable_task_mitigation(struct task_struct *tsk) { struct pt_regs *regs = task_pt_regs(tsk); - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); if (spectre_v4_mitigations_off()) ssbs = true; -- Jens Axboe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations 2023-01-25 16:43 [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations Jens Axboe @ 2023-01-26 14:00 ` Will Deacon 2023-01-26 14:07 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Will Deacon @ 2023-01-26 14:00 UTC (permalink / raw) To: Jens Axboe; +Cc: Catalin Marinas, linux-arm-kernel On Wed, Jan 25, 2023 at 09:43:34AM -0700, Jens Axboe wrote: > Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist > entirely within the kernel, and hence don't need any task mitigations > applied. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > > --- > > diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c > index fca9cc6f5581..25a21c3d446c 100644 > --- a/arch/arm64/kernel/proton-pack.c > +++ b/arch/arm64/kernel/proton-pack.c > @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) > void spectre_v4_enable_task_mitigation(struct task_struct *tsk) > { > struct pt_regs *regs = task_pt_regs(tsk); > - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; > + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); Hmm, the other two uses of PF_KTHREAD in arch/arm64 also look pretty suspect in light of this proposal. Should we also update ssbs_thread_switch() and access_ok()? If not, then a comment would be handy to say why PF_KTHREAD is sufficient there. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations 2023-01-26 14:00 ` Will Deacon @ 2023-01-26 14:07 ` Jens Axboe 2023-01-26 15:04 ` Catalin Marinas 2023-03-28 14:14 ` Will Deacon 0 siblings, 2 replies; 6+ messages in thread From: Jens Axboe @ 2023-01-26 14:07 UTC (permalink / raw) To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel On 1/26/23 7:00 AM, Will Deacon wrote: > On Wed, Jan 25, 2023 at 09:43:34AM -0700, Jens Axboe wrote: >> Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist >> entirely within the kernel, and hence don't need any task mitigations >> applied. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> >> --- >> >> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c >> index fca9cc6f5581..25a21c3d446c 100644 >> --- a/arch/arm64/kernel/proton-pack.c >> +++ b/arch/arm64/kernel/proton-pack.c >> @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) >> void spectre_v4_enable_task_mitigation(struct task_struct *tsk) >> { >> struct pt_regs *regs = task_pt_regs(tsk); >> - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; >> + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); > > Hmm, the other two uses of PF_KTHREAD in arch/arm64 also look pretty > suspect in light of this proposal. Should we also update > ssbs_thread_switch() and access_ok()? If not, then a comment would be > handy to say why PF_KTHREAD is sufficient there. The uaccess one looks like, PF_IO_WORKER threads are just normal userspace threads. The only difference is that they never exit to userspace, they remain in the kernel. But everything else is just like a thread. But yes, the ssbs_thread_switch() should have this check too. I'll send out an updated patch. -- Jens Axboe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations 2023-01-26 14:07 ` Jens Axboe @ 2023-01-26 15:04 ` Catalin Marinas 2023-03-28 14:14 ` Will Deacon 1 sibling, 0 replies; 6+ messages in thread From: Catalin Marinas @ 2023-01-26 15:04 UTC (permalink / raw) To: Jens Axboe; +Cc: Will Deacon, linux-arm-kernel On Thu, Jan 26, 2023 at 07:07:45AM -0700, Jens Axboe wrote: > On 1/26/23 7:00 AM, Will Deacon wrote: > > On Wed, Jan 25, 2023 at 09:43:34AM -0700, Jens Axboe wrote: > >> Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist > >> entirely within the kernel, and hence don't need any task mitigations > >> applied. > >> > >> Signed-off-by: Jens Axboe <axboe@kernel.dk> > >> > >> --- > >> > >> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c > >> index fca9cc6f5581..25a21c3d446c 100644 > >> --- a/arch/arm64/kernel/proton-pack.c > >> +++ b/arch/arm64/kernel/proton-pack.c > >> @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) > >> void spectre_v4_enable_task_mitigation(struct task_struct *tsk) > >> { > >> struct pt_regs *regs = task_pt_regs(tsk); > >> - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; > >> + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); > > > > Hmm, the other two uses of PF_KTHREAD in arch/arm64 also look pretty > > suspect in light of this proposal. Should we also update > > ssbs_thread_switch() and access_ok()? If not, then a comment would be > > handy to say why PF_KTHREAD is sufficient there. > > The uaccess one looks like, PF_IO_WORKER threads are just normal userspace > threads. The only difference is that they never exit to userspace, they > remain in the kernel. But everything else is just like a thread. IIUC these threads are cloned from a user thread and inherit the TIF flags. If the user already opted in to the tagged addr ABI (usually early by libc), TIF_TAGGED_ADDR would be set for all threads and inherited by io_worker threads so we get away with this. But it doesn't hurt to extend the access_ok check to PF_IO_WORKER just in case the user plays with the late enabling of TIF_TAGGED_ADDR. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations 2023-01-26 14:07 ` Jens Axboe 2023-01-26 15:04 ` Catalin Marinas @ 2023-03-28 14:14 ` Will Deacon 2023-03-28 16:47 ` Jens Axboe 1 sibling, 1 reply; 6+ messages in thread From: Will Deacon @ 2023-03-28 14:14 UTC (permalink / raw) To: Jens Axboe; +Cc: Catalin Marinas, linux-arm-kernel Hey Jens, On Thu, Jan 26, 2023 at 07:07:45AM -0700, Jens Axboe wrote: > On 1/26/23 7:00 AM, Will Deacon wrote: > > On Wed, Jan 25, 2023 at 09:43:34AM -0700, Jens Axboe wrote: > >> Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist > >> entirely within the kernel, and hence don't need any task mitigations > >> applied. > >> > >> Signed-off-by: Jens Axboe <axboe@kernel.dk> > >> > >> --- > >> > >> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c > >> index fca9cc6f5581..25a21c3d446c 100644 > >> --- a/arch/arm64/kernel/proton-pack.c > >> +++ b/arch/arm64/kernel/proton-pack.c > >> @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) > >> void spectre_v4_enable_task_mitigation(struct task_struct *tsk) > >> { > >> struct pt_regs *regs = task_pt_regs(tsk); > >> - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; > >> + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); > > > > Hmm, the other two uses of PF_KTHREAD in arch/arm64 also look pretty > > suspect in light of this proposal. Should we also update > > ssbs_thread_switch() and access_ok()? If not, then a comment would be > > handy to say why PF_KTHREAD is sufficient there. > > The uaccess one looks like, PF_IO_WORKER threads are just normal userspace > threads. The only difference is that they never exit to userspace, they > remain in the kernel. But everything else is just like a thread. > > But yes, the ssbs_thread_switch() should have this check too. I'll send > out an updated patch. I think this one slipped through the cracks, as I don't see any usage of PF_IO_WORKER in arch/arm64/. Do you plan to update the patch? Cheers, Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations 2023-03-28 14:14 ` Will Deacon @ 2023-03-28 16:47 ` Jens Axboe 0 siblings, 0 replies; 6+ messages in thread From: Jens Axboe @ 2023-03-28 16:47 UTC (permalink / raw) To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel On 3/28/23 8:14?AM, Will Deacon wrote: > Hey Jens, > > On Thu, Jan 26, 2023 at 07:07:45AM -0700, Jens Axboe wrote: >> On 1/26/23 7:00?AM, Will Deacon wrote: >>> On Wed, Jan 25, 2023 at 09:43:34AM -0700, Jens Axboe wrote: >>>> Like PF_KTHREAD, PF_IO_WORKER never exit to userspace. They exist >>>> entirely within the kernel, and hence don't need any task mitigations >>>> applied. >>>> >>>> Signed-off-by: Jens Axboe <axboe@kernel.dk> >>>> >>>> --- >>>> >>>> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c >>>> index fca9cc6f5581..25a21c3d446c 100644 >>>> --- a/arch/arm64/kernel/proton-pack.c >>>> +++ b/arch/arm64/kernel/proton-pack.c >>>> @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) >>>> void spectre_v4_enable_task_mitigation(struct task_struct *tsk) >>>> { >>>> struct pt_regs *regs = task_pt_regs(tsk); >>>> - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; >>>> + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); >>> >>> Hmm, the other two uses of PF_KTHREAD in arch/arm64 also look pretty >>> suspect in light of this proposal. Should we also update >>> ssbs_thread_switch() and access_ok()? If not, then a comment would be >>> handy to say why PF_KTHREAD is sufficient there. >> >> The uaccess one looks like, PF_IO_WORKER threads are just normal userspace >> threads. The only difference is that they never exit to userspace, they >> remain in the kernel. But everything else is just like a thread. >> >> But yes, the ssbs_thread_switch() should have this check too. I'll send >> out an updated patch. > > I think this one slipped through the cracks, as I don't see any usage of > PF_IO_WORKER in arch/arm64/. Do you plan to update the patch? > Oops, yes looks like it did. Should look something like this, added a comment as well. I'll send it out properly. diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 71d59b5abede..9ce614b2177e 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -445,9 +445,11 @@ static void ssbs_thread_switch(struct task_struct *next) { /* * Nothing to do for kernel threads, but 'regs' may be junk - * (e.g. idle task) so check the flags and bail early. + * (e.g. idle task) so check the flags and bail early. Nothing to do + * for IO worker threads either, as they never transition between + * kernel and userspace. */ - if (unlikely(next->flags & PF_KTHREAD)) + if (unlikely(next->flags & (PF_KTHREAD | PF_IO_WORKER))) return; /* diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c index fca9cc6f5581..25a21c3d446c 100644 --- a/arch/arm64/kernel/proton-pack.c +++ b/arch/arm64/kernel/proton-pack.c @@ -654,7 +654,7 @@ static void __update_pstate_ssbs(struct pt_regs *regs, bool state) void spectre_v4_enable_task_mitigation(struct task_struct *tsk) { struct pt_regs *regs = task_pt_regs(tsk); - bool ssbs = false, kthread = tsk->flags & PF_KTHREAD; + bool ssbs = false, kthread = tsk->flags & (PF_KTHREAD | PF_IO_WORKER); if (spectre_v4_mitigations_off()) ssbs = true; -- Jens Axboe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-28 16:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-25 16:43 [PATCH] arm64: treat PF_IO_WORKER like PF_KTHREAD for mitigations Jens Axboe 2023-01-26 14:00 ` Will Deacon 2023-01-26 14:07 ` Jens Axboe 2023-01-26 15:04 ` Catalin Marinas 2023-03-28 14:14 ` Will Deacon 2023-03-28 16:47 ` Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).