On 09/02/2021 22:53, Mickaël Salaün wrote:
>
> On 09/02/2021 00:05, Eric Snowberg wrote:
>>
>>> On Feb 6, 2021, at 11:30 AM, Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
>>>
>>> On 06/02/2021 02:14, Eric Snowberg wrote:
>>>
>>>> I have done some additional testing, I am seeing a regression. The blacklist
>>>> keyring is no longer picking up any of the hashes from the dbx during boot.
>>>> I backed out the merge with my changes (fdbbe7ceeb95090d09c33ce0497e0394c82aa33d)
>>>> and still see the regression. I then backed out Mickaël merge
>>>> (5bf1adccf5c41dbdd51d1f4de220d335d9548598) and it fixes the regression.
>>>>
>>>> On a x86 with the updated dbx from uefi.org, I’d expect to see 234 bin hash entries
>>>> in the blacklist keyring. With the current merged code, there is none.
>>>
>>> Hum, I missed a part in refactoring (commit
>>> f78e50c8f750c0ac6767ac1ed006360cf77c56c4). :/
>>> Could you please test the following patch?
>>>
>>> diff --git a/certs/blacklist.c b/certs/blacklist.c
>>> index 07c592ae5307..f998a2e85ddc 100644
>>> --- a/certs/blacklist.c
>>> +++ b/certs/blacklist.c
>>> @@ -197,13 +197,16 @@ int mark_hash_blacklisted(const u8 *hash, size_t
>>> hash_len,
>>> enum blacklist_hash_type hash_type)
>>> {
>>> const char *buffer;
>>> + int err;
>>>
>>> buffer = get_raw_hash(hash, hash_len, hash_type);
>>> if (IS_ERR(buffer))
>>> return PTR_ERR(buffer);
>>> + err = mark_raw_hash_blacklisted(buffer);
>>> kfree(buffer);
>>> - return 0;
>>> + return err;
>>> }
>>
>> I applied this patch, it works better, but there is still a regression.
>> Most of the hashes show up in the blacklist keyring now. However some
>> do not, here is what I see in the log during boot:
>>
>> [ 2.321876] blacklist: Problem blacklisting hash (-13)
>> [ 2.322729] blacklist: Problem blacklisting hash (-13)
>> [ 2.323549] blacklist: Problem blacklisting hash (-13)
>> [ 2.324369] blacklist: Problem blacklisting hash (-13)
>>
>>> Is it possible to test these kind of dbx blacklist with Qemu?
>>
>> Yes, just use OVMF.
>>
>
> My changes (with the fix) don't change the previous semantic. I just
> tested without my changes and with my changes (and the fix), and I get
> the same result: 184 bin hashes with
> https://uefi.org/sites/default/files/resources/dbxupdate_x64.bin
>
> Could you please re-test and if there is still an issue bisect and share
> the certificates causing this issue?
>
> David, do you want me to send the two new patches or an updated full
> patch series?
>
I found the issue and fixed it in a new patch series:
https://lore.kernel.org/lkml/20210210120410.471693-1-mic@xxxxxxxxxxx/
