lkml.org 
[lkml]   [2008]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC][Patch 4/4]integrity: IMA as an integrity service provider
From
Date

Thank you for reviewing the code. Will fix these and add
locking around integrity_templates.

Mimi

On Thu, 2008-05-22 at 12:39 +1000, James Morris wrote:
> 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



\
 
 \ /
  Last update: 2008-05-22 20:43    [W:0.136 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site