[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5166F9FA.20003@schaufler-ca.com>
Date: Thu, 11 Apr 2013 10:59:22 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Tomasz Stanislawski <t.stanislaws@...sung.com>
CC: linux-security-module@...r.kernel.org, m.szyprowski@...sung.com,
kyungmin.park@...sung.com, r.krypa@...sung.com,
linux-kernel@...r.kernel.org,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [RFC] security: smack: add hash table for smack for quick label
searching
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(-)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists