lkml.org 
[lkml]   [2014]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [GIT PULL] Fix for Integrity subsystem null pointer deref
    On Oct 29, 2014 6:00 AM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
    >
    > On Tue, 2014-10-28 at 22:08 -0700, Andy Lutomirski wrote:
    > > On Tue, Oct 28, 2014 at 8:55 PM, James Morris <jmorris@namei.org> wrote:
    > > > These changes fix a bug in xattr handling, where the evm and ima
    > > > inode_setxattr() functions do not check for empty xattrs being passed from
    > > > userspace (leading to user-triggerable null pointer dereferences).
    > > >
    > > > Please pull.
    > > >
    > > >
    > > > The following changes since commit 9f76628da20f96a179ca62b504886f99ecc29223:
    > > >
    > > > Merge branch 'for-3.18' of git://linux-nfs.org/~bfields/linux (2014-10-28 13:32:06 -0700)
    > > >
    > > > are available in the git repository at:
    > > >
    > > > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus
    > > >
    > > > Dmitry Kasatkin (2):
    > > > ima: check xattr value length and type in the ima_inode_setxattr()
    > >
    > > I haven't read this one, but:
    > >
    > > > evm: check xattr value length and type in evm_inode_setxattr()
    > >
    > > const struct evm_ima_xattr_data *xattr_data = xattr_value;
    > > - if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0)
    > > - && (xattr_data->type == EVM_XATTR_HMAC))
    > > - return -EPERM;
    > > + if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
    > > + if (!xattr_value_len)
    > > + return -EINVAL;
    > > + if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
    > > + return -EPERM;
    > > + }
    > >
    > > Huh? (Sorry about severe whitespace damage.)
    > >
    > > Shouldn't there be something like if (xattr_value_len < sizeof(struct
    > > evm_ima_xattr_data)) return -EINVAL?
    >
    > Prior to commit 2fb1c9a "evm: prohibit userspace writing 'security.evm'
    > HMAC value", a process with CAP_SYS_ADMIN could write either an HMAC or
    > signature. As the HMAC key should only be known to the kernel, only
    > signatures are now allowed. Instead of "struct evm_ima_xattr_data", the
    > code should reflect this change and use "struct signature_v2_hdr".
    > We'll clean up this code for the next release. For now, this patch
    > prevents the oops.
    >

    I have no idea what the semantics are. All I'm saying is that it
    looks like the code still accesses memory past the end of the buffer.
    The buffer isn't a null pointer, so the symptom is different, but it
    may still be a security bug.

    --Andy

    > thanks,
    >
    > Mimi
    >


    \
     
     \ /
      Last update: 2014-10-29 18:01    [W:3.321 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site