Skip to content

Conversation

@arter97
Copy link
Contributor

@arter97 arter97 commented May 30, 2023

Hi @tiann.

Thanks for the great project, I had great fun playing around with it.

This PR mainly tries to further minimize the possible delays caused by KernelSU hooking.

There are 3 major changes:

  • Processes with 0 < UID < 2000 are blocked straight-up before going through the allow_list.
    I don't see any need for such processes to be interested in root, and this allows returning early before going through a more expensive lookup.
    If there's an expected breakage due to this change, I'll remove it. Let me know.
  • A page-sized (4K) bitmap is added.
    This allows O(1) lookup for UID <= 32767.
    This speeds up ksu_is_allow_uid() by about 4.8x by sacrificing a 4K memory. IMHO, a good trade-off.
    Most notably, this reduces the 99.999% result previously from worrying milliseconds scale to microseconds scale.
    For UID > 32767, another page-sized (4K) sequential array is used to cache allow_list.

Compared to the previous PR #557, this new approach gives another nice 25% performance boost in average, 63-96% boost in worst cases.

Benchmark results are available at https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

Thanks!

@arter97 arter97 mentioned this pull request May 30, 2023
@tiann
Copy link
Owner

tiann commented May 30, 2023

Is the struct list_head allow_list still useful? I noticed that it is not used at all in __ksu_is_allow_uid. We are concerned about this structure because the upcoming App Profile feature may store more information in struct perm_data, not just a bool and uid.

@arter97
Copy link
Contributor Author

arter97 commented May 30, 2023

Is the struct list_head allow_list still useful? I noticed that it is not used at all in __ksu_is_allow_uid.

Not really, but I left it that way to avoid changing other code like saving it in /data/adb.

We are concerned about this structure because the upcoming App Profile feature may store more information in struct perm_data, not just a bool and uid.

Right now, the bitmap and the array introduced in this commit is enough to replace the linked-list in ksu_is_allow_uid(), but we can re-introduce (partially revert) it if it needs more complex data structure in the future. The key point of this PR is to avoid as much overhead as possible for non-target processes, not necessarily replacing the entire linked-list.

So for example, if we decided to add more members to the linked-list structure, we can leave the new PR code intact and just use it to determine whether to check the linked-list or not. This will incur a small overhead in superuser apps, but leave the non-target processes' overhead to minimal.

@tiann
Copy link
Owner

tiann commented Jun 3, 2023

Sorry for the late reply!

These days I am developing the "App Profile" feature, which requires storing more information inside the perm_data structure and enabling each root process to access this information. This may potentially compromise the current optimization, and I am striving to stabilize this behavior as much as possible. Rest assured that I will update you here once this feature is completed.

@tiann
Copy link
Owner

tiann commented Jun 13, 2023

Hi, @arter97 "App Profile" has been released for some time now and the feedback has been good; I believe the core data structure will remain stable for some time, and if you wish, you are welcome to port your changes to the new version!

@arter97
Copy link
Contributor Author

arter97 commented Jun 13, 2023

Good to hear.

I'll look over the changes and update the PR soon.

Thanks!

arter97 added 9 commits June 14, 2023 17:16
Follow the Linux kernel's coding style and add newline at end of files if
it doesn't exist.

Signed-off-by: Juhyung Park <[email protected]>
Those are called from hot system-call handlers.

Signed-off-by: Juhyung Park <[email protected]>
Those whitespaces must be left intact for the patches to be applied
correctly.

Signed-off-by: Juhyung Park <[email protected]>
…pat()

memcmp()'ing 15 bytes is cheaper than ksu_is_allow_uid().

Reorder ksu_is_allow_uid() below memcmp().

Signed-off-by: Juhyung Park <[email protected]>
Avoid going through an additional function (which causes a measurable
overhead) and use *_hook booleans directly to selectively allow ksu hooks.

The existing `if (!*_hook) return 0;` code is left intact to preserve
compatibilities with kernels that have older patches applied.

Signed-off-by: Juhyung Park <[email protected]>
These functions are called extremely frequently.

Add branch hints so that regular processes that is not a target for
KernelSU runs with minimal overhead.

Signed-off-by: Juhyung Park <[email protected]>
Processes with uid < 2000 are typically system daemons.

Check them early to avoid going through the expensive list traversal.

Signed-off-by: Juhyung Park <[email protected]>
…_uid()

ksu_is_allow_uid() is called extremely frequently, and making it run as
fast as possible is crucial.

Assuming a 4K page size, using a page-sized bitmap allows us to see if
processes with uid lower than 32767 (which covers virtually all Android
apps) in O(1).

A quick benchmark shows that this reduces the time spent in
ksu_is_allow_uid() by 4.8 times in average (99%), 12.3 times (99.99%) and
26.0 times (99.999%) in the worst case. Most notably, this reduces the
99.999% result previously from worrying milliseconds scale to microseconds
scale.

For benchmark details, visit https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

Signed-off-by: Juhyung Park <[email protected]>
ksu_is_allow_uid() is called extremely frequently, and making it run as
fast as possible is crucial.

Previous commit improved the most cases in Android (uid <= 32767), but it
isn't enough to get rid of the expensive linked list traversal in
ksu_is_allow_uid() entirely.

Introduce a page-sized sequential array to cache uid > BITMAP_UID_MAX
entries.

With conjunction of the previous commit, ksu_is_allow_uid() will now
lookup the bitmap first in O(1), and then in the extremely unlikely case,
search the allow_list_arr[] sequentially in O(N), where N is equals to
superuser-granted apps with UID higher than 32767. The linked list
traversal can be avoided entirely.

Signed-off-by: Juhyung Park <[email protected]>
@arter97
Copy link
Contributor Author

arter97 commented Jun 14, 2023

Hi @tiann, the new app profile feature doesn't seem to conflict much with the changes I've made.

I've updated the PR to cleanly merge on the latest main. I don't expect any behavior differences.

Thanks.

@tiann
Copy link
Owner

tiann commented Jun 16, 2023

Thank you, it works great!

@tiann tiann merged commit bd8434f into tiann:main Jun 16, 2023
EmanuelCN pushed a commit to EmanuelCN/KernelSU that referenced this pull request Jun 17, 2023
Hi @tiann.

Thanks for the great project, I had great fun playing around with it.

This PR mainly tries to further minimize the possible delays caused by
KernelSU hooking.

There are 3 major changes:
- Processes with 0 < UID < 2000 are blocked straight-up before going
through the allow_list.
I don't see any need for such processes to be interested in root, and
this allows returning early before going through a more expensive
lookup.
If there's an expected breakage due to this change, I'll remove it. Let
me know.
- A page-sized (4K) bitmap is added.
This allows O(1) lookup for UID <= 32767.
This speeds up `ksu_is_allow_uid()` by about 4.8x by sacrificing a 4K
memory. IMHO, a good trade-off.
Most notably, this reduces the 99.999% result previously from worrying
milliseconds scale to microseconds scale.
For UID > 32767, another page-sized (4K) sequential array is used to
cache allow_list.

Compared to the previous PR tiann#557, this new approach gives another nice
25% performance boost in average, 63-96% boost in worst cases.

Benchmark results are available at
https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

Thanks!

---------

Signed-off-by: Juhyung Park <[email protected]>
ImSpiDy added a commit to ImSpiDy/KernelSU that referenced this pull request Jun 26, 2023
commit cd5bc2e
Author: Zillion <[email protected]>
Date:   Mon Jun 26 04:45:24 2023 +0200

    Add Spanish Translation (tiann#689)

commit 477361f
Author: Pegioner <[email protected]>
Date:   Sat Jun 24 15:17:51 2023 +0300

    Update Russian translation (tiann#681)

commit d3632e4
Author: Gustavo Mendes <[email protected]>
Date:   Sat Jun 24 09:17:15 2023 -0300

    Update Portuguese brazilian translation (tiann#682)

    Signed-off-by: Gustavo Mendes <[email protected]>

commit 0c2f901
Author: SoDebug <[email protected]>
Date:   Sat Jun 24 20:16:25 2023 +0800

    repos.json: Update the link of the KernelSU kernel release repo of the device I maintain (tiann#686)

    Update the link of the KernelSU kernel release repo of the device I
    maintain

commit 09d90e1
Author: Howard Wu <[email protected]>
Date:   Fri Jun 23 17:48:18 2023 +0800

    ci: update gki version (tiann#679)

    Fix the version name of android13-5.15.74
    Add android12-5.10.117

commit 4fe167c
Author: Trịnh Văn Lợi <[email protected]>
Date:   Fri Jun 23 16:30:04 2023 +0700

    Update Vietnamese strings (tiann#678)

commit 58ffaeb
Author: raystef66 <[email protected]>
Date:   Fri Jun 23 03:31:58 2023 +0200

    Update Flemish/Dutch translation (tiann#677)

commit 76499ee
Author: Ali Beyaz <[email protected]>
Date:   Fri Jun 23 04:31:23 2023 +0300

    Translated latest strings to Turkish (tiann#676)

commit fedfa3e
Author: weishu <[email protected]>
Date:   Fri Jun 23 00:35:25 2023 +0800

    manager: update card color

commit 2902e42
Author: Igor Sorocean <[email protected]>
Date:   Thu Jun 22 19:32:26 2023 +0300

    manager: update ro translation (tiann#674)

commit 37f4045
Author: weishu <[email protected]>
Date:   Fri Jun 23 00:31:36 2023 +0800

    manager: add a simple manager updater, close tiann#627

commit 12761ee
Author: weishu <[email protected]>
Date:   Thu Jun 22 23:24:35 2023 +0800

    manager: don't remember state when process died.

commit 0d25423
Author: weishu <[email protected]>
Date:   Thu Jun 22 23:20:13 2023 +0800

    manager: fix module install

commit f5bb246
Author: weishu <[email protected]>
Date:   Thu Jun 22 19:46:26 2023 +0800

    manager: fix download state

commit 303a3a8
Author: weishu <[email protected]>
Date:   Thu Jun 22 19:29:37 2023 +0800

    manager: fix update button

commit 07273b6
Author: weishu <[email protected]>
Date:   Thu Jun 22 18:40:28 2023 +0800

    manager: support module update online

commit c7c9e9c
Author: weishu <[email protected]>
Date:   Thu Jun 22 17:00:02 2023 +0800

    ksud: respect the skip_mount flag of module

commit c3c990c
Author: weishu <[email protected]>
Date:   Thu Jun 22 16:48:13 2023 +0800

    ksud: increase reserved size to 256M

commit 6942fe1
Author: weishu <[email protected]>
Date:   Thu Jun 22 16:46:29 2023 +0800

    manager: set keyboard options for inputtext

commit f5cfb32
Author: weishu <[email protected]>
Date:   Thu Jun 22 15:17:32 2023 +0800

    kernel: fix incorrect umount for apps

commit e17f3ea
Author: weishu <[email protected]>
Date:   Thu Jun 22 14:37:17 2023 +0800

    Revert "kernel: use vfs_fstatat on kernel 5.10+, vfs_statx may have cfi."

    This reverts commit cd3e292.

commit 08884da
Author: weishu <[email protected]>
Date:   Thu Jun 22 13:42:28 2023 +0800

    kernel: don't alloc groups for default groups

commit 5f1d70d
Author: weishu <[email protected]>
Date:   Thu Jun 22 12:54:30 2023 +0800

    Revert "kernel: getname might sleep in kprobe handler  (tiann#670)"

    This reverts commit 79bb981.

commit 79bb981
Author: weishu <[email protected]>
Date:   Thu Jun 22 10:54:50 2023 +0800

    kernel: getname might sleep in kprobe handler  (tiann#670)

commit 1cda4ba
Author: Ali Beyaz <[email protected]>
Date:   Tue Jun 20 13:45:24 2023 +0300

    Update latest strings to Turkish (tiann#662)

commit 1cc678d
Author: raystef66 <[email protected]>
Date:   Tue Jun 20 12:44:04 2023 +0200

    Update Flemish/Dutch translation (tiann#665)

commit cd3e292
Author: weishu <[email protected]>
Date:   Tue Jun 20 18:42:22 2023 +0800

    kernel: use vfs_fstatat on kernel 5.10+, vfs_statx may have cfi.

commit 40ea27a
Author: Howard Wu <[email protected]>
Date:   Tue Jun 20 18:10:07 2023 +0800

    ci: Fix kernel version (tiann#666)

commit e95ca93
Author: Azeroth <[email protected]>
Date:   Tue Jun 20 13:28:05 2023 +0330

    Fix typo (tiann#667)

    Fixed typo in this section
    https://kernelsu.org/guide/installation.html#patch-boot-img-manully

commit 9b2f907
Author: Ikko Eltociear Ashimine <[email protected]>
Date:   Tue Jun 20 12:03:09 2023 +0900

    kernel: fix typo in allowlist.c (tiann#663)

    creat -> create

commit 90299ad
Author: Coconut <[email protected]>
Date:   Tue Jun 20 10:35:07 2023 +0800

    kernel:Fix the issue of incompatible __maybe_unused in the GCC compiler kernel used in versions 4.4. x to 4.9. x. (tiann#660)

commit 22d084f
Author: weishu <[email protected]>
Date:   Mon Jun 19 22:16:46 2023 +0800

    manager: Add selinux rules UI

commit 99770a7
Author: Muhammad Fadlyas <[email protected]>
Date:   Mon Jun 19 19:07:47 2023 +0700

    Update Indonesian translation (tiann#659)

commit bbc7ebe
Author: weishu <[email protected]>
Date:   Mon Jun 19 17:57:15 2023 +0800

    kernel: Enforcement of Manager Signature Verification

commit d131b75
Author: exer <[email protected]>
Date:   Sun Jun 18 17:06:41 2023 +0800

    [add device]: Sony Tama (XZ2/c/p, XZ3) (tiann#656)

commit ff8c614
Author: weishu <[email protected]>
Date:   Sun Jun 18 13:00:24 2023 +0800

    kernel: allow uid 1000(system_uid) to grant root. close tiann#645

commit c12ad9d
Author: weishu <[email protected]>
Date:   Sun Jun 18 12:51:27 2023 +0800

    kernel: fix compile err. close tiann#647

commit 1703c16
Author: weishu <[email protected]>
Date:   Sun Jun 18 12:47:29 2023 +0800

    ci: support deprecated kernel versions (tiann#648)

commit a48d7b1
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Sat Jun 17 23:11:41 2023 +0800

    [add device]: (tiann#650)

    has been added to the website.
    Related issue: tiann#644

    Co-authored-by: GitHub Actions <41898282+github-actions[bot]@users.noreply.github.com>

commit f2d5e57
Author: Ylarod <[email protected]>
Date:   Sat Jun 17 22:07:39 2023 +0800

    fix add-device (tiann#649)

commit 4111bbf
Author: Gustavo Mendes <[email protected]>
Date:   Sat Jun 17 10:14:36 2023 -0300

    Update Portuguese brazilian translation (tiann#643)

    Signed-off-by: Gustavo Mendes <[email protected]>

commit cd32ad8
Author: Ali Beyaz <[email protected]>
Date:   Sat Jun 17 16:14:09 2023 +0300

    Fixed some Turkish strings again (tiann#646)

commit fefb826
Author: Rem01Gaming <[email protected]>
Date:   Fri Jun 16 19:50:31 2023 +0700

    Update Indonesian translation (tiann#634)

commit e27fc04
Author: Ali Beyaz <[email protected]>
Date:   Fri Jun 16 15:50:13 2023 +0300

    Fixed some Turkish strings (tiann#640)

commit 168f412
Author: SupeChicken666 <[email protected]>
Date:   Fri Jun 16 20:49:51 2023 +0800

    Add CI workflow for ChromeOS ARCVM (tiann#641)

    All changes are tested on my fork (the Telegram error was fixed in
    9b16150):
    https://github.com/supechicken/KernelSU/actions/runs/5287864543

    ---------

    Co-authored-by: weishu <[email protected]>

commit bd8434f
Author: Juhyung Park <[email protected]>
Date:   Fri Jun 16 20:53:15 2023 +0900

    Hook improvements (take 2) (tiann#563)

    Hi @tiann.

    Thanks for the great project, I had great fun playing around with it.

    This PR mainly tries to further minimize the possible delays caused by
    KernelSU hooking.

    There are 3 major changes:
    - Processes with 0 < UID < 2000 are blocked straight-up before going
    through the allow_list.
    I don't see any need for such processes to be interested in root, and
    this allows returning early before going through a more expensive
    lookup.
    If there's an expected breakage due to this change, I'll remove it. Let
    me know.
    - A page-sized (4K) bitmap is added.
    This allows O(1) lookup for UID <= 32767.
    This speeds up `ksu_is_allow_uid()` by about 4.8x by sacrificing a 4K
    memory. IMHO, a good trade-off.
    Most notably, this reduces the 99.999% result previously from worrying
    milliseconds scale to microseconds scale.
    For UID > 32767, another page-sized (4K) sequential array is used to
    cache allow_list.

    Compared to the previous PR tiann#557, this new approach gives another nice
    25% performance boost in average, 63-96% boost in worst cases.

    Benchmark results are available at
    https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

    Thanks!

    ---------

    Signed-off-by: Juhyung Park <[email protected]>

commit c697398
Author: weishu <[email protected]>
Date:   Fri Jun 16 19:32:48 2023 +0800

    kernel: fix warning on x86_64, close tiann#637
itswill00 pushed a commit to itswill00/KernelSU-Next-4.14 that referenced this pull request Jan 19, 2025
Hi @tiann.

Thanks for the great project, I had great fun playing around with it.

This PR mainly tries to further minimize the possible delays caused by
KernelSU hooking.

There are 3 major changes:
- Processes with 0 < UID < 2000 are blocked straight-up before going
through the allow_list.
I don't see any need for such processes to be interested in root, and
this allows returning early before going through a more expensive
lookup.
If there's an expected breakage due to this change, I'll remove it. Let
me know.
- A page-sized (4K) bitmap is added.
This allows O(1) lookup for UID <= 32767.
This speeds up `ksu_is_allow_uid()` by about 4.8x by sacrificing a 4K
memory. IMHO, a good trade-off.
Most notably, this reduces the 99.999% result previously from worrying
milliseconds scale to microseconds scale.
For UID > 32767, another page-sized (4K) sequential array is used to
cache allow_list.

Compared to the previous PR tiann#557, this new approach gives another nice
25% performance boost in average, 63-96% boost in worst cases.

Benchmark results are available at
https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

Thanks!

---------

Signed-off-by: Juhyung Park <[email protected]>
chenzhu005774 pushed a commit to chenzhu005774/KernelSU that referenced this pull request Sep 12, 2025
Hi @tiann.

Thanks for the great project, I had great fun playing around with it.

This PR mainly tries to further minimize the possible delays caused by
KernelSU hooking.

There are 3 major changes:
- Processes with 0 < UID < 2000 are blocked straight-up before going
through the allow_list.
I don't see any need for such processes to be interested in root, and
this allows returning early before going through a more expensive
lookup.
If there's an expected breakage due to this change, I'll remove it. Let
me know.
- A page-sized (4K) bitmap is added.
This allows O(1) lookup for UID <= 32767.
This speeds up `ksu_is_allow_uid()` by about 4.8x by sacrificing a 4K
memory. IMHO, a good trade-off.
Most notably, this reduces the 99.999% result previously from worrying
milliseconds scale to microseconds scale.
For UID > 32767, another page-sized (4K) sequential array is used to
cache allow_list.

Compared to the previous PR tiann#557, this new approach gives another nice
25% performance boost in average, 63-96% boost in worst cases.

Benchmark results are available at
https://docs.google.com/spreadsheets/d/1w_tO1zRLPNMFRer49pL1TQfL6ndEhilRrDU1XFIcWXY/edit?usp=sharing

Thanks!

---------

Signed-off-by: Juhyung Park <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants