lkml.org 
[lkml]   [2008]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/4] integrity: Linux Integrity Module(LIM)
    On Wed, 12 Nov 2008 22:47:12 -0500
    Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

    > This version resolves the merge issues resulting from the removal
    > of the nameidata parameter to inode_permission(), by moving the
    > integrity_inode_permission() call from inode_permission() to
    > may_open(), and renaming the hook to integrity_nameidata_check().
    > The nameidata is needed in order to open and read the file, so
    > that the file can be hashed(a cryptographically strong checksum.)
    >
    > This patch also fixes the template locking, preventing the template
    > from being freed while being used.
    >
    > This patch is a redesign of the integrity framework, which address a
    > number of issues, including
    > - generalizing the measurement API beyond just inode measurements.
    > - separation of the measurement into distinct collection, appraisal,
    > and commitment phases, for greater flexibility.
    >
    > Extended Verification Module(EVM) and the Integrity Measurement
    > Architecture(IMA) were originally implemented as an LSM module. Based
    > on discussions on the LSM mailing list, a decision was made that the
    > LSM hooks should only be used to enforce mandatory access control
    > decisions and a new set of hooks should be defined specifically for
    > integrity.
    >
    > EVM/IMA was limited to verifying and measuring a file's (i.e. an inode)
    > integrity and the metadata associated with it. Current research is
    > looking into other types of integrity measurements. (i.e. "Linux kernel
    > integrity measurement using contextual inspection", by Peter A. Loscocco,
    > Perry W. Wilson, J. Aaron Pendergrass, C. Durward McDonell,
    > http://doi.acm.org/10.1145/1314354.1314362). As a result, a requirement
    > of the new integrity framework is support for different types of integrity
    > measurements.
    > This patch provides an integrity framework(api and hooks) and placement
    > of the integrity hooks in the appropriate places in the fs directory.
    > Collecting, appraising, and storing of file and other types of integrity
    > data is supported. Multiple integrity templates, which implement the
    > integrity API, may register themselves. For now, only a single integrity
    > provider can register itself for the integrity hooks. (Support for multiple
    > providers registering themselves for the integrity hooks would require
    > some form of stacking.)
    >
    > The six integrity hooks are:
    > nameidata_check_integrity, inode_alloc_integrity, inode_free_integrity,
    > bprm_check_integrity, file_free_integrity, file_mmap
    >
    > The five integrity API calls provided are:
    > integrity_must_measure, integrity_collect_measurement,
    > integrity_appraise_measurement, integrity_store_measurement,
    > and integrity_display_template.
    >
    > The type of integrity data being collected, appraised, stored, or
    > displayed is template dependent.
    >
    >
    > ...
    >
    > +int integrity_register_template(const char *template_name,
    > + const struct template_operations *template_ops)
    > +{
    > + int template_len;
    > + struct template_list_entry *entry;
    > +
    > + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
    > + if (!entry)
    > + return -ENOMEM;
    > + INIT_LIST_HEAD(&entry->template);
    > +
    > + atomic_set(&entry->refcount, 1);
    > + template_len = strlen(template_name);
    > + if (template_len > TEMPLATE_NAME_LEN_MAX) {

    It would be much neater to perform this check before running kzalloc().

    > + kfree(entry);
    > + return -EINVAL;
    > + }
    > + strcpy(entry->template_name, template_name);
    > + entry->template_ops = template_ops;
    > +
    > + mutex_lock(&integrity_templates_mutex);
    > + list_add_rcu(&entry->template, &integrity_templates);
    > + mutex_unlock(&integrity_templates_mutex);
    > + synchronize_rcu();
    > +
    > + return 0;
    > +}
    > +
    > +EXPORT_SYMBOL_GPL(integrity_register_template);

    someone forgot to run checkpatch.

    >
    > ...
    >
    > +static inline void tget(struct template_list_entry *entry)
    > +{
    > + if (!entry)
    > + return;
    > + atomic_inc(&entry->refcount);
    > +}
    > +
    > +static inline void tput(struct template_list_entry *entry)
    > +{
    > + if (!entry)
    > + return;
    > + if (atomic_dec_and_test(&entry->refcount))
    > + kfree(entry);
    > +}

    Do these _really_ need to test for a NULL pointer? It's an extra
    test-n-branch in many fastpaths. It would be better to avoid doing
    this here, if poss.




    \
     
     \ /
      Last update: 2008-11-14 23:19    [W:0.033 / U:0.204 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site