Re: [PATCH v2 5/7] efi: Import certificates from UEFI Secure Boot

[Nayna Jain]

On 12/12/2018 12:17 AM, James Morris wrote:
> On Sun, 9 Dec 2018, Nayna Jain wrote:
>
> > +/*
> > + * Blacklist an X509 TBS hash.
> > + */
> > +static __init void uefi_blacklist_x509_tbs(const char *source,
> > +					   const void *data, size_t len)
> > +{
> > +	char *hash, *p;
> > +
> > +	hash = kmalloc(4 + len * 2 + 1, GFP_KERNEL);
> > +	if (!hash)
> > +		return;
> > +	p = memcpy(hash, "tbs:", 4);
> > +	p += 4;
> > +	bin2hex(p, data, len);
> > +	p += len * 2;
> > +	*p = 0;
> > +
> > +	mark_hash_blacklisted(hash);
> > +	kfree(hash);
> > +}
> > +
> > +/*
> > + * Blacklist the hash of an executable.
> > + */
> > +static __init void uefi_blacklist_binary(const char *source,
> > +					 const void *data, size_t len)
> > +{
> > +	char *hash, *p;
> > +
> > +	hash = kmalloc(4 + len * 2 + 1, GFP_KERNEL);
> > +	if (!hash)
> > +		return;
> > +	p = memcpy(hash, "bin:", 4);
> > +	p += 4;
> > +	bin2hex(p, data, len);
> > +	p += len * 2;
> > +	*p = 0;
> > +
> > +	mark_hash_blacklisted(hash);
> > +	kfree(hash);
> > +}
> These could be refactored into one function.

Thanks James for reviewing.

Yes, the code should be refactored. However, I think making it a single 
function would require adding a new field to the function callback 
definitions as well. Probably, a simpler approach would be to define a 
common function uefi_blacklist_hash(...)Â which can then be used by the 
two functions uefi_blacklist_x509_tbs(...) and 
uefi_blacklist_binary(...). These two functions now act as wrapper 
functions. Below is the example code:

+/*
+ * Blacklist a hash.
+ */
+static __init void uefi_blacklist_hash(const char *source, const void 
*data,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ size_t len, char *type, size_t type_len)
+{
+ÂÂÂÂÂÂ char *hash, *p;
+
+ÂÂÂÂÂÂ hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
+ÂÂÂÂÂÂ if (!hash)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return;
+ÂÂÂÂÂÂ p = memcpy(hash, type, type_len);
+ÂÂÂÂÂÂ p += type_len;
+ÂÂÂÂÂÂ bin2hex(p, data, len);
+ÂÂÂÂÂÂ p += len * 2;
+ÂÂÂÂÂÂ *p = 0;
+
+ÂÂÂÂÂÂ mark_hash_blacklisted(hash);
+ÂÂÂÂÂÂ kfree(hash);
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const void *data, size_t len)
+{
+ÂÂÂÂÂÂ uefi_blacklist_hash(source, data, len, "tbs:" , 4);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const void *data, size_t len)
+{
+ÂÂÂÂÂÂ uefi_blacklist_hash(source, data, len, "bin:" , 4);
+}


Thanks & Regards,
ÂÂ - Nayna