lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHC9VhR4mnoj=uEEO8ceFvtHAy=ziUROnVHgoNAnH1Gr2+tH5g@mail.gmail.com>
Date:   Fri, 2 Jun 2023 10:56:04 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Ignat Korchagin <ignat@...udflare.com>
Cc:     Ivan Babrou <ivan@...udflare.com>, audit@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-team@...udflare.com,
        Eric Paris <eparis@...hat.com>
Subject: Re: [PATCH] audit: check syscall bitmap on entry to avoid extra work

On Fri, Jun 2, 2023 at 7:08 AM Ignat Korchagin <ignat@...udflare.com> wrote:
> On Thu, May 25, 2023 at 3:15 AM Paul Moore <paul@...l-moore.com> wrote:
> > On Wed, May 24, 2023 at 2:05 PM Ivan Babrou <ivan@...udflare.com> wrote:
> > > On Tue, May 23, 2023 at 7:03 PM Paul Moore <paul@...l-moore.com> wrote:
> > > > > Could you elaborate on what exactly you would like to see added? It's
> > > > > not clear to me what is missing.
> > > >
> > > > I should have been more clear, let me try again ...
> > > >
> > > > From my perspective, this patch adds code and complexity to deal with
> > > > the performance impact of auditing.  In some cases that is the right
> > > > thing to do, but I would much rather see a more in-depth analysis of
> > > > where the audit hot spots are in this benchmark, and some thoughts on
> > > > how we might improve that.  In other words, don't just add additional
> > > > processing to bypass (slower, more involved) processing; look at the
> > > > processing that is currently being done and see if you can find a way
> > > > to make it faster.  It will likely take longer, but the results will
> > > > be much more useful.
> > >
> > > The fastest way to do something is to not do it to begin with.
> >
> > While you are not wrong, I believe you and I are focusing on different
> > things.  From my perspective, you appear primarily concerned with
> > improving performance by reducing the overhead of auditing.  I too am
> > interested in reducing the audit overhead, but I also place a very
> > high value on maintainable code, perhaps more than performance simply
> > because the current audit code quality is so very poor.
> > Unfortunately, the patch you posted appears to me as yet another
> > bolt-on performance tweak that doesn't make an attempt at analyzing
> > the current hot spots of syscall auditing, and ideally offering
> > solutions.  Perhaps ultimately this approach is the only sane thing
> > that can be done, but I'd like to see some analysis first of the
> > syscall auditing path.
>
> Ivan is out of office, but I would like to keep the discussion going.
> We do understand your position and we're actually doing a project
> right now to investigate audit performance better ...

That's great, thank you!

> But the way I see it - the audit subsystem performance and the way how
> that subsystem plugs into the rest of the kernel code are two somewhat
> independent things with the patch proposed here addressing the latter
> (with full understanding that the former might be improved as well) ...

You've done a good job explaining the reasoning and motivations behind
the patch submitted, that is good, but I'm not seeing any recognition
or understanding about the perspective I shared with you earlier.  The
performance of audit in general does need to be improved, I don't
think anyone disagrees with that, but my argument is that we need to
focus on changes which not only reduce the processing overhead, but
*also* reduce the complexity of the code as well.

-- 
paul-moore.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ