diff for duplicates of <20171016183141.GD121701@gmail.com> diff --git a/a/1.txt b/N1/1.txt index 0e94a9e..ad4cccc 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -5,7 +5,8 @@ On Fri, Oct 13, 2017 at 04:39:28PM +0100, David Howells wrote: > +++ b/security/keys/gc.c > @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys) > while (!list_empty(keys)) { -> struct key *key > list_entry(keys->next, struct key, graveyard_link); +> struct key *key = +> list_entry(keys->next, struct key, graveyard_link); > + short state = key->state; > + > list_del(&key->graveyard_link); @@ -16,7 +17,7 @@ On Fri, Oct 13, 2017 at 04:39:28PM +0100, David Howells wrote: > /* Throw away the key data if the key is instantiated */ > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && > - !test_bit(KEY_FLAG_NEGATIVE, &key->flags) && -> + if (state = KEY_IS_INSTANTIATED && +> + if (state == KEY_IS_INSTANTIATED && > key->type->destroy) > key->type->destroy(key); @@ -28,12 +29,13 @@ Nit: put the two checks on the same line. > > atomic_dec(&key->user->nkeys); > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) -> + if (state = KEY_IS_INSTANTIATED) +> + if (state == KEY_IS_INSTANTIATED) > atomic_dec(&key->user->nikeys); This changes the behavior. Previously ->nikeys counted both negatively and positively instantiated keys, while this change implies that it now will only -count positively instantiated keys. I think you meant 'state !KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or +count positively instantiated keys. I think you meant 'state != +KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion. > @@ -901,7 +900,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) @@ -41,7 +43,7 @@ KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion. > atomic_inc(&newowner->nkeys); > > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) { -> + if (key->state = KEY_IS_INSTANTIATED) { +> + if (key->state == KEY_IS_INSTANTIATED) { > atomic_dec(&key->user->nikeys); > atomic_inc(&newowner->nikeys); > } @@ -73,7 +75,7 @@ Now it isn't. Shouldn't it test 'key->state != KEY_IS_UNINSTANTIATED'? > seq_printf(m, "%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s ", > key->serial, > - showflag(key, 'I', KEY_FLAG_INSTANTIATED), -> + state = KEY_IS_INSTANTIATED ? 'I' : '-', +> + state == KEY_IS_INSTANTIATED ? 'I' : '-', Similar problem. Previously 'I' was shown for negatively instantiated keys; now it's not. Shouldn't it test 'state != KEY_IS_UNINSTANTIATED'? @@ -96,3 +98,7 @@ through whereas now it only allows positively instantiated keys. Is that intentional? Eric +-- +To unsubscribe from this list: send the line "unsubscribe linux-security-module" in +the body of a message to majordomo at vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N1/content_digest index 294e6de..70363ae 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,9 +1,9 @@ "ref\020171012185612.GA11577@gmail.com\0" "ref\0150782504738.1655.12882942775980793687.stgit@warthog.procyon.org.uk\0" "ref\02176.1507909168@warthog.procyon.org.uk\0" - "From\0Eric Biggers <ebiggers3@gmail.com>\0" - "Subject\0Re: [RFC][PATCH 00/15] KEYS: Fixes\0" - "Date\0Mon, 16 Oct 2017 18:31:41 +0000\0" + "From\0ebiggers3@gmail.com (Eric Biggers)\0" + "Subject\0[RFC][PATCH 00/15] KEYS: Fixes\0" + "Date\0Mon, 16 Oct 2017 11:31:41 -0700\0" "To\0linux-security-module@vger.kernel.org\0" "\00:1\0" "b\0" @@ -14,7 +14,8 @@ "> +++ b/security/keys/gc.c\n" "> @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys)\n" "> \twhile (!list_empty(keys)) {\n" - "> \t\tstruct key *key > \t\t\tlist_entry(keys->next, struct key, graveyard_link);\n" + "> \t\tstruct key *key =\n" + "> \t\t\tlist_entry(keys->next, struct key, graveyard_link);\n" "> +\t\tshort state = key->state;\n" "> +\n" "> \t\tlist_del(&key->graveyard_link);\n" @@ -25,7 +26,7 @@ "> \t\t/* Throw away the key data if the key is instantiated */\n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&\n" "> -\t\t !test_bit(KEY_FLAG_NEGATIVE, &key->flags) &&\n" - "> +\t\tif (state = KEY_IS_INSTANTIATED &&\n" + "> +\t\tif (state == KEY_IS_INSTANTIATED &&\n" "> \t\t key->type->destroy)\n" "> \t\t\tkey->type->destroy(key);\n" "\n" @@ -37,12 +38,13 @@ "> \n" "> \t\tatomic_dec(&key->user->nkeys);\n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))\n" - "> +\t\tif (state = KEY_IS_INSTANTIATED)\n" + "> +\t\tif (state == KEY_IS_INSTANTIATED)\n" "> \t\t\tatomic_dec(&key->user->nikeys);\n" "\n" "This changes the behavior. Previously ->nikeys counted both negatively and\n" "positively instantiated keys, while this change implies that it now will only\n" - "count positively instantiated keys. I think you meant 'state !KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or\n" + "count positively instantiated keys. I think you meant 'state !=\n" + "KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or\n" "KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion.\n" "\n" "> @@ -901,7 +900,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)\n" @@ -50,7 +52,7 @@ "> \t\tatomic_inc(&newowner->nkeys);\n" "> \n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {\n" - "> +\t\tif (key->state = KEY_IS_INSTANTIATED) {\n" + "> +\t\tif (key->state == KEY_IS_INSTANTIATED) {\n" "> \t\t\tatomic_dec(&key->user->nikeys);\n" "> \t\t\tatomic_inc(&newowner->nikeys);\n" "> \t\t}\n" @@ -82,7 +84,7 @@ "> \tseq_printf(m, \"%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s \",\n" "> \t\t key->serial,\n" "> -\t\t showflag(key, 'I', KEY_FLAG_INSTANTIATED),\n" - "> +\t\t state = KEY_IS_INSTANTIATED ? 'I' : '-',\n" + "> +\t\t state == KEY_IS_INSTANTIATED ? 'I' : '-',\n" "\n" "Similar problem. Previously 'I' was shown for negatively instantiated keys; now\n" "it's not. Shouldn't it test 'state != KEY_IS_UNINSTANTIATED'?\n" @@ -104,6 +106,10 @@ "through whereas now it only allows positively instantiated keys. Is that\n" "intentional?\n" "\n" - Eric + "Eric\n" + "--\n" + "To unsubscribe from this list: send the line \"unsubscribe linux-security-module\" in\n" + "the body of a message to majordomo at vger.kernel.org\n" + More majordomo info at http://vger.kernel.org/majordomo-info.html -606743ea7f8960b7c46eb26dcfda77ea925be685ef9a31505ccddf6542815f98 +80a72205803959e84cac22a5ed554b86c11a15396221b4ad19e23747bb967e05
diff --git a/a/1.txt b/N2/1.txt index 0e94a9e..426e362 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -5,7 +5,8 @@ On Fri, Oct 13, 2017 at 04:39:28PM +0100, David Howells wrote: > +++ b/security/keys/gc.c > @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys) > while (!list_empty(keys)) { -> struct key *key > list_entry(keys->next, struct key, graveyard_link); +> struct key *key = +> list_entry(keys->next, struct key, graveyard_link); > + short state = key->state; > + > list_del(&key->graveyard_link); @@ -16,7 +17,7 @@ On Fri, Oct 13, 2017 at 04:39:28PM +0100, David Howells wrote: > /* Throw away the key data if the key is instantiated */ > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && > - !test_bit(KEY_FLAG_NEGATIVE, &key->flags) && -> + if (state = KEY_IS_INSTANTIATED && +> + if (state == KEY_IS_INSTANTIATED && > key->type->destroy) > key->type->destroy(key); @@ -28,12 +29,13 @@ Nit: put the two checks on the same line. > > atomic_dec(&key->user->nkeys); > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) -> + if (state = KEY_IS_INSTANTIATED) +> + if (state == KEY_IS_INSTANTIATED) > atomic_dec(&key->user->nikeys); This changes the behavior. Previously ->nikeys counted both negatively and positively instantiated keys, while this change implies that it now will only -count positively instantiated keys. I think you meant 'state !KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or +count positively instantiated keys. I think you meant 'state != +KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion. > @@ -901,7 +900,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) @@ -41,7 +43,7 @@ KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion. > atomic_inc(&newowner->nkeys); > > - if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) { -> + if (key->state = KEY_IS_INSTANTIATED) { +> + if (key->state == KEY_IS_INSTANTIATED) { > atomic_dec(&key->user->nikeys); > atomic_inc(&newowner->nikeys); > } @@ -73,7 +75,7 @@ Now it isn't. Shouldn't it test 'key->state != KEY_IS_UNINSTANTIATED'? > seq_printf(m, "%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s ", > key->serial, > - showflag(key, 'I', KEY_FLAG_INSTANTIATED), -> + state = KEY_IS_INSTANTIATED ? 'I' : '-', +> + state == KEY_IS_INSTANTIATED ? 'I' : '-', Similar problem. Previously 'I' was shown for negatively instantiated keys; now it's not. Shouldn't it test 'state != KEY_IS_UNINSTANTIATED'? diff --git a/a/content_digest b/N2/content_digest index 294e6de..207a251 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -3,8 +3,12 @@ "ref\02176.1507909168@warthog.procyon.org.uk\0" "From\0Eric Biggers <ebiggers3@gmail.com>\0" "Subject\0Re: [RFC][PATCH 00/15] KEYS: Fixes\0" - "Date\0Mon, 16 Oct 2017 18:31:41 +0000\0" - "To\0linux-security-module@vger.kernel.org\0" + "Date\0Mon, 16 Oct 2017 11:31:41 -0700\0" + "To\0David Howells <dhowells@redhat.com>\0" + "Cc\0ebiggers@google.com" + linux-security-module@vger.kernel.org + keyrings@vger.kernel.org + " linux-kernel@vger.kernel.org\0" "\00:1\0" "b\0" "On Fri, Oct 13, 2017 at 04:39:28PM +0100, David Howells wrote:\n" @@ -14,7 +18,8 @@ "> +++ b/security/keys/gc.c\n" "> @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys)\n" "> \twhile (!list_empty(keys)) {\n" - "> \t\tstruct key *key > \t\t\tlist_entry(keys->next, struct key, graveyard_link);\n" + "> \t\tstruct key *key =\n" + "> \t\t\tlist_entry(keys->next, struct key, graveyard_link);\n" "> +\t\tshort state = key->state;\n" "> +\n" "> \t\tlist_del(&key->graveyard_link);\n" @@ -25,7 +30,7 @@ "> \t\t/* Throw away the key data if the key is instantiated */\n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&\n" "> -\t\t !test_bit(KEY_FLAG_NEGATIVE, &key->flags) &&\n" - "> +\t\tif (state = KEY_IS_INSTANTIATED &&\n" + "> +\t\tif (state == KEY_IS_INSTANTIATED &&\n" "> \t\t key->type->destroy)\n" "> \t\t\tkey->type->destroy(key);\n" "\n" @@ -37,12 +42,13 @@ "> \n" "> \t\tatomic_dec(&key->user->nkeys);\n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))\n" - "> +\t\tif (state = KEY_IS_INSTANTIATED)\n" + "> +\t\tif (state == KEY_IS_INSTANTIATED)\n" "> \t\t\tatomic_dec(&key->user->nikeys);\n" "\n" "This changes the behavior. Previously ->nikeys counted both negatively and\n" "positively instantiated keys, while this change implies that it now will only\n" - "count positively instantiated keys. I think you meant 'state !KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or\n" + "count positively instantiated keys. I think you meant 'state !=\n" + "KEY_IS_UNINSTANTIATED'? Renaming KEY_IS_INSTANTIATED to KEY_IS_POSITIVE or\n" "KEY_IS_POSITIVELY_INSTANTIATED also might help reduce this confusion.\n" "\n" "> @@ -901,7 +900,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)\n" @@ -50,7 +56,7 @@ "> \t\tatomic_inc(&newowner->nkeys);\n" "> \n" "> -\t\tif (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {\n" - "> +\t\tif (key->state = KEY_IS_INSTANTIATED) {\n" + "> +\t\tif (key->state == KEY_IS_INSTANTIATED) {\n" "> \t\t\tatomic_dec(&key->user->nikeys);\n" "> \t\t\tatomic_inc(&newowner->nikeys);\n" "> \t\t}\n" @@ -82,7 +88,7 @@ "> \tseq_printf(m, \"%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s \",\n" "> \t\t key->serial,\n" "> -\t\t showflag(key, 'I', KEY_FLAG_INSTANTIATED),\n" - "> +\t\t state = KEY_IS_INSTANTIATED ? 'I' : '-',\n" + "> +\t\t state == KEY_IS_INSTANTIATED ? 'I' : '-',\n" "\n" "Similar problem. Previously 'I' was shown for negatively instantiated keys; now\n" "it's not. Shouldn't it test 'state != KEY_IS_UNINSTANTIATED'?\n" @@ -106,4 +112,4 @@ "\n" Eric -606743ea7f8960b7c46eb26dcfda77ea925be685ef9a31505ccddf6542815f98 +5220ad968ec01595d4bdd7fe2da374cc7fc432622d79cda856700ed090f972d6
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.