linux-kernel - Re: [PATCH] apparmor: try to avoid refing the label in apparmor_file

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: <3ijkwqkrynfxi6t5bj2jingkpebsnomdcwduhe4pgl6pu25sfs@smvxx7ewexkc>
Date: Thu, 20 Jun 2024 18:41:22 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: John Johansen <john.johansen@...onical.com>
Cc: paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com, 
	linux-kernel@...r.kernel.org, apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org, 
	Neeraj.Upadhyay@....com
Subject: Re: [PATCH] apparmor: try to avoid refing the label in
 apparmor_file_open

On Thu, Jun 20, 2024 at 09:26:00AM -0700, John Johansen wrote:
> On 6/20/24 06:15, Mateusz Guzik wrote:
> > It can be done in the common case.
> > > A 24-way open1_processes from will-it-scale (separate file open) shows:
> >    29.37%  [kernel]           [k] apparmor_file_open
> >    26.84%  [kernel]           [k] apparmor_file_alloc_security
> >    26.62%  [kernel]           [k] apparmor_file_free_security
> >     1.32%  [kernel]           [k] clear_bhb_loop
> > 
> > apparmor_file_open is eliminated from the profile with the patch.
> > 
> > Throughput (ops/s):
> > before:	6092196
> > after:	8309726 (+36%)
> > 
> > Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
> can you cleanup the commit message and I will pull this in
> 

First of all thanks for a timely review.

I thought that's a decent commit message though. ;)

Would something like this work:
<cm>
apparmor: try to avoid refing the label in apparmor_file_open

In the common case it can be avoided, which in turn reduces the
performance impact apparmor on parallel open() invocations.

When benchmarking on 24-core vm using will-it-scale's open1_process
("Separate file open"), the results are (ops/s):
before: 6092196
after:  8309726 (+36%)
</cm>

If this is fine I'll send a v2.

If you are looking for something fundamentally different I would say it
will be the fastest if you write your own commit message while borrowing
the numbers and denoting all the wording is yours. I'm trying to reduce
back and forth over email here.

> > Am I missing something which makes the approach below not work to begin
> > with?
> > 
> no this will work in the short term. Long term there is work that will
> break this. Both replacing unconfined and the object delegation work
> will cause a performance regression as I am not sure we will be able
> to conditionally get the label but that is something for those patch
> series to work out. My biggest concern being people objecting to necessary
> changes that regress performance, if it can't be worked out, but
> that really isn't a reason to stop this now.
> 

hrm. I was looking at going a step further, now I'm going to have to
poke around.