22 Jun, 2011

1 commit

  • Fix error handling in construct_key_and_link().

    If construct_alloc_key() returns an error, it shouldn't pass out through
    the normal path as the key_serial() called by the kleave() statement
    will oops when it gets an error code in the pointer:

    BUG: unable to handle kernel paging request at ffffffffffffff84
    IP: [] request_key_and_link+0x4d7/0x52f
    ..
    Call Trace:
    [] request_key+0x41/0x75
    [] cifs_get_spnego_key+0x206/0x226 [cifs]
    [] CIFS_SessSetup+0x511/0x1234 [cifs]
    [] cifs_setup_session+0x90/0x1ae [cifs]
    [] cifs_get_smb_ses+0x34b/0x40f [cifs]
    [] cifs_mount+0x13f/0x504 [cifs]
    [] cifs_do_mount+0xc4/0x672 [cifs]
    [] mount_fs+0x69/0x155
    [] vfs_kern_mount+0x63/0xa0
    [] do_kern_mount+0x4d/0xdf
    [] do_mount+0x63c/0x69f
    [] sys_mount+0x88/0xc2
    [] system_call_fastpath+0x16/0x1b

    Signed-off-by: David Howells
    Acked-by: Jeff Layton
    Signed-off-by: Linus Torvalds

    David Howells
     

21 Jun, 2011

1 commit

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6:
    devcgroup_inode_permission: take "is it a device node" checks to inlined wrapper
    fix comment in generic_permission()
    kill obsolete comment for follow_down()
    proc_sys_permission() is OK in RCU mode
    reiserfs_permission() doesn't need to bail out in RCU mode
    proc_fd_permission() is doesn't need to bail out in RCU mode
    nilfs2_permission() doesn't need to bail out in RCU mode
    logfs doesn't need ->permission() at all
    coda_ioctl_permission() is safe in RCU mode
    cifs_permission() doesn't need to bail out in RCU mode
    bad_inode_permission() is safe from RCU mode
    ubifs: dereferencing an ERR_PTR in ubifs_mount()

    Linus Torvalds
     

20 Jun, 2011

1 commit


18 Jun, 2011

1 commit

  • ____call_usermodehelper() now erases any credentials set by the
    subprocess_inf::init() function. The problem is that commit
    17f60a7da150 ("capabilites: allow the application of capability limits
    to usermode helpers") creates and commits new credentials with
    prepare_kernel_cred() after the call to the init() function. This wipes
    all keyrings after umh_keys_init() is called.

    The best way to deal with this is to put the init() call just prior to
    the commit_creds() call, and pass the cred pointer to init(). That
    means that umh_keys_init() and suchlike can modify the credentials
    _before_ they are published and potentially in use by the rest of the
    system.

    This prevents request_key() from working as it is prevented from passing
    the session keyring it set up with the authorisation token to
    /sbin/request-key, and so the latter can't assume the authority to
    instantiate the key. This causes the in-kernel DNS resolver to fail
    with ENOKEY unconditionally.

    Signed-off-by: David Howells
    Acked-by: Eric Paris
    Tested-by: Jeff Layton
    Signed-off-by: Linus Torvalds

    David Howells
     

15 Jun, 2011

2 commits


14 Jun, 2011

1 commit

  • In tomoyo_mount_acl() since 2.6.36, kern_path() was called without checking
    dev_name != NULL. As a result, an unprivileged user can trigger oops by issuing
    mount(NULL, "/", "ext3", 0, NULL) request.
    Fix this by checking dev_name != NULL before calling kern_path(dev_name).

    Signed-off-by: Tetsuo Handa
    Cc: stable@kernel.org
    Signed-off-by: James Morris

    Tetsuo Handa
     

09 Jun, 2011

2 commits

  • Affected kernels 2.6.36 - 3.0

    AppArmor may do a GFP_KERNEL memory allocation with task_lock(tsk->group_leader);
    held when called from security_task_setrlimit. This will only occur when the
    task's current policy has been replaced, and the task's creds have not been
    updated before entering the LSM security_task_setrlimit() hook.

    BUG: sleeping function called from invalid context at mm/slub.c:847
    in_atomic(): 1, irqs_disabled(): 0, pid: 1583, name: cupsd
    2 locks held by cupsd/1583:
    #0: (tasklist_lock){.+.+.+}, at: [] do_prlimit+0x61/0x189
    #1: (&(&p->alloc_lock)->rlock){+.+.+.}, at: []
    do_prlimit+0x94/0x189
    Pid: 1583, comm: cupsd Not tainted 3.0.0-rc2-git1 #7
    Call Trace:
    [] __might_sleep+0x10d/0x112
    [] slab_pre_alloc_hook.isra.49+0x2d/0x33
    [] kmem_cache_alloc+0x22/0x132
    [] prepare_creds+0x35/0xe4
    [] aa_replace_current_profile+0x35/0xb2
    [] aa_current_profile+0x45/0x4c
    [] apparmor_task_setrlimit+0x19/0x3a
    [] security_task_setrlimit+0x11/0x13
    [] do_prlimit+0xd2/0x189
    [] sys_setrlimit+0x3b/0x48
    [] system_call_fastpath+0x16/0x1b

    Signed-off-by: John Johansen
    Reported-by: Miles Lane
    Cc: stable@kernel.org
    Signed-off-by: James Morris

    John Johansen
     
  • This is a rather hot function that is called with a potentially NULL
    "struct common_audit_data" pointer argument. And in that case it has to
    provide and initialize its own dummy common_audit_data structure.

    However, all the _common_ cases already pass it a real audit-data
    structure, so that uncommon NULL case not only creates a silly run-time
    test, more importantly it causes that function to have a big stack frame
    for the dummy variable that isn't even used in the common case!

    So get rid of that stupid run-time behavior, and make the (few)
    functions that currently call with a NULL pointer just call a new helper
    function instead (naturally called inode_has_perm_noapd(), since it has
    no adp argument).

    This makes the run-time test be a static code generation issue instead,
    and allows for a much denser stack since none of the common callers need
    the dummy structure. And a denser stack not only means less stack space
    usage, it means better cache behavior. So we have a win-win-win from
    this simplification: less code executed, smaller stack footprint, and
    better cache behavior.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

01 Jun, 2011

1 commit

  • When invalid parameters are passed to apparmor_setprocattr a NULL deref
    oops occurs when it tries to record an audit message. This is because
    it is passing NULL for the profile parameter for aa_audit. But aa_audit
    now requires that the profile passed is not NULL.

    Fix this by passing the current profile on the task that is trying to
    setprocattr.

    Signed-off-by: Kees Cook
    Signed-off-by: John Johansen
    Cc: stable@kernel.org
    Signed-off-by: James Morris

    Kees Cook
     

28 May, 2011

1 commit


27 May, 2011

5 commits

  • Right now security_get_user_sids() will pass in a NULL avd pointer to
    avc_has_perm_noaudit(), which then forces that function to have a dummy
    entry for that case and just generally test it.

    Don't do it. The normal callers all pass a real avd pointer, and this
    helper function is incredibly hot. So don't make avc_has_perm_noaudit()
    do conditional stuff that isn't needed for the common case.

    This also avoids some duplicated stack space.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • Add cgroup subsystem callbacks for per-thread attachment in atomic contexts

    Add can_attach_task(), pre_attach(), and attach_task() as new callbacks
    for cgroups's subsystem interface. Unlike can_attach and attach, these
    are for per-thread operations, to be called potentially many times when
    attaching an entire threadgroup.

    Also, the old "bool threadgroup" interface is removed, as replaced by
    this. All subsystems are modified for the new interface - of note is
    cpuset, which requires from/to nodemasks for attach to be globally scoped
    (though per-cpuset would work too) to persist from its pre_attach to
    attach_task and attach.

    This is a pre-patch for cgroup-procs-writable.patch.

    Signed-off-by: Ben Blum
    Cc: "Eric W. Biederman"
    Cc: Li Zefan
    Cc: Matt Helsley
    Reviewed-by: Paul Menage
    Cc: Oleg Nesterov
    Cc: David Rientjes
    Cc: Miao Xie
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ben Blum
     
  • I submit the patch again, according to patch submission convension.

    This patch enables to accept percent-encoded object names as forth
    argument of /selinux/create interface to avoid possible bugs when we
    give an object name including whitespace or multibutes.

    E.g) if and when a userspace object manager tries to create a new object
    named as "resolve.conf but fake", it shall give this name as the forth
    argument of the /selinux/create. But sscanf() logic in kernel space
    fetches only the part earlier than the first whitespace.
    In this case, selinux may unexpectedly answer a default security context
    configured to "resolve.conf", but it is bug.

    Although I could not test this patch on named TYPE_TRANSITION rules
    actually, But debug printk() message seems to me the logic works
    correctly.
    I assume the libselinux provides an interface to apply this logic
    transparently, so nothing shall not be changed from the viewpoint of
    application.

    Signed-off-by: KaiGai Kohei
    Signed-off-by: Eric Paris

    Kohei Kaigai
     
  • Conflicts:
    lib/flex_array.c
    security/selinux/avc.c
    security/selinux/hooks.c
    security/selinux/ss/policydb.c
    security/smack/smack_lsm.c

    Eric Paris
     
  • Since this cred was not created with copy_creds(), it needs to get
    initialized. Otherwise use of syscall(__NR_keyctl, KEYCTL_SESSION_TO_PARENT);
    can lead to a NULL deref. Thanks to Robert for finding this.

    But introduced by commit 47a150edc2a ("Cache user_ns in struct cred").

    Signed-off-by: Serge E. Hallyn
    Reported-by: Robert Święcki
    Cc: David Howells
    Cc: stable@kernel.org (2.6.39)
    Signed-off-by: Linus Torvalds

    Serge E. Hallyn
     

24 May, 2011

3 commits

  • Conflicts:
    lib/flex_array.c
    security/selinux/avc.c
    security/selinux/hooks.c
    security/selinux/ss/policydb.c
    security/smack/smack_lsm.c

    Manually resolve conflicts.

    Signed-off-by: James Morris

    James Morris
     
  • James Morris
     
  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (39 commits)
    b43: fix comment typo reqest -> request
    Haavard Skinnemoen has left Atmel
    cris: typo in mach-fs Makefile
    Kconfig: fix copy/paste-ism for dell-wmi-aio driver
    doc: timers-howto: fix a typo ("unsgined")
    perf: Only include annotate.h once in tools/perf/util/ui/browsers/annotate.c
    md, raid5: Fix spelling error in comment ('Ofcourse' --> 'Of course').
    treewide: fix a few typos in comments
    regulator: change debug statement be consistent with the style of the rest
    Revert "arm: mach-u300/gpio: Fix mem_region resource size miscalculations"
    audit: acquire creds selectively to reduce atomic op overhead
    rtlwifi: don't touch with treewide double semicolon removal
    treewide: cleanup continuations and remove logging message whitespace
    ath9k_hw: don't touch with treewide double semicolon removal
    include/linux/leds-regulator.h: fix syntax in example code
    tty: fix typo in descripton of tty_termios_encode_baud_rate
    xtensa: remove obsolete BKL kernel option from defconfig
    m68k: fix comment typo 'occcured'
    arch:Kconfig.locks Remove unused config option.
    treewide: remove extra semicolons
    ...

    Linus Torvalds
     

20 May, 2011

4 commits

  • There is no point in counting hits - we can calculate it from the number
    of lookups and misses.

    This makes the avc statistics a bit smaller, and makes the code
    generation better too.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • You can turn off the avc cache stats, but distributions seem to not do
    that (perhaps because several performance tuning how-to's talk about the
    avc cache statistics).

    Which is sad, because the code it generates is truly horrendous, with
    the statistics update being sandwitched between get_cpu/put_cpu which in
    turn causes preemption disables etc. We're talking ten+ instructions
    just to increment a per-cpu variable in some pretty hot code.

    Fix the craziness by just using 'this_cpu_inc()' instead. Suddenly we
    only need a single 'inc' instruction to increment the statistics. This
    is quite noticeable in the incredibly hot avc_has_perm_noaudit()
    function (which triggers all the statistics by virtue of doing an
    avc_lookup() call).

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • * 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip: (78 commits)
    Revert "rcu: Decrease memory-barrier usage based on semi-formal proof"
    net,rcu: convert call_rcu(prl_entry_destroy_rcu) to kfree
    batman,rcu: convert call_rcu(softif_neigh_free_rcu) to kfree_rcu
    batman,rcu: convert call_rcu(neigh_node_free_rcu) to kfree()
    batman,rcu: convert call_rcu(gw_node_free_rcu) to kfree_rcu
    net,rcu: convert call_rcu(kfree_tid_tx) to kfree_rcu()
    net,rcu: convert call_rcu(xt_osf_finger_free_rcu) to kfree_rcu()
    net/mac80211,rcu: convert call_rcu(work_free_rcu) to kfree_rcu()
    net,rcu: convert call_rcu(wq_free_rcu) to kfree_rcu()
    net,rcu: convert call_rcu(phonet_device_rcu_free) to kfree_rcu()
    perf,rcu: convert call_rcu(swevent_hlist_release_rcu) to kfree_rcu()
    perf,rcu: convert call_rcu(free_ctx) to kfree_rcu()
    net,rcu: convert call_rcu(__nf_ct_ext_free_rcu) to kfree_rcu()
    net,rcu: convert call_rcu(net_generic_release) to kfree_rcu()
    net,rcu: convert call_rcu(netlbl_unlhsh_free_addr6) to kfree_rcu()
    net,rcu: convert call_rcu(netlbl_unlhsh_free_addr4) to kfree_rcu()
    security,rcu: convert call_rcu(sel_netif_free) to kfree_rcu()
    net,rcu: convert call_rcu(xps_dev_maps_release) to kfree_rcu()
    net,rcu: convert call_rcu(xps_map_release) to kfree_rcu()
    net,rcu: convert call_rcu(rps_map_release) to kfree_rcu()
    ...

    Linus Torvalds
     
  • move LSM-, credentials-, and keys-related files from Documentation/
    to Documentation/security/,
    add Documentation/security/00-INDEX, and
    update all occurrences of Documentation/
    to Documentation/security/.

    Randy Dunlap
     

19 May, 2011

1 commit


13 May, 2011

2 commits


12 May, 2011

2 commits

  • In tomoyo_correct_domain() since 2.6.36, TOMOYO was by error validating
    "" + "/foo/\" + "/bar" when " /foo/\* /bar" was given.
    As a result, legal domainnames like " /foo/\* /bar" are rejected.

    Reported-by: Hayama Yossihiro
    Signed-off-by: Tetsuo Handa
    Signed-off-by: James Morris

    Tetsuo Handa
     
  • In the interest of keeping userspace from having to create new root
    filesystems all the time, let's follow the lead of the other in-kernel
    filesystems and provide a proper mount point for it in sysfs.

    For selinuxfs, this mount point should be in /sys/fs/selinux/

    Cc: Stephen Smalley
    Cc: James Morris
    Cc: Eric Paris
    Cc: Lennart Poettering
    Cc: Daniel J Walsh
    Signed-off-by: Greg Kroah-Hartman
    [include kobject.h - Eric Paris]
    [use selinuxfs_obj throughout - Eric Paris]
    Signed-off-by: Eric Paris

    Greg Kroah-Hartman
     

08 May, 2011

2 commits


04 May, 2011

1 commit


29 Apr, 2011

9 commits

  • Change flex_array_prealloc to take the number of elements for which space
    should be allocated instead of the last (inclusive) element. Users
    and documentation are updated accordingly. flex_arrays got introduced before
    they had users. When folks started using it, they ended up needing a
    different API than was coded up originally. This swaps over to the API that
    folks apparently need.

    Based-on-patch-by: Steffen Klassert
    Signed-off-by: Eric Paris
    Tested-by: Chris Richards
    Acked-by: Dave Hansen
    Cc: stable@kernel.org [2.6.38+]

    Eric Paris
     
  • New inodes are created in a two stage process. We first will compute the
    label on a new inode in security_inode_create() and check if the
    operation is allowed. We will then actually re-compute that same label and
    apply it in security_inode_init_security(). The change to do new label
    calculations based in part on the last component of the path name only
    passed the path component information all the way down the
    security_inode_init_security hook. Down the security_inode_create hook the
    path information did not make it past may_create. Thus the two calculations
    came up differently and the permissions check might not actually be against
    the label that is created. Pass and use the same information in both places
    to harmonize the calculations and checks.

    Reported-by: Dominick Grift
    Signed-off-by: Eric Paris

    Eric Paris
     
  • We currently have inode_has_perm and dentry_has_perm. dentry_has_perm just
    calls inode_has_perm with additional audit data. But dentry_has_perm can
    take either a dentry or a path. Split those to make the code obvious and
    to fix the previous problem where I thought dentry_has_perm always had a
    valid dentry and mnt.

    Signed-off-by: Eric Paris

    Eric Paris
     
  • Change flex_array_prealloc to take the number of elements for which space
    should be allocated instead of the last (inclusive) element. Users
    and documentation are updated accordingly. flex_arrays got introduced before
    they had users. When folks started using it, they ended up needing a
    different API than was coded up originally. This swaps over to the API that
    folks apparently need.

    Based-on-patch-by: Steffen Klassert
    Signed-off-by: Eric Paris
    Tested-by: Chris Richards
    Acked-by: Dave Hansen
    Cc: stable@kernel.org [2.6.38+]

    Eric Paris
     
  • New inodes are created in a two stage process. We first will compute the
    label on a new inode in security_inode_create() and check if the
    operation is allowed. We will then actually re-compute that same label and
    apply it in security_inode_init_security(). The change to do new label
    calculations based in part on the last component of the path name only
    passed the path component information all the way down the
    security_inode_init_security hook. Down the security_inode_create hook the
    path information did not make it past may_create. Thus the two calculations
    came up differently and the permissions check might not actually be against
    the label that is created. Pass and use the same information in both places
    to harmonize the calculations and checks.

    Reported-by: Dominick Grift
    Signed-off-by: Eric Paris

    Eric Paris
     
  • To shorten the list we need to run if filename trans rules exist for the type
    of the given parent directory I put them in a hashtable. Given the policy we
    are expecting to use in Fedora this takes the worst case list run from about
    5,000 entries to 17.

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

    Eric Paris
     
  • Instead of a hashtab entry counter function only useful for range
    transition rules make a function generic for any hashtable to use.

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

    Eric Paris
     
  • We have custom debug functions like rangetr_hash_eval and symtab_hash_eval
    which do the same thing. Just create a generic function that takes the name
    of the hash table as an argument instead of having custom functions.

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

    Eric Paris
     
  • Right now we walk to filename trans rule list for every inode that is
    created. First passes at policy using this facility creates around 5000
    filename trans rules. Running a list of 5000 entries every time is a bad
    idea. This patch adds a new ebitmap to policy which has a bit set for each
    ttype that has at least 1 filename trans rule. Thus when an inode is
    created we can quickly determine if any rules exist for this parent
    directory type and can skip the list if we know there is definitely no
    relevant entry.

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

    Eric Paris