* [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().
@ 2021-12-14 9:45 Dmitry Vyukov
2021-12-14 11:36 ` Tetsuo Handa
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Vyukov @ 2021-12-14 9:45 UTC (permalink / raw)
To: takedakn, penguin-kernel, jmorris, serge
Cc: nogikh, Dmitry Vyukov, linux-security-module, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().
2021-12-14 9:45 [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok() Dmitry Vyukov
@ 2021-12-14 11:36 ` Tetsuo Handa
2021-12-14 11:42 ` Dmitry Vyukov
0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2021-12-14 11:36 UTC (permalink / raw)
To: Dmitry Vyukov, takedakn, jmorris, serge
Cc: nogikh, linux-security-module, linux-kernel
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...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().
2021-12-14 11:36 ` Tetsuo Handa
@ 2021-12-14 11:42 ` Dmitry Vyukov
2021-12-15 11:46 ` Tetsuo Handa
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Vyukov @ 2021-12-14 11:42 UTC (permalink / raw)
To: Tetsuo Handa
Cc: takedakn, jmorris, serge, nogikh, linux-security-module,
linux-kernel
On Tue, 14 Dec 2021 at 12:36, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> 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...
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().
2021-12-14 11:42 ` Dmitry Vyukov
@ 2021-12-15 11:46 ` Tetsuo Handa
2021-12-15 13:17 ` Dmitry Vyukov
0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2021-12-15 11:46 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: takedakn, jmorris, serge, nogikh, linux-security-module,
linux-kernel
On 2021/12/14 20:42, Dmitry Vyukov wrote:
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok().
2021-12-15 11:46 ` Tetsuo Handa
@ 2021-12-15 13:17 ` Dmitry Vyukov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Vyukov @ 2021-12-15 13:17 UTC (permalink / raw)
To: Tetsuo Handa
Cc: takedakn, jmorris, serge, nogikh, linux-security-module,
linux-kernel
On Wed, 15 Dec 2021 at 12:46, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2021/12/14 20:42, Dmitry Vyukov wrote:
> >> 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?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-15 13:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-14 9:45 [PATCH] tomoyo: Check exceeded quota early in tomoyo_domain_quota_is_ok() Dmitry Vyukov
2021-12-14 11:36 ` Tetsuo Handa
2021-12-14 11:42 ` Dmitry Vyukov
2021-12-15 11:46 ` Tetsuo Handa
2021-12-15 13:17 ` Dmitry Vyukov
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).