On Mon, 2018-05-14 at 13:19 -0400, Mimi Zohar wrote:
> On Mon, 2018-05-14 at 10:01 -0700, Matthew Garrett wrote:
> > On Sun, May 13, 2018 at 9:41 AM Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > > On Fri, 2018-05-11 at 16:12 -0700, Matthew Garrett wrote:
> > > +
> > > > + if (strcmp(xattr->name, ".") == 0) {
> > > > + evm_xattrs_locked = 1;
> > > > + inode = evm_xattrs->d_inode;
> > > > + inode_lock(inode);
> > > > + newattrs.ia_mode = S_IFREG | 0440;
> > > > + newattrs.ia_valid = ATTR_MODE;
> > > > + err = notify_change(evm_xattrs, &newattrs, NULL);
> > > > + inode_unlock(inode);
> > > > + audit_log_format(ab, "locked");
> > > > + if (!err)
> > > > + err = count;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + audit_log_format(ab, "xattr=");
> > > > + audit_log_untrustedstring(ab, xattr->name);
> > > > +
> > > > + if (strncmp(xattr->name, XATTR_SECURITY_PREFIX,
> > > > + XATTR_SECURITY_PREFIX_LEN) != 0) {
> > > > + err = -EINVAL;
> > > > + goto out;
> > > > + }
> >
> > > This test now prevents locking the xattr names list. Making this an
> > > else clause will fix it.
> >
> > Are you sure? The check for "." happens before this, and jumps over the
> > rest of the function.
>
> Oh! It did work, but the messages are confusing.
>
> # cat /sys/kernel/security/integrity/evm/evm_xattrs
> security.selinux
> security.ima
> security.capability
> security.foo
> # echo foo > /sys/kernel/security/integrity/evm/evm_xattrs
> bash: echo: write error: Operation not permitted
>
> # echo security.foo > /sys/kernel/security/integrity/evm/evm_xattrs
> bash: echo: write error: Operation not permitted
This makes sense, as it was already added.
>
> # cat /sys/kernel/security/integrity/evm/evm_xattrs
> security.selinux
> security.ima
> security.capability
> security.foo
>
> # echo . > /sys/kernel/security/integrity/evm/evm_xattrs
> bash: echo: write error: Operation not permitted
I'm still seeing this message.
Mimi
