22 May, 2010

1 commit

  • Of the three uses of kref_set in the kernel:

    One really should be kref_put as the code is letting go of a
    reference,
    Two really should be kref_init because the kref is being
    initialised.

    This suggests that making kref_set available encourages bad code.
    So fix the three uses and remove kref_set completely.

    Signed-off-by: NeilBrown
    Acked-by: Mimi Zohar
    Acked-by: Serge Hallyn
    Signed-off-by: Greg Kroah-Hartman

    NeilBrown
     

17 May, 2010

1 commit

  • The ACPI dependency moved to the TPM, where it belongs. Although
    IMA per-se does not require access to the bios measurement log,
    verifying the IMA boot aggregate does, which requires ACPI.

    This patch prereq's 'TPM: ACPI/PNP dependency removal'
    http://lkml.org/lkml/2010/5/4/378.

    Signed-off-by: Mimi Zohar
    Reported-by: Jean-Christophe Dubois
    Acked-by: Serge Hallyn
    Tested-by: Serge Hallyn
    Signed-off-by: James Morris

    Mimi Zohar
     

07 May, 2010

1 commit


06 May, 2010

1 commit


05 May, 2010

1 commit

  • The ACPI dependency moved to the TPM, where it belongs. Although
    IMA per-se does not require access to the bios measurement log,
    verifying the IMA boot aggregate does, which requires ACPI.

    This patch prereq's 'TPM: ACPI/PNP dependency removal'
    http://lkml.org/lkml/2010/5/4/378.

    Signed-off-by: Mimi Zohar
    Reported-by: Jean-Christophe Dubois
    Acked-by: Serge Hallyn
    Tested-by: Serge Hallyn
    Signed-off-by: James Morris

    Mimi Zohar
     

23 Apr, 2010

1 commit

  • As an example IMA emits a warning when it can't find a TPM chip:

    "No TPM chip found, activating TPM-bypass!"

    This patch prefaces that message with IMA so we know what subsystem is
    bypassing the TPM. Do this for all pr_info and pr_err messages.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     

21 Apr, 2010

8 commits

  • integrity_audit_msg() uses "integrity:" in the audit message. This
    violates the (loosely defined) audit system requirements that everything be
    a key=value pair and it doesn't provide additional information. This can
    be obviously gleaned from the message type. Just drop it.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • Convert all of the places IMA calls audit_log_format with %s into
    audit_log_untrusted_string(). This is going to cause them all to get
    quoted, but it should make audit log injection harder.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • IMA policy load parser will reject any policies with a comment. This patch
    will allow the parser to just ignore lines which start with a #. This is not
    very robust. # can ONLY be used at the very beginning of a line. Inline
    comments are not allowed.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • IMA parser will fail if whitespace is used in any way other than a single
    space. Using a tab or even using 2 spaces in a row will result in a policy
    being rejected. This patch makes the kernel ignore whitespace a bit better.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • Currently the ima policy load code will print what it doesn't understand
    but really I think it should reject any policy it doesn't understand. This
    patch makes it so!

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • ima_parse_rule currently sets entry->action = -1 and then later tests
    if (entry->action == UNKNOWN). It is true that UNKNOWN == -1 but actually
    setting it to UNKNOWN makes a lot more sense in case things change in the
    future.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • IMA will accept rules which specify things twice and will only pay
    attention to the last one. We should reject such rules.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     
  • Currently IMA will only accept one rule per write(). This patch allows IMA to
    accept writes which contain multiple rules but only processes one rule per
    write. \n is used as the delimiter between rules. IMA will return a short
    write indicating that it only accepted up to the first \n.

    This allows simple userspace utilities like cat to be used to load an IMA
    policy instead of needing a special userspace utility that understood 'one
    write per rule'

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris

    Eric Paris
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

10 Mar, 2010

1 commit


25 Feb, 2010

1 commit


07 Feb, 2010

4 commits

  • With the movement of the ima hooks functions were renamed from *path* to
    *file* since they always deal with struct file. This patch renames some of
    the ima internal flags to make them consistent with the rest of the code.

    Signed-off-by: Mimi Zohar
    Signed-off-by: Eric Paris
    Signed-off-by: Al Viro

    Mimi Zohar
     
  • ima_path_check actually deals with files! call it ima_file_check instead.

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: Al Viro

    Mimi Zohar
     
  • ima wants to create an inode information struct (iint) when inodes are
    allocated. This means that at least the part of ima which does this
    allocation (the allocation is filled with information later) should
    before any inodes are created. To accomplish this we split the ima
    initialization routine placing the kmem cache allocator inside a
    security_initcall() function. Since this makes use of radix trees we also
    need to make sure that is initialized before security_initcall().

    Signed-off-by: Eric Paris
    Acked-by: Mimi Zohar
    Signed-off-by: Al Viro

    Eric Paris
     
  • The "Untangling ima mess, part 2 with counters" patch messed
    up the counters. Based on conversations with Al Viro, this patch
    streamlines ima_path_check() by removing the counter maintaince.
    The counters are now updated independently, from measuring the file,
    in __dentry_open() and alloc_file() by calling ima_counts_get().
    ima_path_check() is called from nfsd and do_filp_open().
    It also did not measure all files that should have been measured.
    Reason: ima_path_check() got bogus value passed as mask.
    [AV: mea culpa]
    [AV: add missing nfsd bits]

    Signed-off-by: Mimi Zohar
    Signed-off-by: Al Viro

    Mimi Zohar
     

17 Dec, 2009

6 commits

  • Limit the number of imbalance messages to once per filesystem type instead of
    once per system boot. (it's actually slightly racy and could give you a
    couple per fs, but this isn't a real issue)

    Signed-off-by: Mimi Zohar
    Signed-off-by: Al Viro

    Mimi Zohar
     
  • Kill the 'update' argument of ima_path_check(), kill
    dead code in ima.

    Current rules: ima counters are bumped at the same time
    when the file switches from put_filp() fodder to fput()
    one. Which happens exactly in two places - alloc_file()
    and __dentry_open(). Nothing else needs to do that at
    all.

    Signed-off-by: Al Viro

    Al Viro
     
  • ima_inode_free() has some funky #define just to confuse the crap out of me.

    void ima_iint_delete(struct inode *inode)

    and then things actually call ima_inode_free() and nothing calls
    ima_iint_delete().

    Signed-off-by: Eric Paris
    Signed-off-by: Al Viro

    Eric Paris
     
  • We currently have a lot of duplicated code around ima file counts. Clean
    that all up.

    Signed-off-by: Eric Paris
    Acked-by: Serge Hallyn
    Signed-off-by: Al Viro

    Eric Paris
     
  • iints are supposed to be allocated when an inode is allocated (during
    security_inode_alloc()) But we have code which will attempt to allocate
    an iint during measurement calls. If we couldn't allocate the iint and we
    cared, we should have died during security_inode_alloc(). Not make the
    code more complex and less efficient.

    Signed-off-by: Eric Paris
    Signed-off-by: Al Viro

    Eric Paris
     
  • ima_inode_alloc returns 0 and 1, but the LSM hooks expects an errno.

    Signed-off-by: Eric Paris
    Signed-off-by: Al Viro

    Eric Paris
     

03 Dec, 2009

1 commit


19 Nov, 2009

1 commit

  • While running fsstress tests on the NFSv4 mounted ext3 and ext4
    filesystem, the following call trace was generated on the nfs
    server machine.

    Replace GFP_KERNEL with GFP_NOFS in ima_iint_insert() to avoid a
    potential deadlock.

    =================================
    [ INFO: inconsistent lock state ]
    2.6.31-31.el6.x86_64 #1
    ---------------------------------
    inconsistent {RECLAIM_FS-ON-W} -> {IN-RECLAIM_FS-W} usage.
    kswapd2/75 [HC0[0]:SC0[0]:HE1:SE1] takes:
    (jbd2_handle){+.+.?.}, at: [] jbd2_journal_start+0xfe/0x13f
    {RECLAIM_FS-ON-W} state was registered at:
    [] mark_held_locks+0x65/0x99
    [] lockdep_trace_alloc+0xbd/0xf5
    [] kmem_cache_alloc+0x40/0x185
    [] ima_iint_insert+0x3d/0xf1
    [] ima_inode_alloc+0x25/0x44
    [] inode_init_always+0xec/0x271
    [] alloc_inode+0x51/0xa1
    [] new_inode+0x2e/0x94
    [] ext4_new_inode+0xb8/0xdc9
    [] ext4_create+0xcf/0x175
    [] vfs_create+0x82/0xb8
    [] do_filp_open+0x32c/0x9ee
    [] do_sys_open+0x6c/0x12c
    [] sys_open+0x2e/0x44
    [] system_call_fastpath+0x16/0x1b
    [] 0xffffffffffffffff
    irq event stamp: 90371
    hardirqs last enabled at (90371): []
    kmem_cache_alloc+0xf0/0x185
    hardirqs last disabled at (90370): []
    kmem_cache_alloc+0x89/0x185
    softirqs last enabled at (89492): []
    __do_softirq+0x1bf/0x1eb
    softirqs last disabled at (89477): [] call_softirq+0x1c/0x30

    other info that might help us debug this:
    2 locks held by kswapd2/75:
    #0: (shrinker_rwsem){++++..}, at: [] shrink_slab+0x44/0x177
    #1: (&type->s_umount_key#25){++++..}, at: []

    Reported-by: Muni P. Beerakam
    Reported-by: Amit K. Arora
    Cc: stable@kernel.org
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris

    Mimi Zohar
     

25 Oct, 2009

1 commit


02 Oct, 2009

1 commit


23 Sep, 2009

1 commit

  • Make all seq_operations structs const, to help mitigate against
    revectoring user-triggerable function pointers.

    This is derived from the grsecurity patch, although generated from scratch
    because it's simpler than extracting the changes from there.

    Signed-off-by: James Morris
    Acked-by: Serge Hallyn
    Acked-by: Casey Schaufler
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    James Morris
     

07 Sep, 2009

1 commit

  • - As ima_counts_put() may be called after the inode has been freed,
    verify that the inode is not NULL, before dereferencing it.

    - Maintain the IMA file counters in may_open() properly, decrementing
    any counter increments on subsequent errors.

    Reported-by: Ciprian Docan
    Reported-by: J.R. Okajima
    Signed-off-by: Mimi Zohar
    Acked-by: Eric Paris

    Mimi Zohar
     

27 Aug, 2009

2 commits

  • …s/security-testing-2.6

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6:
    IMA: iint put in ima_counts_get and put

    Linus Torvalds
     
  • ima_counts_get() calls ima_iint_find_insert_get() which takes a reference
    to the iint in question, but does not put that reference at the end of the
    function. This can lead to a nasty memory leak. Easy enough to reproduce:

    #include
    #include

    int main (void)
    {
    int i;
    void *ptr;

    for (i=0; i < 100000; i++) {
    ptr = mmap(NULL, 4096, PROT_READ|PROT_WRITE,
    MAP_SHARED|MAP_ANONYMOUS, -1, 0);
    if (ptr == MAP_FAILED)
    return 2;
    munmap(ptr, 4096);
    }

    return 0;
    }

    Signed-off-by: Eric Paris
    Signed-off-by: James Morris

    Eric Paris
     

24 Aug, 2009

1 commit

  • Hashing files larger than INT_MAX causes process to loop.
    Dependent on redefining kernel_read() offset type to loff_t.

    (http://bugzilla.kernel.org/show_bug.cgi?id=13909)

    Cc: stable@kernel.org
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris

    Mimi Zohar
     

29 Jun, 2009

2 commits


12 Jun, 2009

1 commit

  • …s/security-testing-2.6

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6: (44 commits)
    nommu: Provide mmap_min_addr definition.
    TOMOYO: Add description of lists and structures.
    TOMOYO: Remove unused field.
    integrity: ima audit dentry_open failure
    TOMOYO: Remove unused parameter.
    security: use mmap_min_addr indepedently of security models
    TOMOYO: Simplify policy reader.
    TOMOYO: Remove redundant markers.
    SELinux: define audit permissions for audit tree netlink messages
    TOMOYO: Remove unused mutex.
    tomoyo: avoid get+put of task_struct
    smack: Remove redundant initialization.
    integrity: nfsd imbalance bug fix
    rootplug: Remove redundant initialization.
    smack: do not beyond ARRAY_SIZE of data
    integrity: move ima_counts_get
    integrity: path_check update
    IMA: Add __init notation to ima functions
    IMA: Minimal IMA policy and boot param for TCB IMA policy
    selinux: remove obsolete read buffer limit from sel_read_bool
    ...

    Linus Torvalds
     

05 Jun, 2009

1 commit

  • Until we start appraising measurements, the ima_path_check()
    return code should always be 0.

    - Update the ima_path_check() return code comment
    - Instead of the pr_info, audit the dentry_open failure

    Signed-off-by: Mimi Zohar
    Acked-by: Eric Paris
    Signed-off-by: James Morris

    Mimi Zohar