ima: cleanup the match_token policy code
Subsystems (4)
Mimi Zohar
maintainerPATCH 1/1 ima: cleanup the match_token policy code
Start the policy_tokens and the associated enumeration from zero, simplifying the pt macro.
--- security/integrity/ima/ima_policy.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
security/integrity/ima/ima_policy.c +5 -5
@@ -665,9 +665,9 @@ void ima_update_policy(void)ima_update_policy_flag (); } +/* Keep the enumeration in sync with the policy_tokens! */ enum { - Opt_err =-1 , - Opt_measure =1 , Opt_dont_measure, + Opt_measure, Opt_dont_measure, Opt_appraise, Opt_dont_appraise, Opt_audit, Opt_hash, Opt_dont_hash, Opt_obj_user, Opt_obj_role, Opt_obj_type,
@@ -677,10 +677,10 @@ enum { Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt, Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, Opt_appraise_type, Opt_permit_directio, - Opt_pcr + Opt_pcr, Opt_err }; -static match_table_t policy_tokens = { +static const match_table_t policy_tokens = { {Opt_measure,"measure" }, {Opt_dont_measure,"dont_measure" }, {Opt_appraise, "appraise"},
@@ -1188,7 +1188,7 @@ void ima_policy_stop(struct seq_file *m, void *v) { } -#define pt (token) policy_tokens[token + Opt_err].pattern +#define pt (token) policy_tokens[token].pattern#define mt (token) mask_tokens[token]/*
Signature · 1 line
2.7.5
Quoted 3 lines · Mimi Zohar
Start the
policy_tokensand the associated enumeration from zero, simplifying the pt macro.
I applied this directly, since I decided to just commit my own "don't use negative Opt_err" patch for the test_and_set_bit() cases, and they kind of go together.
There's still a -1 in security/keys/encrypted-keys/encrypted.c, and there are also three cases of "Opt_error = -1" in the security layer.
All of which look pointless and wrong, but not actively buggy, so I'll leave them alone.
Linus
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Mimi Zohar
Start the
policy_tokensand the associated enumeration from zero, simplifying the pt macro.I applied this directly, since I decided to just commit my own "don't use negative
Opt_err" patch for thetest_and_set_bit()cases, and they kind of go together.
Quoted 5 lines
There's still a -1 in
security/keys/encrypted-keys/encrypted.c, and there are also three cases of "Opt_error= -1" in the security layer.All of which look pointless and wrong, but not actively buggy, so I'll leave them alone.
FWIW, that part of LSM shite is getting taken out and shot - LSM-related preps in the beginning of mount API series are, at the moment, at
fs/btrfs/ctree.h | 4 - fs/btrfs/super.c | 82 ++------- fs/namespace.c | 9 +- fs/nfs/internal.h | 2 +- fs/nfs/super.c | 34 ++-- fs/super.c | 23 +-- include/linux/lsm_hooks.h | 17 +- include/linux/security.h | 82 ++------- security/security.c | 39 +++-- security/selinux/hooks.c | 798 ++++++++++++++++++++++++++++++++------------------------------------------------------ security/smack/smack_lsm.c | 359 +++++++++++++++------------------------ 11 files changed, 523 insertions(+), 926 deletions(-)
and unlike the original variant there's not much touched in security/* further in the series...
No match_token() uses left in there after this part.
Quoted 30 lines · Al Viro
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Mimi Zohar
Start the
policy_tokensand the associated enumeration from zero, simplifying the pt macro.I applied this directly, since I decided to just commit my own "don't use negative
Opt_err" patch for thetest_and_set_bit()cases, and they kind of go together.Quoted 5 lines
There's still a -1 in
security/keys/encrypted-keys/encrypted.c, and there are also three cases of "Opt_error= -1" in the security layer.All of which look pointless and wrong, but not actively buggy, so I'll leave them alone.
FWIW, that part of LSM shite is getting taken out and shot - LSM-related preps in the beginning of mount API series are, at the moment, at
fs/btrfs/ctree.h | 4 - fs/btrfs/super.c | 82 ++------- fs/namespace.c | 9 +- fs/nfs/internal.h | 2 +- fs/nfs/super.c | 34 ++-- fs/super.c | 23 +-- include/linux/lsm_hooks.h | 17 +- include/linux/security.h | 82 ++------- security/security.c | 39 +++-- security/selinux/hooks.c | 798 ++++++++++++++++++++++++++++++++------------------------------------------------------ security/smack/smack_lsm.c | 359 +++++++++++++++------------------------ 11 files changed, 523 insertions(+), 926 deletions(-)
Quoted 4 lines
and unlike the original variant there's not much touched in
security/*further in the series...No
match_token()uses left in there after this part.
In LSM, that is...
Mimi Zohar
maintainerQuoted 30 lines · Al Viro
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Mimi Zohar
Start the
policy_tokensand the associated enumeration from zero, simplifying the pt macro.I applied this directly, since I decided to just commit my own "don't use negative
Opt_err" patch for thetest_and_set_bit()cases, and they kind of go together.Quoted 5 lines
There's still a -1 in
security/keys/encrypted-keys/encrypted.c, and there are also three cases of "Opt_error= -1" in the security layer.All of which look pointless and wrong, but not actively buggy, so I'll leave them alone.
FWIW, that part of LSM shite is getting taken out and shot - LSM-related preps in the beginning of mount API series are, at the moment, at
fs/btrfs/ctree.h | 4 - fs/btrfs/super.c | 82 ++------- fs/namespace.c | 9 +- fs/nfs/internal.h | 2 +- fs/nfs/super.c | 34 ++-- fs/super.c | 23 +-- include/linux/lsm_hooks.h | 17 +- include/linux/security.h | 82 ++------- security/security.c | 39 +++-- security/selinux/hooks.c | 798 ++++++++++++++++++++++++++++++++------------------------------------------------------ security/smack/smack_lsm.c | 359 +++++++++++++++------------------------ 11 files changed, 523 insertions(+), 926 deletions(-)
Quoted 4 lines
and unlike the original variant there's not much touched in
security/*further in the series...No
match_token()uses left in there after this part.
Could you expand on commit 5b2ea6199614 ("selinux: switch away from match_token()") patch description. All that it says is "It's not a good fit, unfortunately, and the next step will make it even less so. Open-code what we need here." And there's even less for the equivalent Smack patch, which just says "same issue as with selinux...".
Mimi
Quoted 6 lines · Mimi Zohar
Could you expand on commit
5b2ea6199614("selinux: switch away frommatch_token()") patch description. All that it says is "It's not a good fit, unfortunately, and the next step will make it even less so. Open-code what we need here." And there's even less for the equivalent Smack patch, which just says "same issue as with selinux...".
match_token() would require messing around with strsep() or something equivalent. It's not a regex; foo=%s has no idea that comma is in any way special, etc.
As for the next commit... Killing the Cthulhu-awful mess in selinux_sb_eat_lsm_opts() (allocating two temproraries, concatenating (comma-separated) non-LSM options into one, concatenating (pipe-separated) dequoted LSM options into another, then splitting that another by '|' instances and figuring out which option each piece is, etc.) is a Good Thing(tm). And having to dance around the needs of match_token() adds extra headache, for no good reason.
Mimi Zohar
maintainerQuoted 13 lines · Al Viro
Quoted 6 lines · Mimi Zohar
Could you expand on commit
5b2ea6199614("selinux: switch away frommatch_token()") patch description. All that it says is "It's not a good fit, unfortunately, and the next step will make it even less so. Open-code what we need here." And there's even less for the equivalent Smack patch, which just says "same issue as with selinux...".
match_token()would require messing around withstrsep()or something equivalent. It's not a regex; foo=%s has no idea that comma is in any way special, etc.
Quoted 7 lines
As for the next commit... Killing the Cthulhu-awful mess in
selinux_sb_eat_lsm_opts()(allocating two temproraries, concatenating (comma-separated) non-LSM options into one, concatenating (pipe-separated) dequoted LSM options into another, then splitting that another by '|' instances and figuring out which option each piece is, etc.) is a Good Thing(tm). And having to dance around the needs ofmatch_token()adds extra headache, for no good reason.
Ok, so it is this particular combination of things, not the general usage of strsep or match_token that you're objecting to. So fixing the other match_token non-LSM instances is fine.
To prevent the enumeration and the match_table from going out of sync, I was thinking about defining a macro to create the match_table_t:
#define __policy_tokens_match(ENUM, str) {Opt_ ## ENUM, #str},
static const match_table_t policy_tokens = {
__policy_tokens_id(__policy_tokens_match)
};
and the enumeration:
enum policy_tokens_id { __policy_tokens_id(__policy_tokens_enumify) };
Mimi