[PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().

If tomoyo is used in a testing/fuzzing environment in learning mode,
for lots of domains the quota will be exceeded and stay exceeded
for prolonged periods of time. In such cases it's pointless (and slow)
to walk the whole acl list again and again just to rediscover that
the quota is exceeded. We already have the TOMOYO_DIF_QUOTA_WARNED flag
that notes the overflow condition. Check it early to avoid the slowdown.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-security-module@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 security/tomoyo/util.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c
index 1da2e3722b126..af8cd2af3466d 100644
--- a/security/tomoyo/util.c
+++ b/security/tomoyo/util.c

@@ -1051,6 +1051,8 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r)
 		return false;
 	if (!domain)
 		return true;
+	if (READ_ONCE(domain->flags[TOMOYO_DIF_QUOTA_WARNED]))
+		return false;
 	list_for_each_entry_rcu(ptr, &domain->acl_info_list, list,
 				srcu_read_lock_held(&tomoyo_ss)) {
 		u16 perm;

@@ -1096,14 +1098,12 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r)
 	if (count < tomoyo_profile(domain->ns, domain->profile)->
 	    pref[TOMOYO_PREF_MAX_LEARNING_ENTRY])
 		return true;
-	if (!domain->flags[TOMOYO_DIF_QUOTA_WARNED]) {
-		domain->flags[TOMOYO_DIF_QUOTA_WARNED] = true;
-		/* r->granted = false; */
-		tomoyo_write_log(r, "%s", tomoyo_dif[TOMOYO_DIF_QUOTA_WARNED]);
+	WRITE_ONCE(domain->flags[TOMOYO_DIF_QUOTA_WARNED], true);
+	/* r->granted = false; */
+	tomoyo_write_log(r, "%s", tomoyo_dif[TOMOYO_DIF_QUOTA_WARNED]);
 #ifndef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
-		pr_warn("WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.\n",
-			domain->domainname->name);
+	pr_warn("WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.\n",
+		domain->domainname->name);
 #endif
-	}
 	return false;
 }

base-commit: 5472f14a37421d1bca3dddf33cabd3bd6dbefbbc
-- 
2.34.1.173.g76aa8bc2d0-goog

On 2021/12/14 18:45, Dmitry Vyukov wrote:

If tomoyo is used in a testing/fuzzing environment in learning mode,
for lots of domains the quota will be exceeded and stay exceeded
for prolonged periods of time. In such cases it's pointless (and slow)
to walk the whole acl list again and again just to rediscover that
the quota is exceeded. We already have the TOMOYO_DIF_QUOTA_WARNED flag
that notes the overflow condition. Check it early to avoid the slowdown.

Thank you.

This patch will make a slight but user visible change.

When tomoyo_profile(domain->ns, domain->profile)->pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] is
increased (or domain->profile switches to a different profile which has larger
pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value) after domain->flags[TOMOYO_DIF_QUOTA_WARNED] = true
is called, tomoyo_domain_quota_is_ok() will continue returning "false", and ACLs are no longer
appended.

Therefore, administrator will have to manually do domain->flags[TOMOYO_DIF_QUOTA_WARNED] = false
after increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile).

But since the message

  WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.

is a hint that tells administrator that "you will surely fail to try the enforcing mode on this
domain because the kernel has failed to automatically append at least one ACL to this domain",
administrator would have to retry the learning mode after increasing
pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) even without this patch.

Therefore, asking administrator to also clear domain->flags[TOMOYO_DIF_QUOTA_WARNED] after
increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) would be
tolerable...

On Tue, 14 Dec 2021 at 12:36, Tetsuo Handa
[off-list ref] wrote:

On 2021/12/14 18:45, Dmitry Vyukov wrote:

quoted

If tomoyo is used in a testing/fuzzing environment in learning mode,
for lots of domains the quota will be exceeded and stay exceeded
for prolonged periods of time. In such cases it's pointless (and slow)
to walk the whole acl list again and again just to rediscover that
the quota is exceeded. We already have the TOMOYO_DIF_QUOTA_WARNED flag
that notes the overflow condition. Check it early to avoid the slowdown.

Thank you.

This patch will make a slight but user visible change.

When tomoyo_profile(domain->ns, domain->profile)->pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] is
increased (or domain->profile switches to a different profile which has larger
pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value) after domain->flags[TOMOYO_DIF_QUOTA_WARNED] = true
is called, tomoyo_domain_quota_is_ok() will continue returning "false", and ACLs are no longer
appended.

Therefore, administrator will have to manually do domain->flags[TOMOYO_DIF_QUOTA_WARNED] = false
after increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile).

But since the message

  WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.

is a hint that tells administrator that "you will surely fail to try the enforcing mode on this
domain because the kernel has failed to automatically append at least one ACL to this domain",
administrator would have to retry the learning mode after increasing
pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) even without this patch.

Therefore, asking administrator to also clear domain->flags[TOMOYO_DIF_QUOTA_WARNED] after
increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) would be
tolerable...

Should we reset flags[TOMOYO_DIF_QUOTA_WARNED] on any writes that
change TOMOYO_PREF_MAX_LEARNING_ENTRY?

If I am increasing TOMOYO_PREF_MAX_LEARNING_ENTRY because I observed
the warning, it's useful for me to receive the warning again.

On 2021/12/14 20:42, Dmitry Vyukov wrote:

quoted

Therefore, asking administrator to also clear domain->flags[TOMOYO_DIF_QUOTA_WARNED] after
increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) would be
tolerable...

Should we reset flags[TOMOYO_DIF_QUOTA_WARNED] on any writes that
change TOMOYO_PREF_MAX_LEARNING_ENTRY?

If I am increasing TOMOYO_PREF_MAX_LEARNING_ENTRY because I observed
the warning, it's useful for me to receive the warning again.

I decided not to reset flags[TOMOYO_DIF_QUOTA_WARNED] automatically, and
applied your proposal as-is. Thank you.

https://osdn.net/projects/tomoyo/scm/git/tomoyo-test1/commits/04e57a2d952bbd34bc45744e72be3eecdc344294

On Wed, 15 Dec 2021 at 12:46, Tetsuo Handa
[off-list ref] wrote:

On 2021/12/14 20:42, Dmitry Vyukov wrote:

quoted

quoted

Therefore, asking administrator to also clear domain->flags[TOMOYO_DIF_QUOTA_WARNED] after
increasing pref[TOMOYO_PREF_MAX_LEARNING_ENTRY] value (or changing domain->profile) would be
tolerable...

Should we reset flags[TOMOYO_DIF_QUOTA_WARNED] on any writes that
change TOMOYO_PREF_MAX_LEARNING_ENTRY?

If I am increasing TOMOYO_PREF_MAX_LEARNING_ENTRY because I observed
the warning, it's useful for me to receive the warning again.

I decided not to reset flags[TOMOYO_DIF_QUOTA_WARNED] automatically, and
applied your proposal as-is. Thank you.

https://osdn.net/projects/tomoyo/scm/git/tomoyo-test1/commits/04e57a2d952bbd34bc45744e72be3eecdc344294

Thanks!

It will appear in v5.17, right?