linux-kernel - Re: [RFC][Patch 4/4]integrity: IMA as an integrity service provider

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:	Thu, 22 May 2008 12:39:26 +1000 (EST)
From:	James Morris <jmorris@...ei.org>
To:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
cc:	linux-kernel@...r.kernel.org, safford@...son.ibm.com,
	serue@...ux.vnet.ibm.com, sailer@...son.ibm.com, zohar@...ibm.com,
	Stephen Smalley <sds@...ho.nsa.gov>,
	CaseySchaufler <casey@...aufler-ca.com>
Subject: Re: [RFC][Patch 4/4]integrity: IMA as an integrity service provider

On Wed, 21 May 2008, Mimi Zohar wrote:

> +#define IMA_HASH_KEY(digest) (hash_long(\
> +	(unsigned long)(*digest), IMA_HASH_BITS));

Please make this a static inline and use hash_ptr().

> +	i_size = i_size_read(file->f_dentry->d_inode);
> +	while (offset < i_size) {
> +		int error;
> +
> +		rbuf_len = kernel_read(file, offset, rbuf, PAGE_SIZE);
> +		if (rbuf_len <= 0)
> +			break;

Why is the return value of kernel_read() not being propagated to the 
caller?

> +		error = crypto_hash_update(desc, sg, rbuf_len);
> +		if (error) {
> +			result = -EINVAL;
> +			break;
> +		}

Why are you replacing the error code from crypto_hash_update() with 
-EINVAL ?

> +	tfm = crypto_alloc_hash(ima_hash, 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(tfm)) {
> +		printk(KERN_INFO "%s: failed to load %s transform: %ld\n",
> +		       __func__, ima_hash, PTR_ERR(tfm));
> +		return -ENOSYS;
> +	}

Similarly, why not propagate the actual error ?

(This kind of thing happens quite a lot in the code).



> +static int __init init_ima(void)
> +{
> +	int error;
> +	tfm_hash = crypto_alloc_hash(ima_hash, 0, CRYPTO_ALG_ASYNC);
> +
> +	error = ima_init();
> +	if (error)
> +		goto out;
> +	ima_fixup_inodes();
> +	if (ima_base_hooks)
> +		error = register_integrity(&ima_base_ops);
> +	else
> +		error = register_integrity(&ima_integrity_ops);
> +	register_template("ima", &ima_template_ops);
> +out:
> +	return error;
> +}

What if crypto_alloc_hash() fails ?  Actually, where is it used?


- James
-- 
James Morris
<jmorris@...ei.org>
--
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/