diff for duplicates of <20171012185612.GA11577@gmail.com> diff --git a/a/1.txt b/N1/1.txt index 14e0135..0a4b3d0 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -22,10 +22,10 @@ the new/changed code, and there is one other bug: > { > - return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && > - !test_bit(KEY_FLAG_NEGATIVE, &key->flags); -> + return key->state = KEY_IS_INSTANTIATED; +> + return key->state == KEY_IS_INSTANTIATED; > +} -This should use 'smp_load_acquire(&key->state) = KEY_IS_INSTANTIATED', since +This should use 'smp_load_acquire(&key->state) == KEY_IS_INSTANTIATED', since some ->describe() methods expect to access the payload after this. Yes, it's no worse than before, but as long as the line of code is being replaced we might as well get it right... @@ -36,7 +36,8 @@ well get it right... > +++ 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 = READ_ONCE(key->state); > + @@ -157,3 +158,7 @@ example, but that is no longer a valid example since this patch comes after "KEYS: Fix race between updating and finding a negative key". 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 1622c14..7b5d0f9 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,7 +1,7 @@ "ref\0150782504738.1655.12882942775980793687.stgit@warthog.procyon.org.uk\0" - "From\0Eric Biggers <ebiggers3@gmail.com>\0" - "Subject\0Re: [RFC][PATCH 00/15] KEYS: Fixes\0" - "Date\0Thu, 12 Oct 2017 18:56:12 +0000\0" + "From\0ebiggers3@gmail.com (Eric Biggers)\0" + "Subject\0[RFC][PATCH 00/15] KEYS: Fixes\0" + "Date\0Thu, 12 Oct 2017 11:56:12 -0700\0" "To\0linux-security-module@vger.kernel.org\0" "\00:1\0" "b\0" @@ -29,10 +29,10 @@ "> {\n" "> - return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&\n" "> - !test_bit(KEY_FLAG_NEGATIVE, &key->flags);\n" - "> + return key->state = KEY_IS_INSTANTIATED;\n" + "> + return key->state == KEY_IS_INSTANTIATED;\n" "> +}\n" "\n" - "This should use 'smp_load_acquire(&key->state) = KEY_IS_INSTANTIATED', since\n" + "This should use 'smp_load_acquire(&key->state) == KEY_IS_INSTANTIATED', since\n" "some ->describe() methods expect to access the payload after this. Yes, it's no\n" "worse than before, but as long as the line of code is being replaced we might as\n" "well get it right...\n" @@ -43,7 +43,8 @@ "> +++ b/security/keys/gc.c\n" "> @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys)\n" "> while (!list_empty(keys)) {\n" - "> struct key *key > list_entry(keys->next, struct key, graveyard_link);\n" + "> struct key *key =\n" + "> list_entry(keys->next, struct key, graveyard_link);\n" "> + short state = READ_ONCE(key->state);\n" "> +\n" "\n" @@ -163,6 +164,10 @@ "example, but that is no longer a valid example since this patch comes after\n" "\"KEYS: Fix race between updating and finding a negative key\".\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 -d59fc0a1a6603bb10008a8b3070fc62e5304b2d218dbd443ec8d427c27e453e9 +0b08be4c5558633f5e14f6c38839e391c945415f3f75845215f8c39dd16b3b94
diff --git a/a/1.txt b/N2/1.txt index 14e0135..91975e4 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -22,10 +22,10 @@ the new/changed code, and there is one other bug: > { > - return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && > - !test_bit(KEY_FLAG_NEGATIVE, &key->flags); -> + return key->state = KEY_IS_INSTANTIATED; +> + return key->state == KEY_IS_INSTANTIATED; > +} -This should use 'smp_load_acquire(&key->state) = KEY_IS_INSTANTIATED', since +This should use 'smp_load_acquire(&key->state) == KEY_IS_INSTANTIATED', since some ->describe() methods expect to access the payload after this. Yes, it's no worse than before, but as long as the line of code is being replaced we might as well get it right... @@ -36,7 +36,8 @@ well get it right... > +++ 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 = READ_ONCE(key->state); > + diff --git a/a/content_digest b/N2/content_digest index 1622c14..44e0fe6 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -1,8 +1,12 @@ "ref\0150782504738.1655.12882942775980793687.stgit@warthog.procyon.org.uk\0" "From\0Eric Biggers <ebiggers3@gmail.com>\0" "Subject\0Re: [RFC][PATCH 00/15] KEYS: Fixes\0" - "Date\0Thu, 12 Oct 2017 18:56:12 +0000\0" - "To\0linux-security-module@vger.kernel.org\0" + "Date\0Thu, 12 Oct 2017 11:56:12 -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" "Hi David, a few comments:\n" @@ -29,10 +33,10 @@ "> {\n" "> - return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&\n" "> - !test_bit(KEY_FLAG_NEGATIVE, &key->flags);\n" - "> + return key->state = KEY_IS_INSTANTIATED;\n" + "> + return key->state == KEY_IS_INSTANTIATED;\n" "> +}\n" "\n" - "This should use 'smp_load_acquire(&key->state) = KEY_IS_INSTANTIATED', since\n" + "This should use 'smp_load_acquire(&key->state) == KEY_IS_INSTANTIATED', since\n" "some ->describe() methods expect to access the payload after this. Yes, it's no\n" "worse than before, but as long as the line of code is being replaced we might as\n" "well get it right...\n" @@ -43,7 +47,8 @@ "> +++ b/security/keys/gc.c\n" "> @@ -129,14 +129,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys)\n" "> while (!list_empty(keys)) {\n" - "> struct key *key > list_entry(keys->next, struct key, graveyard_link);\n" + "> struct key *key =\n" + "> list_entry(keys->next, struct key, graveyard_link);\n" "> + short state = READ_ONCE(key->state);\n" "> +\n" "\n" @@ -165,4 +170,4 @@ "\n" Eric -d59fc0a1a6603bb10008a8b3070fc62e5304b2d218dbd443ec8d427c27e453e9 +8b61644fad585347950fa8cd5a1a19186e0088a7bd302b9a61dd70d51c9a449a
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.