Hi Stephen et al.,
There is a contextual conflict between commit e1d337335207 ("cfi: Fix
__cfi_slowpath_diag RCU usage with cpuidle") in the kspp tree and commit
dcc0c11aa87b ("rcu/context-tracking: Remove rcu_irq_enter/exit()") in
the rcu tree, which is visible when building ARCH=arm64 defconfig +
CONFIG_LTO_CLANG_THIN=y + CONFIG_CFI_CLANG=y with clang:
kernel/cfi.c:298:3: error: call to undeclared function 'rcu_irq_enter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
rcu_irq_enter();
^
kernel/cfi.c:298:3: note: did you mean 'ct_irq_enter'?
./include/linux/context_tracking_irq.h:6:6: note: 'ct_irq_enter' declared here
void ct_irq_enter(void);
^
kernel/cfi.c:307:3: error: call to undeclared function 'rcu_irq_exit'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
rcu_irq_exit();
^
kernel/cfi.c:307:3: note: did you mean 'ct_irq_exit'?
./include/linux/context_tracking_irq.h:7:6: note: 'ct_irq_exit' declared here
void ct_irq_exit(void);
^
2 errors generated.
Per the above RCU commit and commit 6c5218715286 ("context_tracking:
Take IRQ eqs entrypoints over RCU"), it appears that the following diff
is the proper fix up. Would you mind applying it to the merge of
whichever tree comes second if possible? I did build and boot test it
but it would not be a bad idea for Sami and Frederic to verify that it
is correct so that Kees/Paul can mention it to Linus :)
Cheers,
Nathan
diff --git a/kernel/cfi.c b/kernel/cfi.c
index 08102d19ec15..2046276ee234 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -295,7 +295,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
rcu_idle = !rcu_is_watching();
if (rcu_idle) {
local_irq_save(flags);
- rcu_irq_enter();
+ ct_irq_enter();
}
if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW))
@@ -304,7 +304,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
fn = find_module_check_fn(ptr);
if (rcu_idle) {
- rcu_irq_exit();
+ ct_irq_exit();
local_irq_restore(flags);
}
On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> Hi Stephen et al.,
>
> There is a contextual conflict between commit e1d337335207 ("cfi: Fix
> __cfi_slowpath_diag RCU usage with cpuidle") in the kspp tree and commit
> dcc0c11aa87b ("rcu/context-tracking: Remove rcu_irq_enter/exit()") in
> the rcu tree, which is visible when building ARCH=arm64 defconfig +
> CONFIG_LTO_CLANG_THIN=y + CONFIG_CFI_CLANG=y with clang:
>
> kernel/cfi.c:298:3: error: call to undeclared function 'rcu_irq_enter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_enter();
> ^
> kernel/cfi.c:298:3: note: did you mean 'ct_irq_enter'?
> ./include/linux/context_tracking_irq.h:6:6: note: 'ct_irq_enter' declared here
> void ct_irq_enter(void);
> ^
> kernel/cfi.c:307:3: error: call to undeclared function 'rcu_irq_exit'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_exit();
> ^
> kernel/cfi.c:307:3: note: did you mean 'ct_irq_exit'?
> ./include/linux/context_tracking_irq.h:7:6: note: 'ct_irq_exit' declared here
> void ct_irq_exit(void);
> ^
> 2 errors generated.
>
>
> Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> is the proper fix up. Would you mind applying it to the merge of
> whichever tree comes second if possible? I did build and boot test it
> but it would not be a bad idea for Sami and Frederic to verify that it
> is correct so that Kees/Paul can mention it to Linus :)
Ah! Thanks for tracking this down! Maybe dcc0c11aa87b should be updated
to leave an alias until the next merge window?
--
Kees Cook
On Wed, Jun 15, 2022 at 01:28:00PM -0700, Kees Cook wrote:
> On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> > Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> > Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> > is the proper fix up. Would you mind applying it to the merge of
> > whichever tree comes second if possible? I did build and boot test it
> > but it would not be a bad idea for Sami and Frederic to verify that it
> > is correct so that Kees/Paul can mention it to Linus :)
>
> Actually, the CFI fix (and a few others) are meant to be sent for
> -rc3, so if the ct_irq_enter() change is in -next, this can maybe get
> sorted out?
Indeed, Frederic's context-tracking series is for the next merge window.
So once your changes are added, I will adjust.
Thanx, Paul
On Wed, Jun 15, 2022 at 01:28:00PM -0700, Kees Cook wrote:
> On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> > Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> > Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> > is the proper fix up. Would you mind applying it to the merge of
> > whichever tree comes second if possible? I did build and boot test it
> > but it would not be a bad idea for Sami and Frederic to verify that it
> > is correct so that Kees/Paul can mention it to Linus :)
>
> Actually, the CFI fix (and a few others) are meant to be sent for
> -rc3, so if the ct_irq_enter() change is in -next, this can maybe get
> sorted out?
Ah, I had assumed that branch was destined for the next release. If it
is for 5.19 and they make -rc3 then it should be trivial for Paul to
either rebase the changes on -rc3 and apply that diff as part of
"context_tracking: Take IRQ eqs entrypoints over RCU" (if his tree is
mutable) or just merge -rc3 and apply that diff as part of the merge. I
don't really care how it gets resolved, just so long as it does :)
Cheers,
Nathan
On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> is the proper fix up. Would you mind applying it to the merge of
> whichever tree comes second if possible? I did build and boot test it
> but it would not be a bad idea for Sami and Frederic to verify that it
> is correct so that Kees/Paul can mention it to Linus :)
Actually, the CFI fix (and a few others) are meant to be sent for
-rc3, so if the ct_irq_enter() change is in -next, this can maybe get
sorted out?
--
Kees Cook
On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> Hi Stephen et al.,
>
> There is a contextual conflict between commit e1d337335207 ("cfi: Fix
> __cfi_slowpath_diag RCU usage with cpuidle") in the kspp tree and commit
> dcc0c11aa87b ("rcu/context-tracking: Remove rcu_irq_enter/exit()") in
> the rcu tree, which is visible when building ARCH=arm64 defconfig +
> CONFIG_LTO_CLANG_THIN=y + CONFIG_CFI_CLANG=y with clang:
>
> kernel/cfi.c:298:3: error: call to undeclared function 'rcu_irq_enter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_enter();
> ^
> kernel/cfi.c:298:3: note: did you mean 'ct_irq_enter'?
> ./include/linux/context_tracking_irq.h:6:6: note: 'ct_irq_enter' declared here
> void ct_irq_enter(void);
> ^
> kernel/cfi.c:307:3: error: call to undeclared function 'rcu_irq_exit'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_exit();
> ^
> kernel/cfi.c:307:3: note: did you mean 'ct_irq_exit'?
> ./include/linux/context_tracking_irq.h:7:6: note: 'ct_irq_exit' declared here
> void ct_irq_exit(void);
> ^
> 2 errors generated.
>
>
> Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> is the proper fix up. Would you mind applying it to the merge of
> whichever tree comes second if possible? I did build and boot test it
> but it would not be a bad idea for Sami and Frederic to verify that it
> is correct so that Kees/Paul can mention it to Linus :)
Agreed on Frederic verifying it, but in the meantime, it looks good to
me as well.
Thanx, Paul
> Cheers,
> Nathan
>
> diff --git a/kernel/cfi.c b/kernel/cfi.c
> index 08102d19ec15..2046276ee234 100644
> --- a/kernel/cfi.c
> +++ b/kernel/cfi.c
> @@ -295,7 +295,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> rcu_idle = !rcu_is_watching();
> if (rcu_idle) {
> local_irq_save(flags);
> - rcu_irq_enter();
> + ct_irq_enter();
> }
>
> if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW))
> @@ -304,7 +304,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> fn = find_module_check_fn(ptr);
>
> if (rcu_idle) {
> - rcu_irq_exit();
> + ct_irq_exit();
> local_irq_restore(flags);
> }
>
On Wed, Jun 15, 2022 at 01:35:28PM -0700, Nathan Chancellor wrote:
> On Wed, Jun 15, 2022 at 01:28:00PM -0700, Kees Cook wrote:
> > On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> > > Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> > > Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> > > is the proper fix up. Would you mind applying it to the merge of
> > > whichever tree comes second if possible? I did build and boot test it
> > > but it would not be a bad idea for Sami and Frederic to verify that it
> > > is correct so that Kees/Paul can mention it to Linus :)
> >
> > Actually, the CFI fix (and a few others) are meant to be sent for
> > -rc3, so if the ct_irq_enter() change is in -next, this can maybe get
> > sorted out?
>
> Ah, I had assumed that branch was destined for the next release. If it
> is for 5.19 and they make -rc3 then it should be trivial for Paul to
> either rebase the changes on -rc3 and apply that diff as part of
> "context_tracking: Take IRQ eqs entrypoints over RCU" (if his tree is
> mutable) or just merge -rc3 and apply that diff as part of the merge. I
> don't really care how it gets resolved, just so long as it does :)
My tree will remain mutable for a few more weeks, so we should be
good. ;-)
Thanx, Paul
Hi Nathan,
On Wed, 15 Jun 2022 12:55:53 -0700 Nathan Chancellor <[email protected]> wrote:
>
> There is a contextual conflict between commit e1d337335207 ("cfi: Fix
> __cfi_slowpath_diag RCU usage with cpuidle") in the kspp tree and commit
> dcc0c11aa87b ("rcu/context-tracking: Remove rcu_irq_enter/exit()") in
> the rcu tree, which is visible when building ARCH=arm64 defconfig +
> CONFIG_LTO_CLANG_THIN=y + CONFIG_CFI_CLANG=y with clang:
>
> kernel/cfi.c:298:3: error: call to undeclared function 'rcu_irq_enter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_enter();
> ^
> kernel/cfi.c:298:3: note: did you mean 'ct_irq_enter'?
> ./include/linux/context_tracking_irq.h:6:6: note: 'ct_irq_enter' declared here
> void ct_irq_enter(void);
> ^
> kernel/cfi.c:307:3: error: call to undeclared function 'rcu_irq_exit'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> rcu_irq_exit();
> ^
> kernel/cfi.c:307:3: note: did you mean 'ct_irq_exit'?
> ./include/linux/context_tracking_irq.h:7:6: note: 'ct_irq_exit' declared here
> void ct_irq_exit(void);
> ^
> 2 errors generated.
>
>
> Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> is the proper fix up. Would you mind applying it to the merge of
> whichever tree comes second if possible? I did build and boot test it
> but it would not be a bad idea for Sami and Frederic to verify that it
> is correct so that Kees/Paul can mention it to Linus :)
>
> Cheers,
> Nathan
>
> diff --git a/kernel/cfi.c b/kernel/cfi.c
> index 08102d19ec15..2046276ee234 100644
> --- a/kernel/cfi.c
> +++ b/kernel/cfi.c
> @@ -295,7 +295,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> rcu_idle = !rcu_is_watching();
> if (rcu_idle) {
> local_irq_save(flags);
> - rcu_irq_enter();
> + ct_irq_enter();
> }
>
> if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW))
> @@ -304,7 +304,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> fn = find_module_check_fn(ptr);
>
> if (rcu_idle) {
> - rcu_irq_exit();
> + ct_irq_exit();
> local_irq_restore(flags);
> }
>
I will apply that to the merge of the rcu tree today (unless Paul finds
time to update (and test :-) ) the rcu tree before I get to it) as the
CFI fix is now in Linus' tree.
--
Cheers,
Stephen Rothwell
Hi all,
On Wed, 15 Jun 2022 14:12:20 -0700 "Paul E. McKenney" <[email protected]> wrote:
>
> On Wed, Jun 15, 2022 at 01:35:28PM -0700, Nathan Chancellor wrote:
> > On Wed, Jun 15, 2022 at 01:28:00PM -0700, Kees Cook wrote:
> > > On Wed, Jun 15, 2022 at 12:55:53PM -0700, Nathan Chancellor wrote:
> > > > Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> > > > Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> > > > is the proper fix up. Would you mind applying it to the merge of
> > > > whichever tree comes second if possible? I did build and boot test it
> > > > but it would not be a bad idea for Sami and Frederic to verify that it
> > > > is correct so that Kees/Paul can mention it to Linus :)
> > >
> > > Actually, the CFI fix (and a few others) are meant to be sent for
> > > -rc3, so if the ct_irq_enter() change is in -next, this can maybe get
> > > sorted out?
> >
> > Ah, I had assumed that branch was destined for the next release. If it
> > is for 5.19 and they make -rc3 then it should be trivial for Paul to
> > either rebase the changes on -rc3 and apply that diff as part of
> > "context_tracking: Take IRQ eqs entrypoints over RCU" (if his tree is
> > mutable) or just merge -rc3 and apply that diff as part of the merge. I
> > don't really care how it gets resolved, just so long as it does :)
>
> My tree will remain mutable for a few more weeks, so we should be
> good. ;-)
The CFI fix is in Linus' tree today.
Paul, you could just merge commit 1dfbe9fcda4a ("usercopy: Make
usercopy resilient against ridiculously large copies") from Linus' tree
(or rebase on top of that or 30306f6194ca "Merge tag
"hardening-v5.19-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"
since it is based on -rc2).
--
Cheers,
Stephen Rothwell
On Thu, Jun 16, 2022 at 09:16:34AM +1000, Stephen Rothwell wrote:
> Hi Nathan,
>
> On Wed, 15 Jun 2022 12:55:53 -0700 Nathan Chancellor <[email protected]> wrote:
> >
> > There is a contextual conflict between commit e1d337335207 ("cfi: Fix
> > __cfi_slowpath_diag RCU usage with cpuidle") in the kspp tree and commit
> > dcc0c11aa87b ("rcu/context-tracking: Remove rcu_irq_enter/exit()") in
> > the rcu tree, which is visible when building ARCH=arm64 defconfig +
> > CONFIG_LTO_CLANG_THIN=y + CONFIG_CFI_CLANG=y with clang:
> >
> > kernel/cfi.c:298:3: error: call to undeclared function 'rcu_irq_enter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> > rcu_irq_enter();
> > ^
> > kernel/cfi.c:298:3: note: did you mean 'ct_irq_enter'?
> > ./include/linux/context_tracking_irq.h:6:6: note: 'ct_irq_enter' declared here
> > void ct_irq_enter(void);
> > ^
> > kernel/cfi.c:307:3: error: call to undeclared function 'rcu_irq_exit'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> > rcu_irq_exit();
> > ^
> > kernel/cfi.c:307:3: note: did you mean 'ct_irq_exit'?
> > ./include/linux/context_tracking_irq.h:7:6: note: 'ct_irq_exit' declared here
> > void ct_irq_exit(void);
> > ^
> > 2 errors generated.
> >
> >
> > Per the above RCU commit and commit 6c5218715286 ("context_tracking:
> > Take IRQ eqs entrypoints over RCU"), it appears that the following diff
> > is the proper fix up. Would you mind applying it to the merge of
> > whichever tree comes second if possible? I did build and boot test it
> > but it would not be a bad idea for Sami and Frederic to verify that it
> > is correct so that Kees/Paul can mention it to Linus :)
> >
> > Cheers,
> > Nathan
> >
> > diff --git a/kernel/cfi.c b/kernel/cfi.c
> > index 08102d19ec15..2046276ee234 100644
> > --- a/kernel/cfi.c
> > +++ b/kernel/cfi.c
> > @@ -295,7 +295,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> > rcu_idle = !rcu_is_watching();
> > if (rcu_idle) {
> > local_irq_save(flags);
> > - rcu_irq_enter();
> > + ct_irq_enter();
> > }
> >
> > if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW))
> > @@ -304,7 +304,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
> > fn = find_module_check_fn(ptr);
> >
> > if (rcu_idle) {
> > - rcu_irq_exit();
> > + ct_irq_exit();
> > local_irq_restore(flags);
> > }
> >
>
> I will apply that to the merge of the rcu tree today (unless Paul finds
> time to update (and test :-) ) the rcu tree before I get to it) as the
> CFI fix is now in Linus' tree.
Please!
In theory, I could rebase -rcu to linus/master now, but in practice the
odds of it producing something useful are all too low. I will take care
of it tomorrow (Thursday) morning, Pacific Time.
Thanx, Paul
Hi Paul,
On Wed, 15 Jun 2022 18:26:29 -0700 "Paul E. McKenney" <[email protected]> wrote:
>
> > I will apply that to the merge of the rcu tree today (unless Paul finds
> > time to update (and test :-) ) the rcu tree before I get to it) as the
> > CFI fix is now in Linus' tree.
>
> Please!
>
> In theory, I could rebase -rcu to linus/master now, but in practice the
> odds of it producing something useful are all too low. I will take care
> of it tomorrow (Thursday) morning, Pacific Time.
No worries.
--
Cheers,
Stephen Rothwell