From: Casey Schaufler <casey@schaufler-ca.com>
To: Tomasz Stanislawski <t.stanislaws@samsung.com>
Cc: linux-security-module@vger.kernel.org, m.szyprowski@samsung.com,
kyungmin.park@samsung.com, r.krypa@samsung.com,
linux-kernel@vger.kernel.org,
Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [RFC] security: smack: add hash table for smack for quick label searching
Date: Thu, 11 Apr 2013 10:59:22 -0700 [thread overview]
Message-ID: <5166F9FA.20003@schaufler-ca.com> (raw)
In-Reply-To: <1365669972-21461-1-git-send-email-t.stanislaws@samsung.com>
On 4/11/2013 1:46 AM, Tomasz Stanislawski wrote:
> Hi everyone,
> I am a developer working on optimization of the TIZEN system.
> Recently, I've discovered a performance issue in SMACK subsystem.
> I used the PERF tool to find performance bottlenecks.
>
> The test scenario was simple. Run multiple applications and
> see what the system does using the following command:
>
> perf record -a -g
>
> Next, see the results with the command:
>
> perf report -s symbol -g graph,0.5
>
> Among the many lines, the following ones are especially interesting:
>
> 5.96% [k] smk_find_entry
> |
> |--5.06%-- smk_access
> | |
> | --4.99%-- smk_curacc
> | |
> | |--3.79%-- smack_ptrace_access_check
> | | security_ptrace_access_check
> | | __ptrace_may_access
> | | ptrace_may_access
> | | |
> | | --3.78%-- mm_access
> | | mm_for_maps
> | | m_start
> | | seq_read
> | | vfs_read
> | | sys_read
> | | ret_fast_syscall
> | | |
> | | --3.19%-- (nil)
> | |
> | --0.71%-- smack_inode_permission
> | security_inode_permission
> | inode_permission
> |
> --0.89%-- smack_to_secid
> smack_socket_getpeersec_dgram
> security_socket_getpeersec_dgram
> |
> --0.54%-- unix_stream_sendmsg
>
> 4.63% [k] strcmp
> |
> |--2.16%-- smk_find_entry
> | |
> | --1.92%-- smk_access
> | |
> | --1.85%-- smk_curacc
> | |
> | --1.20%-- smack_ptrace_access_check
> | security_ptrace_access_check
> | __ptrace_may_access
> | ptrace_may_access
> | mm_access
> | mm_for_maps
> | m_start
> | seq_read
> | vfs_read
> | sys_read
> | ret_fast_syscall
> | |
> | --0.99%-- (nil)
> |
> --2.14%-- smk_access
> |
> --2.11%-- smk_curacc
> |
> --1.75%-- smack_ptrace_access_check
> security_ptrace_access_check
> __ptrace_may_access
> ptrace_may_access
> |
> --1.73%-- mm_access
> mm_for_maps
> m_start
> seq_read
> vfs_read
> sys_read
> ret_fast_syscall
> |
> --1.40%-- (nil)
>
> To sum up, the result indicates that the CPU spents circa 8% (2.16% + 5.96%)
> of cycles searching for a SMACK label in the smk_find_entry function.
> The function iterates through smack_known_list to find an entry.
> The further analysis showed that the size of the list can reach even 600.
> I measured that it takes circa 200 tries to find an entry on average.
> The value was computed as a total number iterations in the smk_find_entry's
> loop divided by the times smk_find_entry was called in a time-window of
> the length of 10 seconds.
>
> IMO, this is a serious performance issue which scales badly with
> a complexity of the system.
>
> I implemented a patch that makes a use of a hash table to quicken searching
> for SMACK's labels. The patch is rebased onto the latest v3.9-rc6 kernel.
> The code is thread-safe (I hope) because it shares the RCU mechanism
> and locks with smack_known_list.
>
> There is still some place for improvements like:
> a) using struct hlist_head instead of struct list_head to reduce
> the memory size of the hash table.
>
> OR
>
> b) use smack_known::list instead of introducing smack_known::htab_list
> and modify all smack_known_list related code to iterate over
> the hash table.
>
> I decided to postpone the mentioned improvements for a sake of simplicity
> of this RFC. After applying the patch, the smk_find_entry overhead was
> reduced to mere 0.05% of CPU cycles.
>
> I hope you find the measurement and the patch useful.
> All comments are welcome.
NAK
There will be no hash tables in Smack.
The correct solution is simple.
In the task_smack structure there are two Smack label pointers,
smk_task and smk_forked. Replace these fields with pointers to
the smack_known structures that contain the Smack label pointers
used today. This will require trivial changes throughout the
Smack code to accommodate the type change and a few logical twists
around smk_import. It will eliminate the need for smk_lookup_entry.
>
> Regards,
> Tomasz Stanislawski
>
>
> Tomasz Stanislawski (1):
> security: smack: add hash table for smack for quick label searching
>
> security/smack/smack.h | 5 +++++
> security/smack/smack_access.c | 33 +++++++++++++++++++++++++++++++--
> security/smack/smack_lsm.c | 21 +++++++++++++++------
> 3 files changed, 51 insertions(+), 8 deletions(-)
>
next prev parent reply other threads:[~2013-04-11 18:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-11 8:46 [RFC] security: smack: add hash table for smack for quick label searching Tomasz Stanislawski
2013-04-11 8:46 ` Tomasz Stanislawski
2013-06-08 20:26 ` Casey Schaufler
2013-04-11 17:59 ` Casey Schaufler [this message]
2013-04-12 15:12 ` Łukasz Stelmach
2013-04-12 18:00 ` Casey Schaufler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5166F9FA.20003@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=r.krypa@samsung.com \
--cc=t.stanislaws@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).