Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Mon, 17 Dec 2018 11:39:57 -0800 | Subject | Re: [PATCH RESEND] KEYS: fix parsing invalid pkey info string |
| |
On Mon, Dec 17, 2018 at 11:06 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Honestly, for being about "security", all of this code seems to be > doing some really questionable things with all those Opt_xyz enums.
Yeah, at least security/keys/trusted.c ends up mixing that enum and just using "int" completely randomly, and you have datablob_parse() returning either a negative integer _or_ an "Opt_xyz" value, so having Opt_err be -1 is doubly confusing there (it would also be "-EPERM" depending on how you treat it).
There doesn't seem to be any _actual_ confusion (because Opt_err is always turned into an actual real error code), but it's just another sign of "those enums should not be negative".
So on the whole, I think that the "Opt_err = -1" is a serious mistake, but at least for now, ima_policy.c clearly has (bogus) code that relies on it.
But the two cases that use "test_and_set_bit()" do not seem to have any reason to use that -1 enum, so while we can't do the "just remove -1" in general, I do think the proper fix is to just do it for those two files.
Eric, mind testing a patch like that? Untested patch attached just for completeness..
Linus security/keys/keyctl_pkey.c | 2 +- security/keys/trusted.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyctl_pkey.c b/security/keys/keyctl_pkey.c index 783978842f13..70e65a2ff207 100644 --- a/security/keys/keyctl_pkey.c +++ b/security/keys/keyctl_pkey.c @@ -25,7 +25,7 @@ static void keyctl_pkey_params_free(struct kernel_pkey_params *params) } enum { - Opt_err = -1, + Opt_err, Opt_enc, /* "enc=<encoding>" eg. "enc=oaep" */ Opt_hash, /* "hash=<digest-name>" eg. "hash=sha1" */ }; diff --git a/security/keys/trusted.c b/security/keys/trusted.c index ff6789365a12..697bfc6c8192 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -711,7 +711,7 @@ static int key_unseal(struct trusted_key_payload *p, } enum { - Opt_err = -1, + Opt_err, Opt_new, Opt_load, Opt_update, Opt_keyhandle, Opt_keyauth, Opt_blobauth, Opt_pcrinfo, Opt_pcrlock, Opt_migratable, | |