linux-kernel - Re: [PATCH v2] dm: verity target

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 Nov 2011 10:22:52 +0100
From:	Steffen Klassert <steffen.klassert@...unet.com>
To:	Mandeep Singh Baines <msb@...omium.org>
Cc:	Alasdair G Kergon <agk@...hat.com>, dm-devel@...hat.com,
	Will Drewry <wad@...omium.org>,
	Elly Jones <ellyjones@...omium.org>,
	Milan Broz <mbroz@...hat.com>,
	Olof Johansson <olofj@...omium.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] dm: verity target

On Fri, Nov 11, 2011 at 09:34:44AM -0800, Mandeep Singh Baines wrote:
>
> +/**
> + * dm_bht_compute_hash: hashes a page of data
> + */
> +static int dm_bht_compute_hash(struct dm_bht *bht, struct page *pg,
> +			       unsigned int offset, u8 *digest)
> +{
> +	struct shash_desc *hash_desc = bht->hash_desc[smp_processor_id()];
> +	void *data;
> +	int err;

You don't need to have one shash_desc per cpu. As the shash interface is
synchronous, you could either allocate a new shash_desc with every call
to dm_bht_compute_hash, or just place the shash_desc with the context to
the stack of this function. The easiest thing would be to add the
shash transformation direct to dm_bht and then something like

        struct {
                struct shash_desc desc;        
                char ctx[crypto_shash_descsize(bht->tfm)];                                                
        } hash_desc;

	hash_desc.desc.tfm = bmt->tfm;

The intermediate and final digest values are stored to hash_desc.ctx.
This is local, hence it's reentrant.

> +
> +	/* Note, this is synchronous. */
> +	if (crypto_shash_init(hash_desc)) {
> +		DMCRIT("failed to reinitialize crypto hash (proc:%d)",
> +			smp_processor_id());
> +		return -EINVAL;
> +	}
> +	data = kmap_atomic(pg);
> +	err = crypto_shash_update(hash_desc, data + offset, PAGE_SIZE);
> +	kunmap_atomic(data);
> +	if (err) {
> +		DMCRIT("crypto_hash_update failed");
> +		return -EINVAL;
> +	}
> +	if (crypto_shash_update(hash_desc, bht->salt, sizeof(bht->salt))) {
> +		DMCRIT("crypto_hash_update failed");
> +		return -EINVAL;
> +	}
> +	if (crypto_shash_final(hash_desc, digest)) {
> +		DMCRIT("crypto_hash_final failed");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +

[snip]

> + * TODO(wad): All hash storage memory is pre-allocated and freed once an
> + * entire branch has been verified.
> + */
> +struct dm_bht {
> +	/* Configured values */
> +	int depth;  /* Depth of the tree including the root */
> +	unsigned int block_count;  /* Number of blocks hashed */
> +	unsigned int block_size;  /* Size of a hash block */
> +	char hash_alg[CRYPTO_MAX_ALG_NAME];
> +	unsigned char salt[DM_BHT_SALT_SIZE];
> +
> +	/* Computed values */
> +	unsigned int node_count;  /* Data size (in hashes) for each entry */
> +	unsigned int node_count_shift;  /* first bit set - 1 */
> +	/* There is one per CPU so that verified can be simultaneous. */
> +	struct shash_desc *hash_desc[NR_CPUS];  /* Container for the hash alg */

As mentioned above, you don't need to have one shash_desc per cpu.
Replace this with a 'struct crypto_shash *tfm;' and add this transformation
to the local hash_desc.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/