| Message ID | 1405596402-23844-1-git-send-email-broonie@kernel.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > interface) removed the arch parameter from __audit_syscall_entry() and > updated the only current user in mainline but this breaks the ARMv8 audit > code that has been added in -next. Fix this by making the equivalent > update to ARMv8. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > arch/arm64/kernel/ptrace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > index 70526cfda056..310842e3d477 100644 > --- a/arch/arm64/kernel/ptrace.c > +++ b/arch/arm64/kernel/ptrace.c > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > trace_sys_enter(regs, regs->syscallno); > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > + regs->regs[2], regs->regs[3]); > > return regs->syscallno; > } Thanks. This will have to be applied after 3.17-rc1 since commit 3efe33f5d2 does not seem to be in mainline yet (nor the arm64 audit patches).
On Thu, Jul 17, 2014 at 12:59:38PM +0100, Catalin Marinas wrote: > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > interface) removed the arch parameter from __audit_syscall_entry() and > Thanks. This will have to be applied after 3.17-rc1 since commit > 3efe33f5d2 does not seem to be in mainline yet (nor the arm64 audit > patches). That's not great and is kind of missing the point of -next - the core idea is to find these inter-tree dependencies, ensure things work when they're merged together and avoid things like massive bisection breaks. Leaving things also causes problems for anyone trying to do integration testing on -next. What would be better would be if we could get the two trees synced up, in this case something like picking the audit change up in the ARMv8 tree (or vice versa) seems like it'd be quick and easy since it looks like neither tree uses topic branches.
On Thu, Jul 17, 2014 at 03:50:02PM +0100, Mark Brown wrote: > On Thu, Jul 17, 2014 at 12:59:38PM +0100, Catalin Marinas wrote: > > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > > interface) removed the arch parameter from __audit_syscall_entry() and > > > Thanks. This will have to be applied after 3.17-rc1 since commit > > 3efe33f5d2 does not seem to be in mainline yet (nor the arm64 audit > > patches). > > That's not great and is kind of missing the point of -next - the core > idea is to find these inter-tree dependencies, ensure things work when > they're merged together and avoid things like massive bisection breaks. > Leaving things also causes problems for anyone trying to do integration > testing on -next. > > What would be better would be if we could get the two trees synced up, > in this case something like picking the audit change up in the ARMv8 > tree (or vice versa) seems like it'd be quick and easy since it looks > like neither tree uses topic branches. It's not ideal indeed but applying this patch on my tree would break it. So we need a common base where the audit API is changed, guaranteed not to be rebased so that I can merge it in the arm64 for-next/core branch. Do you know which tree commit 3efe33f5d2 is in? I assume some tip branch?
On Thu, Jul 17, 2014 at 03:58:38PM +0100, Catalin Marinas wrote: > On Thu, Jul 17, 2014 at 03:50:02PM +0100, Mark Brown wrote: > > What would be better would be if we could get the two trees synced up, > > in this case something like picking the audit change up in the ARMv8 > > tree (or vice versa) seems like it'd be quick and easy since it looks > > like neither tree uses topic branches. > It's not ideal indeed but applying this patch on my tree would break it. > So we need a common base where the audit API is changed, guaranteed not > to be rebased so that I can merge it in the arm64 for-next/core branch. > Do you know which tree commit 3efe33f5d2 is in? I assume some tip > branch? It's the head commit in Eric's audit tree.
On 14/07/17, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > interface) removed the arch parameter from __audit_syscall_entry() and > updated the only current user in mainline but this breaks the ARMv8 audit > code that has been added in -next. Fix this by making the equivalent > update to ARMv8. > > Signed-off-by: Mark Brown <broonie@linaro.org> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> I'm not sure the best way to propagate this patch, but it will be necessary. > --- > arch/arm64/kernel/ptrace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > index 70526cfda056..310842e3d477 100644 > --- a/arch/arm64/kernel/ptrace.c > +++ b/arch/arm64/kernel/ptrace.c > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > trace_sys_enter(regs, regs->syscallno); > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > + regs->regs[2], regs->regs[3]); > > return regs->syscallno; > } > -- > 2.0.1 > - RGB -- Richard Guy Briggs <rbriggs@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
On Thu, Jul 24, 2014 at 08:12:37PM +0100, Mark Brown wrote: > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > From: Mark Brown <broonie@linaro.org> > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > interface) removed the arch parameter from __audit_syscall_entry() and > > updated the only current user in mainline but this breaks the ARMv8 audit > > code that has been added in -next. Fix this by making the equivalent > > update to ARMv8. > > This is still broken -next. It should now be temporarily fixed for the defconfig (CONFIG_AUTIDSYSCALL disabled). I'm reluctant to merge the audit tree in the arm64 tree as (a) I haven't seen a confirmation that this branch is stable and will go upstream as is and (b) there are x86 changes without Acks from the x86 maintainers (whether needed or not, I can't say). So my temporary patch is some #ifndef's around the offending code. It will be reverted and your patch applied once the audit tree meets the arm64 tree.
On Fri, Jul 25, 2014 at 10:21:09AM +0100, Catalin Marinas wrote: > On Thu, Jul 24, 2014 at 08:12:37PM +0100, Mark Brown wrote: > > This is still broken -next. > It should now be temporarily fixed for the defconfig > (CONFIG_AUTIDSYSCALL disabled). I'm reluctant to merge the audit tree in > the arm64 tree as (a) I haven't seen a confirmation that this branch is > stable and will go upstream as is and (b) there are x86 changes without > Acks from the x86 maintainers (whether needed or not, I can't say). I'd not expect it to be essential, it's just a function prototype change. > So my temporary patch is some #ifndef's around the offending code. It > will be reverted and your patch applied once the audit tree meets the > arm64 tree. That's good thanks, though it's still going to mean we have massive bisection breaks and anyone with an existing config will run into trouble - I'm going to be adding all*config builds to my daily tests soon too which will still trigger the issue). I suspect the easiest thing would be to cherry pick the commit over (it can always be reverted later on if there's an issue) if a shared branch can't be arranged. It's a real shame Eric hasn't replied here...
On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > interface) removed the arch parameter from __audit_syscall_entry() and > updated the only current user in mainline but this breaks the ARMv8 audit > code that has been added in -next. Fix this by making the equivalent > update to ARMv8. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > arch/arm64/kernel/ptrace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > index 70526cfda056..310842e3d477 100644 > --- a/arch/arm64/kernel/ptrace.c > +++ b/arch/arm64/kernel/ptrace.c > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > trace_sys_enter(regs, regs->syscallno); > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > + regs->regs[2], regs->regs[3]); Eric, Richard: when is 3efe33f5d2 ("audit: x86: drop arch from __audit_syscall_entry() interface") going to hit mainline? I've been holding off this fix until the offending commit is merged, but if that's not going to happen for 3.17, then we probably need to do something else to fix -next. Will
On Mon, 2014-08-11 at 10:09 +0100, Will Deacon wrote: > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > From: Mark Brown <broonie@linaro.org> > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > interface) removed the arch parameter from __audit_syscall_entry() and > > updated the only current user in mainline but this breaks the ARMv8 audit > > code that has been added in -next. Fix this by making the equivalent > > update to ARMv8. > > > > Signed-off-by: Mark Brown <broonie@linaro.org> > > --- > > arch/arm64/kernel/ptrace.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > > index 70526cfda056..310842e3d477 100644 > > --- a/arch/arm64/kernel/ptrace.c > > +++ b/arch/arm64/kernel/ptrace.c > > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > > trace_sys_enter(regs, regs->syscallno); > > > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > > + regs->regs[2], regs->regs[3]); > > Eric, Richard: when is 3efe33f5d2 ("audit: x86: drop arch from > __audit_syscall_entry() interface") going to hit mainline? I've been holding > off this fix until the offending commit is merged, but if that's not going > to happen for 3.17, then we probably need to do something else to fix -next. I think I'm being lazy this window and not oging to send a pull. So I'll pick up this fix as soon as rc1 cuts in my tree.
On Mon, Aug 11, 2014 at 02:22:24PM +0100, Eric Paris wrote: > On Mon, 2014-08-11 at 10:09 +0100, Will Deacon wrote: > > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > > From: Mark Brown <broonie@linaro.org> > > > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > > interface) removed the arch parameter from __audit_syscall_entry() and > > > updated the only current user in mainline but this breaks the ARMv8 audit > > > code that has been added in -next. Fix this by making the equivalent > > > update to ARMv8. > > > > > > Signed-off-by: Mark Brown <broonie@linaro.org> > > > --- > > > arch/arm64/kernel/ptrace.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > > > index 70526cfda056..310842e3d477 100644 > > > --- a/arch/arm64/kernel/ptrace.c > > > +++ b/arch/arm64/kernel/ptrace.c > > > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > > > trace_sys_enter(regs, regs->syscallno); > > > > > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > > > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > > > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > > > + regs->regs[2], regs->regs[3]); > > > > Eric, Richard: when is 3efe33f5d2 ("audit: x86: drop arch from > > __audit_syscall_entry() interface") going to hit mainline? I've been holding > > off this fix until the offending commit is merged, but if that's not going > > to happen for 3.17, then we probably need to do something else to fix -next. > > I think I'm being lazy this window and not oging to send a pull. So > I'll pick up this fix as soon as rc1 cuts in my tree. Oh, alright then. If you're not going to send the code for mainline, you could also just drop it from -next ;) Anyway, if you do fix it, please let me know so that I can remove our #ifdef CONFIG_AUDITSYSCALL guards (which only exist to stop build breakage in -next with defconfig). Will
On Mon, Aug 11, 2014 at 02:33:38PM +0100, Will Deacon wrote: > On Mon, Aug 11, 2014 at 02:22:24PM +0100, Eric Paris wrote: > > On Mon, 2014-08-11 at 10:09 +0100, Will Deacon wrote: > > > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > > > From: Mark Brown <broonie@linaro.org> > > > > > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > > > interface) removed the arch parameter from __audit_syscall_entry() and > > > > updated the only current user in mainline but this breaks the ARMv8 audit > > > > code that has been added in -next. Fix this by making the equivalent > > > > update to ARMv8. > > > > > > > > Signed-off-by: Mark Brown <broonie@linaro.org> > > > > --- > > > > arch/arm64/kernel/ptrace.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > > > > index 70526cfda056..310842e3d477 100644 > > > > --- a/arch/arm64/kernel/ptrace.c > > > > +++ b/arch/arm64/kernel/ptrace.c > > > > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > > > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > > > > trace_sys_enter(regs, regs->syscallno); > > > > > > > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > > > > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > > > > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > > > > + regs->regs[2], regs->regs[3]); > > > > > > Eric, Richard: when is 3efe33f5d2 ("audit: x86: drop arch from > > > __audit_syscall_entry() interface") going to hit mainline? I've been holding > > > off this fix until the offending commit is merged, but if that's not going > > > to happen for 3.17, then we probably need to do something else to fix -next. > > > > I think I'm being lazy this window and not oging to send a pull. So > > I'll pick up this fix as soon as rc1 cuts in my tree. > > Oh, alright then. If you're not going to send the code for mainline, you > could also just drop it from -next ;) > > Anyway, if you do fix it, please let me know so that I can remove our #ifdef > CONFIG_AUDITSYSCALL guards (which only exist to stop build breakage in -next > with defconfig). Actually, Eric could carry the arm64 change from Mark into -next as well and we can drop the arm64 #ifdef before the API change hits mainline.
On Mon, Aug 11, 2014 at 04:15:02PM +0100, Catalin Marinas wrote: > On Mon, Aug 11, 2014 at 02:33:38PM +0100, Will Deacon wrote: > > On Mon, Aug 11, 2014 at 02:22:24PM +0100, Eric Paris wrote: > > > On Mon, 2014-08-11 at 10:09 +0100, Will Deacon wrote: > > > > On Thu, Jul 17, 2014 at 12:26:42PM +0100, Mark Brown wrote: > > > > > From: Mark Brown <broonie@linaro.org> > > > > > > > > > > Commit 3efe33f5d2 (audit: x86: drop arch from __audit_syscall_entry() > > > > > interface) removed the arch parameter from __audit_syscall_entry() and > > > > > updated the only current user in mainline but this breaks the ARMv8 audit > > > > > code that has been added in -next. Fix this by making the equivalent > > > > > update to ARMv8. > > > > > > > > > > Signed-off-by: Mark Brown <broonie@linaro.org> > > > > > --- > > > > > arch/arm64/kernel/ptrace.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > > > > > index 70526cfda056..310842e3d477 100644 > > > > > --- a/arch/arm64/kernel/ptrace.c > > > > > +++ b/arch/arm64/kernel/ptrace.c > > > > > @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) > > > > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > > > > > trace_sys_enter(regs, regs->syscallno); > > > > > > > > > > - audit_syscall_entry(syscall_get_arch(), regs->syscallno, > > > > > - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); > > > > > + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], > > > > > + regs->regs[2], regs->regs[3]); > > > > > > > > Eric, Richard: when is 3efe33f5d2 ("audit: x86: drop arch from > > > > __audit_syscall_entry() interface") going to hit mainline? I've been holding > > > > off this fix until the offending commit is merged, but if that's not going > > > > to happen for 3.17, then we probably need to do something else to fix -next. > > > > > > I think I'm being lazy this window and not oging to send a pull. So > > > I'll pick up this fix as soon as rc1 cuts in my tree. > > > > Oh, alright then. If you're not going to send the code for mainline, you > > could also just drop it from -next ;) > > > > Anyway, if you do fix it, please let me know so that I can remove our #ifdef > > CONFIG_AUDITSYSCALL guards (which only exist to stop build breakage in -next > > with defconfig). > > Actually, Eric could carry the arm64 change from Mark into -next as well > and we can drop the arm64 #ifdef before the API change hits mainline. Indeed -- I think that's what we'd agreed, right? Will
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 70526cfda056..310842e3d477 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -1115,8 +1115,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, regs->syscallno); - audit_syscall_entry(syscall_get_arch(), regs->syscallno, - regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]); + audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], + regs->regs[2], regs->regs[3]); return regs->syscallno; }