Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1608986 > unrolled thread

Audit fixes for v4.11 (#1)

Back to article view | Back to linux.kernel

Contents

  Audit fixes for v4.11 (#1) Paul Moore <pmoore@redhat.com> - 2017-03-24 22:40 +0100
    [GIT PULL] Audit fixes for v4.11 (#1) Paul Moore <pmoore@redhat.com> - 2017-03-25 15:50 +0100
    Re: Audit fixes for v4.11 (#1) Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-25 23:30 +0100
      Re: Audit fixes for v4.11 (#1) Paul Moore <pmoore@redhat.com> - 2017-03-26 17:20 +0200
        Re: Audit fixes for v4.11 (#1) Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-26 19:30 +0200

#1608986 — Audit fixes for v4.11 (#1)

Hi Linus,

We've got an audit fix, and unfortunately it is two things I don't
like: big and based on a -rcX tag.  The size of the patch is
(hopefully) explained well in the patch description, the -rcX base is
to get access to code not present in the v4.11 pull request
(audit/next is still based off v4.8; I'll be updating soon).  While
I'm not excited that we need to be sending you something this large
during the -rcX phase, it does fix some very real, and very tangled,
problems relating to locking, backlog queues, and the audit daemon
connection.

This code has passed our testsuite without problem and it has held up
to my ad-hoc stress tests (arguably better than the existing code),
please consider pulling this as fix for the next v4.11-rcX tag.

Thanks,
-Paul

---
The following changes since commit 97da3854c526d3a6ee05c849c96e48d21527606c:

 Linux 4.11-rc3 (2017-03-19 19:09:39 -0700)

are available in the git repository at:

 git://git.infradead.org/users/pcmoore/audit stable-4.11

for you to fetch changes up to 5b52330bbfe63b3305765354d6046c9f7f89c011:

 audit: fix auditd/kernel connection state tracking (2017-03-21 11:26:35 -0400)

----------------------------------------------------------------
Paul Moore (1):
     audit: fix auditd/kernel connection state tracking

kernel/audit.c   | 639 ++++++++++++++++++++++++++++++++++---------------------
kernel/audit.h   |   9 +-
kernel/auditsc.c |   6 +-
3 files changed, 399 insertions(+), 255 deletions(-)

-- 
paul moore
security @ redhat

[toc] | [next] | [standalone]

#1609171 — [GIT PULL] Audit fixes for v4.11 (#1)

On Fri, Mar 24, 2017 at 5:39 PM, Paul Moore <pmoore@redhat.com> wrote:
> Hi Linus,
>
> We've got an audit fix, and unfortunately it is two things I don't
> like: big and based on a -rcX tag.  The size of the patch is
> (hopefully) explained well in the patch description, the -rcX base is
> to get access to code not present in the v4.11 pull request
> (audit/next is still based off v4.8; I'll be updating soon).  While
> I'm not excited that we need to be sending you something this large
> during the -rcX phase, it does fix some very real, and very tangled,
> problems relating to locking, backlog queues, and the audit daemon
> connection.
>
> This code has passed our testsuite without problem and it has held up
> to my ad-hoc stress tests (arguably better than the existing code),
> please consider pulling this as fix for the next v4.11-rcX tag.
>
> Thanks,
> -Paul
>
> ---
> The following changes since commit 97da3854c526d3a6ee05c849c96e48d21527606c:
>
>  Linux 4.11-rc3 (2017-03-19 19:09:39 -0700)
>
> are available in the git repository at:
>
>  git://git.infradead.org/users/pcmoore/audit stable-4.11
>
> for you to fetch changes up to 5b52330bbfe63b3305765354d6046c9f7f89c011:
>
>  audit: fix auditd/kernel connection state tracking (2017-03-21 11:26:35 -0400)
>
> ----------------------------------------------------------------
> Paul Moore (1):
>      audit: fix auditd/kernel connection state tracking
>
> kernel/audit.c   | 639 ++++++++++++++++++++++++++++++++++---------------------
> kernel/audit.h   |   9 +-
> kernel/auditsc.c |   6 +-
> 3 files changed, 399 insertions(+), 255 deletions(-)

It would appear that I left off the magic "[GIT PULL]" subject prefix
in my last message.  I suppose this is one more reason why one
shouldn't send pull requests past 5:00p on a Friday.

-- 
paul moore
security @ redhat

[toc] | [prev] | [next] | [standalone]

#1609252

On Fri, Mar 24, 2017 at 2:39 PM, Paul Moore <pmoore@redhat.com> wrote:
>
> This code has passed our testsuite without problem and it has held up
> to my ad-hoc stress tests (arguably better than the existing code),
> please consider pulling this as fix for the next v4.11-rcX tag.

Ok, pulled. However, looking at the changes in the patch it becomes
obvious that it is now completely bogus to inline the
"audit_signal_info()" function.

That silly inline in the header file now only generates bigger and
slower code, since that inlined function now calls another function
auditd_test_task() that is *not* inlined, so it ends up being a
function call anyway.

It would be much better to just unlinline audit_signal_info(), move it
into kernel/audit.c, and let the compiler then inline the
__audit_signal_info() helper function (or just fold it into that
function manually as part of the move).

The whole reason for that inlined part, and the uninlined
__audit_signal_info() helper was that the code *used* to be able to
avoid a function call entirely. That reason is now gone.

                 Linus

[toc] | [prev] | [next] | [standalone]

#1609341

On Sat, Mar 25, 2017 at 6:23 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Mar 24, 2017 at 2:39 PM, Paul Moore <pmoore@redhat.com> wrote:
>>
>> This code has passed our testsuite without problem and it has held up
>> to my ad-hoc stress tests (arguably better than the existing code),
>> please consider pulling this as fix for the next v4.11-rcX tag.
>
> Ok, pulled. However, looking at the changes in the patch it becomes
> obvious that it is now completely bogus to inline the
> "audit_signal_info()" function.
>
> That silly inline in the header file now only generates bigger and
> slower code, since that inlined function now calls another function
> auditd_test_task() that is *not* inlined, so it ends up being a
> function call anyway.

Good catch.  I was so worried about the other issues in this patch I
missed this obvious point.

> It would be much better to just unlinline audit_signal_info(), move it
> into kernel/audit.c, and let the compiler then inline the
> __audit_signal_info() helper function (or just fold it into that
> function manually as part of the move).
>
> The whole reason for that inlined part, and the uninlined
> __audit_signal_info() helper was that the code *used* to be able to
> avoid a function call entirely. That reason is now gone.

Agreed.  Normally I would say let's just fix it in audit/next and I'll
send it to you during the next merge window; however, since we're
breaking the whole point of this inline in the -rcX stage, and the
uninline'ing patch would be rather trivial, would you prefer I send it
to you now for v4.11?

-- 
paul moore
security @ redhat

[toc] | [prev] | [next] | [standalone]

#1609364

On Sun, Mar 26, 2017 at 8:18 AM, Paul Moore <pmoore@redhat.com> wrote:
> On Sat, Mar 25, 2017 at 6:23 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> The whole reason for that inlined part, and the uninlined
>> __audit_signal_info() helper was that the code *used* to be able to
>> avoid a function call entirely. That reason is now gone.
>
> Agreed.  Normally I would say let's just fix it in audit/next and I'll
> send it to you during the next merge window; however, since we're
> breaking the whole point of this inline in the -rcX stage, and the
> uninline'ing patch would be rather trivial, would you prefer I send it
> to you now for v4.11?

It's fine if it does into some "next" branch for 4.12, I don't think
there is any hurry as long as this gets fixed eventually.

But I'll take the obvious cleanup for 4.11 too if you end up having
other things coming my way.

                      Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web