linux-kernel - Re: [PATCH v1 1/2] IMA: Define workqueue for early boot "key" measurements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 Dec 2019 08:04:02 -0500
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
        linux-integrity@...r.kernel.org
Cc:     eric.snowberg@...cle.com, dhowells@...hat.com,
        mathew.j.martineau@...ux.intel.com, matthewgarrett@...gle.com,
        sashal@...nel.org, jamorris@...ux.microsoft.com,
        linux-kernel@...r.kernel.org, keyrings@...r.kernel.org
Subject: Re: [PATCH v1 1/2] IMA: Define workqueue for early boot "key"
 measurements

On Tue, 2019-12-10 at 20:40 -0500, Mimi Zohar wrote:
> > +void ima_process_queued_keys_for_measurement(void)
> > +{
> > +	struct ima_measure_key_entry *entry, *tmp;
> > +	LIST_HEAD(temp_ima_measure_keys);
> > +
> > +	if (ima_process_keys_for_measurement)
> > +		return;
> > +
> > +	/*
> > +	 * Any queued keys will be processed now. From here on
> > +	 * keys should be processed right away.
> > +	 */
> > +	ima_process_keys_for_measurement = true;
> 
> This function and the ima_queue_key_for_measurement() are not
> exported, so don't require kernel-doc style comments, but at least
> this comment should not be here.  It could be included as part of the
> function description at the head of the function.

Sorry, one more comment.  Appending "_for_measurement" or inserting
"_measure_" makes these function names unnecessarily long.  This
information can be included in the function descriptions.

Mimi


> 
> Remember we don't add code comments needlessly.  Refer to section "8)
> Commenting" in Documentation/process/coding-style.rst.
> 
> > +
> > +	/*
> > +	 * To avoid holding the mutex when processing queued keys,
> > +	 * transfer the queued keys with the mutex held to a temp list,
> > +	 * release the mutex, and then process the queued keys from
> > +	 * the temp list.
> > +	 *
> > +	 * Since ima_process_keys_for_measurement is set to true above,
> > +	 * any new key will be processed immediately and not be queued.
> > +	 */
> > +	INIT_LIST_HEAD(&temp_ima_measure_keys);
> > +
> > +	mutex_lock(&ima_measure_keys_mutex);
> > +
> > +	list_for_each_entry_safe(entry, tmp, &ima_measure_keys, list)
> > +		list_move_tail(&entry->list, &temp_ima_measure_keys);
> > +
> > +	mutex_unlock(&ima_measure_keys_mutex);
> > +
> > +	list_for_each_entry_safe(entry, tmp, &temp_ima_measure_keys, list) {
> > +		process_buffer_measurement(entry->payload, entry->payload_len,
> > +					   entry->keyring_name, KEY_CHECK, 0,
> > +					   entry->keyring_name);
> > +		list_del(&entry->list);
> > +		ima_free_measure_key_entry(entry);
> > +	}
> > +}
> > +
> >  /**
> >   * ima_post_key_create_or_update - measure asymmetric keys
> >   * @keyring: keyring to which the key is linked to
>