* [PATCH 01/13] security: Override creds in __fput() with last fputter's creds [ver #4]
From: David Howells @ 2019-06-07 14:17 UTC (permalink / raw)
To: viro
Cc: Casey Schaufler, Casey Schaufler, dhowells, raven, linux-fsdevel,
linux-api, linux-block, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <155991702981.15579.6007568669839441045.stgit@warthog.procyon.org.uk>
So that the LSM can see the credentials of the last process to do an fput()
on a file object when the file object is being dismantled, do the following
steps:
(1) Cache the current credentials in file->f_fput_cred at the point the
file object's reference count reaches zero.
(2) In __fput(), use override_creds() to apply those credentials to the
dismantling process. This is necessary so that if we're dismantling a
unix socket that has semi-passed fds still in it, their fputs will
pick up the same credentials if they're reduced to zero at that point.
Note that it's probably not strictly necessary to take an extra ref on
the creds here (which override_creds() does).
(3) Destroy the fput creds in file_free_rcu().
This additionally makes the creds available to:
fsnotify
eventpoll
file locking
->fasync, ->release file ops
superblock destruction
mountpoint destruction
This allows various notifications about object cleanups/destructions to
carry appropriate credentials for the LSM to approve/disapprove them based
on the process that caused them, even if indirectly.
Note that this means that someone looking at /proc/<pid>/fd/<n> may end up
being inadvertently noted as the subject of a cleanup message if the
process they're looking at croaks whilst they're looking at it.
Further, kernel services like nfsd and cachefiles may be seen as the
fputter and may not have a system credential. In cachefiles's case, it may
appear that cachefilesd caused the notification.
Suggested-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Casey Schaufler <casey@schaufler-ca.com>
---
fs/file_table.c | 12 ++++++++++++
include/linux/fs.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/fs/file_table.c b/fs/file_table.c
index 3f9c1b452c1d..9bf2be45b7f9 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -46,6 +46,7 @@ static void file_free_rcu(struct rcu_head *head)
struct file *f = container_of(head, struct file, f_u.fu_rcuhead);
put_cred(f->f_cred);
+ put_cred(f->f_fput_cred);
kmem_cache_free(filp_cachep, f);
}
@@ -252,6 +253,7 @@ struct file *alloc_file_clone(struct file *base, int flags,
*/
static void __fput(struct file *file)
{
+ const struct cred *saved_cred;
struct dentry *dentry = file->f_path.dentry;
struct vfsmount *mnt = file->f_path.mnt;
struct inode *inode = file->f_inode;
@@ -262,6 +264,12 @@ static void __fput(struct file *file)
might_sleep();
+ /* Set the creds of whoever triggered the last fput for the LSM. Note
+ * that this has to be made available to further fputs, say on fds
+ * trapped in a unix socket.
+ */
+ saved_cred = override_creds(file->f_fput_cred);
+
fsnotify_close(file);
/*
* The function eventpoll_release() should be the first called
@@ -293,6 +301,8 @@ static void __fput(struct file *file)
if (unlikely(mode & FMODE_NEED_UNMOUNT))
dissolve_on_fput(mnt);
mntput(mnt);
+
+ revert_creds(saved_cred);
out:
file_free(file);
}
@@ -334,6 +344,7 @@ void fput_many(struct file *file, unsigned int refs)
if (atomic_long_sub_and_test(refs, &file->f_count)) {
struct task_struct *task = current;
+ file->f_fput_cred = get_current_cred();
if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
init_task_work(&file->f_u.fu_rcuhead, ____fput);
if (!task_work_add(task, &file->f_u.fu_rcuhead, true))
@@ -368,6 +379,7 @@ void __fput_sync(struct file *file)
if (atomic_long_dec_and_test(&file->f_count)) {
struct task_struct *task = current;
BUG_ON(!(task->flags & PF_KTHREAD));
+ file->f_fput_cred = get_current_cred();
__fput(file);
}
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f1c74596cd77..db05738b1951 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -943,6 +943,7 @@ struct file {
loff_t f_pos;
struct fown_struct f_owner;
const struct cred *f_cred;
+ const struct cred *f_fput_cred; /* Who did the last fput() (for LSM) */
struct file_ra_state f_ra;
u64 f_version;
^ permalink raw reply related
* [PATCH 02/13] uapi: General notification ring definitions [ver #4]
From: David Howells @ 2019-06-07 14:17 UTC (permalink / raw)
To: viro
Cc: dhowells, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155991702981.15579.6007568669839441045.stgit@warthog.procyon.org.uk>
Add UAPI definitions for the general notification ring, including the
following pieces:
(1) struct watch_notification.
This is the metadata header for each entry in the ring. It includes a
type and subtype that indicate the source of the message
(eg. WATCH_TYPE_MOUNT_NOTIFY) and the kind of the message
(eg. NOTIFY_MOUNT_NEW_MOUNT).
The header also contains an information field that conveys the
following information:
- WATCH_INFO_LENGTH. The size of the entry (entries are variable
length).
- WATCH_INFO_OVERRUN. If preceding messages were lost due to ring
overrun or lack of memory.
- WATCH_INFO_ENOMEM. If preceding messages were lost due to lack
of memory.
- WATCH_INFO_RECURSIVE. If the event detected was applied to
multiple objects (eg. a recursive change to mount attributes).
- WATCH_INFO_IN_SUBTREE. If the event didn't happen at the watched
object, but rather to some related object (eg. a subtree mount
watch saw a mount happen somewhere within the subtree).
- WATCH_INFO_TYPE_FLAGS. Eight flags whose meanings depend on the
message type.
- WATCH_INFO_ID. The watch ID specified when the watchpoint was
set.
All the information in the header can be used in filtering messages at
the point of writing into the buffer.
(2) struct watch_queue_buffer.
This describes the layout of the ring. Note that the first slots in
the ring contain a special metadata entry that contains the ring
pointers. The producer in the kernel knows to skip this and it has a
proper header (WATCH_TYPE_META, WATCH_META_SKIP_NOTIFICATION) that
indicates the size so that the ring consumer can handle it the same as
any other record and just skip it.
Note that this means that ring entries can never be split over the end
of the ring, so if an entry would need to be split, a skip record is
inserted to wrap the ring first; this is also WATCH_TYPE_META,
WATCH_META_SKIP_NOTIFICATION.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/uapi/linux/watch_queue.h | 63 ++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 include/uapi/linux/watch_queue.h
diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
new file mode 100644
index 000000000000..c3a88fa5f62a
--- /dev/null
+++ b/include/uapi/linux/watch_queue.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_WATCH_QUEUE_H
+#define _UAPI_LINUX_WATCH_QUEUE_H
+
+#include <linux/types.h>
+
+enum watch_notification_type {
+ WATCH_TYPE_META = 0, /* Special record */
+ WATCH_TYPE_MOUNT_NOTIFY = 1, /* Mount notification record */
+ WATCH_TYPE_SB_NOTIFY = 2, /* Superblock notification */
+ WATCH_TYPE_KEY_NOTIFY = 3, /* Key/keyring change notification */
+ WATCH_TYPE_BLOCK_NOTIFY = 4, /* Block layer notifications */
+#define WATCH_TYPE___NR 5
+};
+
+enum watch_meta_notification_subtype {
+ WATCH_META_SKIP_NOTIFICATION = 0, /* Just skip this record */
+ WATCH_META_REMOVAL_NOTIFICATION = 1, /* Watched object was removed */
+};
+
+/*
+ * Notification record
+ */
+struct watch_notification {
+ __u32 type:24; /* enum watch_notification_type */
+ __u32 subtype:8; /* Type-specific subtype (filterable) */
+ __u32 info;
+#define WATCH_INFO_OVERRUN 0x00000001 /* Event(s) lost due to overrun */
+#define WATCH_INFO_ENOMEM 0x00000002 /* Event(s) lost due to ENOMEM */
+#define WATCH_INFO_RECURSIVE 0x00000004 /* Change was recursive */
+#define WATCH_INFO_LENGTH 0x000001f8 /* Length of record / sizeof(watch_notification) */
+#define WATCH_INFO_IN_SUBTREE 0x00000200 /* Change was not at watched root */
+#define WATCH_INFO_TYPE_FLAGS 0x00ff0000 /* Type-specific flags */
+#define WATCH_INFO_FLAG_0 0x00010000
+#define WATCH_INFO_FLAG_1 0x00020000
+#define WATCH_INFO_FLAG_2 0x00040000
+#define WATCH_INFO_FLAG_3 0x00080000
+#define WATCH_INFO_FLAG_4 0x00100000
+#define WATCH_INFO_FLAG_5 0x00200000
+#define WATCH_INFO_FLAG_6 0x00400000
+#define WATCH_INFO_FLAG_7 0x00800000
+#define WATCH_INFO_ID 0xff000000 /* ID of watchpoint */
+#define WATCH_INFO_ID__SHIFT 24
+};
+
+#define WATCH_LENGTH_SHIFT 3
+
+struct watch_queue_buffer {
+ union {
+ /* The first few entries are special, containing the
+ * ring management variables.
+ */
+ struct {
+ struct watch_notification watch; /* WATCH_TYPE_META */
+ __u32 head; /* Ring head index */
+ __u32 tail; /* Ring tail index */
+ __u32 mask; /* Ring index mask */
+ } meta;
+ struct watch_notification slots[0];
+ };
+};
+
+#endif /* _UAPI_LINUX_WATCH_QUEUE_H */
^ permalink raw reply related
* [RFC][PATCH 00/13] Mount, FS, Block and Keyrings notifications [ver #4]
From: David Howells @ 2019-06-07 14:17 UTC (permalink / raw)
To: viro
Cc: linux-usb, linux-security-module, Casey Schaufler,
Stephen Smalley, Greg Kroah-Hartman, dhowells, raven,
linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
Hi Al,
Here's a set of patches to add a general variable-length notification queue
concept and to add sources of events for:
(1) Mount topology events, such as mounting, unmounting, mount expiry,
mount reconfiguration.
(2) Superblock events, such as R/W<->R/O changes, quota overrun and I/O
errors (not complete yet).
(3) Key/keyring events, such as creating, linking and removal of keys.
(4) General device events (single common queue) including:
- Block layer events, such as device errors
- USB subsystem events, such as device/bus attach/remove, device
reset, device errors.
One of the reasons for this is so that we can remove the issue of processes
having to repeatedly and regularly scan /proc/mounts, which has proven to
be a system performance problem. To further aid this, the fsinfo() syscall
on which this patch series depends, provides a way to access superblock and
mount information in binary form without the need to parse /proc/mounts.
LSM support is included, but controversial:
(1) The creds of the process that did the fput() that reduced the refcount
to zero are cached in the file struct.
(2) __fput() overrides the current creds with the creds from (1) whilst
doing the cleanup, thereby making sure that the creds seen by the
destruction notification generated by mntput() appears to come from
the last fputter.
(3) security_post_notification() is called for each queue that we might
want to post a notification into, thereby allowing the LSM to prevent
covert communications.
(?) Do I need to add security_set_watch(), say, to rule on whether a watch
may be set in the first place? I might need to add a variant per
watch-type.
(?) Do I really need to keep track of the process creds in which an
implicit object destruction happened? For example, imagine you create
an fd with fsopen()/fsmount(). It is marked to dissolve the mount it
refers to on close unless move_mount() clears that flag. Now, imagine
someone looking at that fd through procfs at the same time as you exit
due to an error. The LSM sees the destruction notification come from
the looker if they happen to do their fput() after yours.
Design decisions:
(1) A misc chardev is used to create and open a ring buffer:
fd = open("/dev/watch_queue", O_RDWR);
which is then configured and mmap'd into userspace:
ioctl(fd, IOC_WATCH_QUEUE_SET_SIZE, BUF_SIZE);
ioctl(fd, IOC_WATCH_QUEUE_SET_FILTER, &filter);
buf = mmap(NULL, BUF_SIZE * page_size, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, 0);
The fd cannot be read or written (though there is a facility to use
write to inject records for debugging) and userspace just pulls data
directly out of the buffer.
(2) The ring index pointers are stored inside the ring and are thus
accessible to userspace. Userspace should only update the tail
pointer and never the head pointer or risk breaking the buffer. The
kernel checks that the pointers appear valid before trying to use
them. A 'skip' record is maintained around the pointers.
(3) poll() can be used to wait for data to appear in the buffer.
(4) Records in the buffer are binary, typed and have a length so that they
can be of varying size.
This means that multiple heterogeneous sources can share a common
buffer. Tags may be specified when a watchpoint is created to help
distinguish the sources.
(5) The queue is reusable as there are 16 million types available, of
which I've used 4, so there is scope for others to be used.
(6) Records are filterable as types have up to 256 subtypes that can be
individually filtered. Other filtration is also available.
(7) Each time the buffer is opened, a new buffer is created - this means
that there's no interference between watchers.
(8) When recording a notification, the kernel will not sleep, but will
rather mark a queue as overrun if there's insufficient space, thereby
avoiding userspace causing the kernel to hang.
(9) The 'watchpoint' should be specific where possible, meaning that you
specify the object that you want to watch.
(10) The buffer is created and then watchpoints are attached to it, using
one of:
keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fd, 0x01);
mount_notify(AT_FDCWD, "/", 0, fd, 0x02);
sb_notify(AT_FDCWD, "/mnt", 0, fd, 0x03);
where in all three cases, fd indicates the queue and the number after
is a tag between 0 and 255.
(11) The watch must be removed if either the watch buffer is destroyed or
the watched object is destroyed.
Things I want to avoid:
(1) Introducing features that make the core VFS dependent on the network
stack or networking namespaces (ie. usage of netlink).
(2) Dumping all this stuff into dmesg and having a daemon that sits there
parsing the output and distributing it as this then puts the
responsibility for security into userspace and makes handling
namespaces tricky. Further, dmesg might not exist or might be
inaccessible inside a container.
(3) Letting users see events they shouldn't be able to see.
Further things that could be considered:
(1) Adding a keyctl call to allow a watch on a keyring to be extended to
"children" of that keyring, such that the watch is removed from the
child if it is unlinked from the keyring.
(2) Adding global superblock event queue.
(3) Propagating watches to child superblock over automounts.
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
Changes:
v4: Split the basic UAPI bits out into their own patch and then split the
LSM hooks out into an intermediate patch. Add LSM hooks for setting
watches.
Rename the *_notify() system calls to watch_*() for consistency.
v3: I've added a USB notification source and reformulated the block
notification source so that there's now a common watch list, for which
the system call is now device_notify().
I've assigned a pair of unused ioctl numbers in the 'W' series to the
ioctls added by this series.
I've also added a description of the kernel API to the documentation.
v2: I've fixed various issues raised by Jann Horn and GregKH and moved to
krefs for refcounting. I've added some security features to try and
give Casey Schaufler the LSM control he wants.
David
---
David Howells (13):
security: Override creds in __fput() with last fputter's creds
uapi: General notification ring definitions
security: Add hooks to rule on setting a watch
security: Add a hook for the point of notification insertion
General notification queue with user mmap()'able ring buffer
keys: Add a notification facility
vfs: Add a mount-notification facility
vfs: Add superblock notifications
fsinfo: Export superblock notification counter
Add a general, global device notification watch list
block: Add block layer notifications
usb: Add USB subsystem notifications
Add sample notification program
Documentation/ioctl/ioctl-number.txt | 1
Documentation/security/keys/core.rst | 58 ++
Documentation/watch_queue.rst | 492 ++++++++++++++++++
arch/x86/entry/syscalls/syscall_32.tbl | 3
arch/x86/entry/syscalls/syscall_64.tbl | 3
block/Kconfig | 9
block/blk-core.c | 29 +
drivers/base/Kconfig | 9
drivers/base/Makefile | 1
drivers/base/watch.c | 89 +++
drivers/misc/Kconfig | 13
drivers/misc/Makefile | 1
drivers/misc/watch_queue.c | 889 ++++++++++++++++++++++++++++++++
drivers/usb/core/Kconfig | 10
drivers/usb/core/devio.c | 55 ++
drivers/usb/core/hub.c | 3
fs/Kconfig | 21 +
fs/Makefile | 1
fs/file_table.c | 12
fs/fsinfo.c | 12
fs/mount.h | 33 +
fs/mount_notify.c | 187 +++++++
fs/namespace.c | 9
fs/super.c | 122 ++++
include/linux/blkdev.h | 15 +
include/linux/dcache.h | 1
include/linux/device.h | 7
include/linux/fs.h | 79 +++
include/linux/key.h | 4
include/linux/lsm_hooks.h | 48 ++
include/linux/security.h | 35 +
include/linux/syscalls.h | 5
include/linux/usb.h | 19 +
include/linux/watch_queue.h | 87 +++
include/uapi/linux/fsinfo.h | 10
include/uapi/linux/keyctl.h | 1
include/uapi/linux/watch_queue.h | 213 ++++++++
kernel/sys_ni.c | 7
samples/Kconfig | 6
samples/Makefile | 1
samples/vfs/test-fsinfo.c | 13
samples/watch_queue/Makefile | 9
samples/watch_queue/watch_test.c | 308 +++++++++++
security/keys/Kconfig | 10
security/keys/compat.c | 2
security/keys/gc.c | 5
security/keys/internal.h | 30 +
security/keys/key.c | 37 +
security/keys/keyctl.c | 95 +++
security/keys/keyring.c | 17 -
security/keys/request_key.c | 4
security/security.c | 29 +
52 files changed, 3121 insertions(+), 38 deletions(-)
create mode 100644 Documentation/watch_queue.rst
create mode 100644 drivers/base/watch.c
create mode 100644 drivers/misc/watch_queue.c
create mode 100644 fs/mount_notify.c
create mode 100644 include/linux/watch_queue.h
create mode 100644 include/uapi/linux/watch_queue.h
create mode 100644 samples/watch_queue/Makefile
create mode 100644 samples/watch_queue/watch_test.c
^ permalink raw reply
* Re: [PATCH 09/10] usb: Add USB subsystem notifications [ver #3]
From: Alan Stern @ 2019-06-07 14:01 UTC (permalink / raw)
To: Felipe Balbi
Cc: Greg Kroah-Hartman, David Howells, viro, linux-usb, raven,
linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <87imthdhjb.fsf@linux.intel.com>
On Fri, 7 Jun 2019, Felipe Balbi wrote:
> Hi,
>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > On Thu, Jun 06, 2019 at 10:55:24AM -0400, Alan Stern wrote:
> >> On Thu, 6 Jun 2019, Greg Kroah-Hartman wrote:
> >>
> >> > On Thu, Jun 06, 2019 at 10:24:18AM -0400, Alan Stern wrote:
> >> > > On Thu, 6 Jun 2019, David Howells wrote:
> >> > >
> >> > > > Add a USB subsystem notification mechanism whereby notifications about
> >> > > > hardware events such as device connection, disconnection, reset and I/O
> >> > > > errors, can be reported to a monitoring process asynchronously.
> >> > >
> >> > > USB I/O errors covers an awfully large and vague field. Do we really
> >> > > want to include them? I'm doubtful.
> >> >
> >> > See the other patch on the linux-usb list that wanted to start adding
> >> > KOBJ_CHANGE notifications about USB "i/o errors".
> >>
> >> That patch wanted to add notifications only for enumeration failures
> >> (assuming you're talking about the patch from Eugeniu Rosca), not I/O
> >> errors in general.
> >
> > Yes, sorry, I was thinking that as a "I/O failed in enumeration" :)
> >
> >> > So for "severe" issues, yes, we should do this, but perhaps not for all
> >> > of the "normal" things we see when a device is yanked out of the system
> >> > and the like.
> >>
> >> Then what counts as a "severe" issue? Anything besides enumeration
> >> failure?
> >
> > Not that I can think of at the moment, other than the other recently
> > added KOBJ_CHANGE issue. I'm sure we have other "hard failure" issues
> > in the USB stack that people will want exposed over time.
>
> From an XHCI standpoint, Transaction Errors might be one thing. They
> happen rarely and are a strong indication that the bus itself is
> bad. Either bad cable, misbehaving PHYs, improper power management, etc.
Don't you also get transaction errors if the user unplugs a device in
the middle of a transfer? That's not the sort of thing we want to sent
notifications about.
Alan Stern
^ permalink raw reply
* Re: [PATCH 00/58] LSM: Module stacking for AppArmor
From: José Bollo @ 2019-06-07 13:03 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux, keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <66a87b0b-b6f4-74ff-2e51-afc8e2d30de1@schaufler-ca.com>
On Tue, 4 Jun 2019 09:14:42 -0700
Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
> > On 6/2/19 12:50 PM, Casey Schaufler wrote:
> >> This patchset provides the changes required for
> >> the AppArmor security module to stack safely with any other.
> >
> > Please explain the motivation
>
> I'll add some explanation for the next revision.
> It won't be anything that I haven't posted many times
> before, but you're right that it belongs in the log.
>
> > - why do we want to allow AppArmor to stack with other modules,
>
> First, is there a reason not to? Sure, you can confuse
> administrators by implementing complex security policies,
> but there are lots of ways to do that already.
>
> AppArmor provides a different security model than SELinux,
> TOMOYO or Smack. Smack is better at system component
> separation, while AppArmor is better at application isolation.
> It's a win to use each to its strength rather than trying to
> stretch either to the edge of what it can do.
>
> > who would use it,
Hi all,
I would like to expose a potential use of interest for me: being able
to have containers running Smack on Ubuntu or Fedora platforms.
But it could also be interesting for running a container having fedora
on ubuntu or suse or the opposite.
How it will work? Will it work? Ask Casey.
just my 2 pennies
José Bollo
> Can't name names, but there have been multiple requests.
>
> > how would it be used,
>
> As mentioned above, Smack for system separation, AppArmor for
> application isolation.
>
> > what does it provide that isn't already possible in the absence of
> > it.
>
> It's not necessary that something be impossible to do any
> other way. The question should be whether this provides for
> a better way to achieve the goals, and this does that.
> If I tried the come up with something that's impossible I
> would expect the usual "you can do that with SELinux policy"
> argument. We know we can do things. We want to have the tools
> to do them better.
>
> > Also, Ubuntu fully upstreamed all of their changes to AppArmor,
> > would this still suffice to enable stacking of AppArmor or do they
> > rely on hooks that are not handled here?
>
> Some amount of merging will likely be required. But that's
> always going to be true with parallel development tracks.
> That's why we have git!
>
> > Please explain the cost of the change - what do we pay in terms of
> > memory, runtime, or other overheads in order to support this
> > change?
>
> Do you have particular benchmarks you want to see?
> When I've supplied numbers in the past they have not
> been remarked on.
>
> >
> >>
> >> A new process attribute identifies which security module
> >> information should be reported by SO_PEERSEC and the
> >> /proc/.../attr/current interface. This is provided by
> >> /proc/.../attr/display. Writing the name of the security
> >> module desired to this interface will set which LSM hooks
> >> will be called for this information. The first security
> >> module providing the hooks will be used by default.
> >
> > Doesn't this effectively undo making the hooks read-only after
> > init, at least for the subset involved? What are the security
> > implications thereof?
>
> Any mechanism, be it a separate set of hooks, a name used to
> do list look ups, or an sophisticated hash scheme will have that
> impact for the processes that use it. This scheme has the best
> performance profile of the mechanisms I experimented with and
> avoids all sorts of special cases.
>
> >
> >> The use of integer based security tokens (secids) is
> >> generally (but not completely) replaced by a structure
> >> lsm_export. The lsm_export structure can contain information
> >> for each of the security modules that export information
> >> outside the LSM layer.
> >>
> >> The LSM interfaces that provide "secctx" text strings
> >> have been changed to use a structure "lsm_context"
> >> instead of a pointer/length pair. In some cases the
> >> interfaces used a "char *" pointer and in others a
> >> "void *". This was necessary to ensure that the correct
> >> release mechanism for the text is used. It also makes
> >> many of the interfaces cleaner.
> >>
> >> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> drivers/android/binder.c | 25 ++-
> >> fs/kernfs/dir.c | 6 +-
> >> fs/kernfs/inode.c | 31 ++-
> >> fs/kernfs/kernfs-internal.h | 3 +-
> >> fs/nfs/inode.c | 13 +-
> >> fs/nfs/internal.h | 8 +-
> >> fs/nfs/nfs4proc.c | 17 +-
> >> fs/nfs/nfs4xdr.c | 16 +-
> >> fs/nfsd/nfs4proc.c | 8 +-
> >> fs/nfsd/nfs4xdr.c | 14 +-
> >> fs/nfsd/vfs.c | 7 +-
> >> fs/proc/base.c | 1 +
> >> include/linux/cred.h | 3 +-
> >> include/linux/lsm_hooks.h | 91 +++++----
> >> include/linux/nfs4.h | 8 +-
> >> include/linux/security.h | 133 +++++++++----
> >> include/net/af_unix.h | 2 +-
> >> include/net/netlabel.h | 10 +-
> >> include/net/scm.h | 14 +-
> >> kernel/audit.c | 43 ++--
> >> kernel/audit.h | 9 +-
> >> kernel/auditfilter.c | 6 +-
> >> kernel/auditsc.c | 77 ++++----
> >> kernel/cred.c | 15 +-
> >> net/ipv4/cipso_ipv4.c | 13 +-
> >> net/ipv4/ip_sockglue.c | 12 +-
> >> net/netfilter/nf_conntrack_netlink.c | 29 ++-
> >> net/netfilter/nf_conntrack_standalone.c | 16 +-
> >> net/netfilter/nfnetlink_queue.c | 38 ++--
> >> net/netfilter/nft_meta.c | 13 +-
> >> net/netfilter/xt_SECMARK.c | 14 +-
> >> net/netlabel/netlabel_kapi.c | 5 +-
> >> net/netlabel/netlabel_unlabeled.c | 101 +++++-----
> >> net/netlabel/netlabel_unlabeled.h | 2 +-
> >> net/netlabel/netlabel_user.c | 13 +-
> >> net/netlabel/netlabel_user.h | 2 +-
> >> net/unix/af_unix.c | 6 +-
> >> security/apparmor/audit.c | 4 +-
> >> security/apparmor/include/audit.h | 2 +-
> >> security/apparmor/include/net.h | 6 +-
> >> security/apparmor/include/secid.h | 9 +-
> >> security/apparmor/lsm.c | 64 +++---
> >> security/apparmor/secid.c | 42 ++--
> >> security/integrity/ima/ima.h | 14 +-
> >> security/integrity/ima/ima_api.c | 9 +-
> >> security/integrity/ima/ima_appraise.c | 6 +-
> >> security/integrity/ima/ima_main.c | 34 ++--
> >> security/integrity/ima/ima_policy.c | 19 +-
> >> security/security.c | 338
> >> +++++++++++++++++++++++++++-----
> >> security/selinux/hooks.c | 259
> >> ++++++++++++------------ security/selinux/include/audit.h
> >> | 5 +- security/selinux/include/objsec.h | 42 +++-
> >> security/selinux/netlabel.c | 25 +--
> >> security/selinux/ss/services.c | 18 +-
> >> security/smack/smack.h | 18 ++
> >> security/smack/smack_lsm.c | 238
> >> +++++++++++----------- security/smack/smack_netfilter.c |
> >> 8 +- security/smack/smackfs.c | 12 +- 58 files
> >> changed, 1217 insertions(+), 779 deletions(-)
> >
^ permalink raw reply
* [PATCH v6 3/3] lib: introduce test_meminit module
From: Alexander Potapenko @ 2019-06-06 16:48 UTC (permalink / raw)
To: Kees Cook, Andrew Morton, Christoph Lameter
Cc: Alexander Potapenko, Nick Desaulniers, Kostya Serebryany,
Dmitry Vyukov, Sandeep Patil, Laura Abbott, Jann Horn,
Marco Elver, linux-mm, linux-security-module, kernel-hardening
In-Reply-To: <20190606164845.179427-1-glider@google.com>
Add tests for heap and pagealloc initialization.
These can be used to check init_on_alloc and init_on_free implementations
as well as other approaches to initialization.
Expected test output in the case the kernel provides heap initialization
(e.g. when running with either init_on_alloc=1 or init_on_free=1):
test_meminit: all 10 tests in test_pages passed
test_meminit: all 40 tests in test_kvmalloc passed
test_meminit: all 60 tests in test_kmemcache passed
test_meminit: all 10 tests in test_rcu_persistent passed
test_meminit: all 120 tests passed!
Signed-off-by: Alexander Potapenko <glider@google.com>
To: Kees Cook <keescook@chromium.org>
To: Andrew Morton <akpm@linux-foundation.org>
To: Christoph Lameter <cl@linux.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-mm@kvack.org
Cc: linux-security-module@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
v3:
- added example test output to the description
- fixed a missing include spotted by kbuild test robot <lkp@intel.com>
- added a missing MODULE_LICENSE
- call do_kmem_cache_size() with size >= sizeof(void*) to unbreak
debug builds
v5:
- added tests for RCU slabs and __GFP_ZERO
---
lib/Kconfig.debug | 8 +
lib/Makefile | 1 +
lib/test_meminit.c | 362 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 lib/test_meminit.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cbdfae379896..085711f14abf 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2040,6 +2040,14 @@ config TEST_STACKINIT
If unsure, say N.
+config TEST_MEMINIT
+ tristate "Test heap/page initialization"
+ help
+ Test if the kernel is zero-initializing heap and page allocations.
+ This can be useful to test init_on_alloc and init_on_free features.
+
+ If unsure, say N.
+
endif # RUNTIME_TESTING_MENU
config MEMTEST
diff --git a/lib/Makefile b/lib/Makefile
index fb7697031a79..05980c802500 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
+obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
obj-$(CONFIG_TEST_LIVEPATCH) += livepatch/
diff --git a/lib/test_meminit.c b/lib/test_meminit.c
new file mode 100644
index 000000000000..ed7efec1387b
--- /dev/null
+++ b/lib/test_meminit.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test cases for SL[AOU]B/page initialization at alloc/free time.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/vmalloc.h>
+
+#define GARBAGE_INT (0x09A7BA9E)
+#define GARBAGE_BYTE (0x9E)
+
+#define REPORT_FAILURES_IN_FN() \
+ do { \
+ if (failures) \
+ pr_info("%s failed %d out of %d times\n", \
+ __func__, failures, num_tests); \
+ else \
+ pr_info("all %d tests in %s passed\n", \
+ num_tests, __func__); \
+ } while (0)
+
+/* Calculate the number of uninitialized bytes in the buffer. */
+static int __init count_nonzero_bytes(void *ptr, size_t size)
+{
+ int i, ret = 0;
+ unsigned char *p = (unsigned char *)ptr;
+
+ for (i = 0; i < size; i++)
+ if (p[i])
+ ret++;
+ return ret;
+}
+
+/* Fill a buffer with garbage, skipping |skip| first bytes. */
+static void __init fill_with_garbage_skip(void *ptr, size_t size, size_t skip)
+{
+ unsigned int *p = (unsigned int *)ptr;
+ int i = 0;
+
+ if (skip) {
+ WARN_ON(skip > size);
+ p += skip;
+ }
+ while (size >= sizeof(*p)) {
+ p[i] = GARBAGE_INT;
+ i++;
+ size -= sizeof(*p);
+ }
+ if (size)
+ memset(&p[i], GARBAGE_BYTE, size);
+}
+
+static void __init fill_with_garbage(void *ptr, size_t size)
+{
+ fill_with_garbage_skip(ptr, size, 0);
+}
+
+static int __init do_alloc_pages_order(int order, int *total_failures)
+{
+ struct page *page;
+ void *buf;
+ size_t size = PAGE_SIZE << order;
+
+ page = alloc_pages(GFP_KERNEL, order);
+ buf = page_address(page);
+ fill_with_garbage(buf, size);
+ __free_pages(page, order);
+
+ page = alloc_pages(GFP_KERNEL, order);
+ buf = page_address(page);
+ if (count_nonzero_bytes(buf, size))
+ (*total_failures)++;
+ fill_with_garbage(buf, size);
+ __free_pages(page, order);
+ return 1;
+}
+
+/* Test the page allocator by calling alloc_pages with different orders. */
+static int __init test_pages(int *total_failures)
+{
+ int failures = 0, num_tests = 0;
+ int i;
+
+ for (i = 0; i < 10; i++)
+ num_tests += do_alloc_pages_order(i, &failures);
+
+ REPORT_FAILURES_IN_FN();
+ *total_failures += failures;
+ return num_tests;
+}
+
+/* Test kmalloc() with given parameters. */
+static int __init do_kmalloc_size(size_t size, int *total_failures)
+{
+ void *buf;
+
+ buf = kmalloc(size, GFP_KERNEL);
+ fill_with_garbage(buf, size);
+ kfree(buf);
+
+ buf = kmalloc(size, GFP_KERNEL);
+ if (count_nonzero_bytes(buf, size))
+ (*total_failures)++;
+ fill_with_garbage(buf, size);
+ kfree(buf);
+ return 1;
+}
+
+/* Test vmalloc() with given parameters. */
+static int __init do_vmalloc_size(size_t size, int *total_failures)
+{
+ void *buf;
+
+ buf = vmalloc(size);
+ fill_with_garbage(buf, size);
+ vfree(buf);
+
+ buf = vmalloc(size);
+ if (count_nonzero_bytes(buf, size))
+ (*total_failures)++;
+ fill_with_garbage(buf, size);
+ vfree(buf);
+ return 1;
+}
+
+/* Test kmalloc()/vmalloc() by allocating objects of different sizes. */
+static int __init test_kvmalloc(int *total_failures)
+{
+ int failures = 0, num_tests = 0;
+ int i, size;
+
+ for (i = 0; i < 20; i++) {
+ size = 1 << i;
+ num_tests += do_kmalloc_size(size, &failures);
+ num_tests += do_vmalloc_size(size, &failures);
+ }
+
+ REPORT_FAILURES_IN_FN();
+ *total_failures += failures;
+ return num_tests;
+}
+
+#define CTOR_BYTES (sizeof(unsigned int))
+#define CTOR_PATTERN (0x41414141)
+/* Initialize the first 4 bytes of the object. */
+static void test_ctor(void *obj)
+{
+ *(unsigned int *)obj = CTOR_PATTERN;
+}
+
+/*
+ * Check the invariants for the buffer allocated from a slab cache.
+ * If the cache has a test constructor, the first 4 bytes of the object must
+ * always remain equal to CTOR_PATTERN.
+ * If the cache isn't an RCU-typesafe one, or if the allocation is done with
+ * __GFP_ZERO, then the object contents must be zeroed after allocation.
+ * If the cache is an RCU-typesafe one, the object contents must never be
+ * zeroed after the first use. This is checked by memcmp() in
+ * do_kmem_cache_size().
+ */
+static bool __init check_buf(void *buf, int size, bool want_ctor,
+ bool want_rcu, bool want_zero)
+{
+ int bytes;
+ bool fail = false;
+
+ bytes = count_nonzero_bytes(buf, size);
+ WARN_ON(want_ctor && want_zero);
+ if (want_zero)
+ return bytes;
+ if (want_ctor) {
+ if (*(unsigned int *)buf != CTOR_PATTERN)
+ fail = 1;
+ } else {
+ if (bytes)
+ fail = !want_rcu;
+ }
+ return fail;
+}
+
+/*
+ * Test kmem_cache with given parameters:
+ * want_ctor - use a constructor;
+ * want_rcu - use SLAB_TYPESAFE_BY_RCU;
+ * want_zero - use __GFP_ZERO.
+ */
+static int __init do_kmem_cache_size(size_t size, bool want_ctor,
+ bool want_rcu, bool want_zero,
+ int *total_failures)
+{
+ struct kmem_cache *c;
+ int iter;
+ bool fail = false;
+ gfp_t alloc_mask = GFP_KERNEL | (want_zero ? __GFP_ZERO : 0);
+ void *buf, *buf_copy;
+
+ c = kmem_cache_create("test_cache", size, 1,
+ want_rcu ? SLAB_TYPESAFE_BY_RCU : 0,
+ want_ctor ? test_ctor : NULL);
+ for (iter = 0; iter < 10; iter++) {
+ buf = kmem_cache_alloc(c, alloc_mask);
+ /* Check that buf is zeroed, if it must be. */
+ fail = check_buf(buf, size, want_ctor, want_rcu, want_zero);
+ fill_with_garbage_skip(buf, size, want_ctor ? CTOR_BYTES : 0);
+ /*
+ * If this is an RCU cache, use a critical section to ensure we
+ * can touch objects after they're freed.
+ */
+ if (want_rcu) {
+ rcu_read_lock();
+ /*
+ * Copy the buffer to check that it's not wiped on
+ * free().
+ */
+ buf_copy = kmalloc(size, GFP_KERNEL);
+ if (buf_copy)
+ memcpy(buf_copy, buf, size);
+ }
+ kmem_cache_free(c, buf);
+ if (want_rcu) {
+ /*
+ * Check that |buf| is intact after kmem_cache_free().
+ * |want_zero| is false, because we wrote garbage to
+ * the buffer already.
+ */
+ fail |= check_buf(buf, size, want_ctor, want_rcu,
+ false);
+ if (buf_copy) {
+ fail |= (bool)memcmp(buf, buf_copy, size);
+ kfree(buf_copy);
+ }
+ rcu_read_unlock();
+ }
+ }
+ kmem_cache_destroy(c);
+
+ *total_failures += fail;
+ return 1;
+}
+
+/*
+ * Check that the data written to an RCU-allocated object survives
+ * reallocation.
+ */
+static int __init do_kmem_cache_rcu_persistent(int size, int *total_failures)
+{
+ struct kmem_cache *c;
+ void *buf, *buf_contents, *saved_ptr;
+ void **used_objects;
+ int i, iter, maxiter = 1024;
+ bool fail = false;
+
+ c = kmem_cache_create("test_cache", size, size, SLAB_TYPESAFE_BY_RCU,
+ NULL);
+ buf = kmem_cache_alloc(c, GFP_KERNEL);
+ saved_ptr = buf;
+ fill_with_garbage(buf, size);
+ buf_contents = kmalloc(size, GFP_KERNEL);
+ if (!buf_contents)
+ goto out;
+ used_objects = kmalloc_array(maxiter, sizeof(void *), GFP_KERNEL);
+ if (!used_objects) {
+ kfree(buf_contents);
+ goto out;
+ }
+ memcpy(buf_contents, buf, size);
+ kmem_cache_free(c, buf);
+ /*
+ * Run for a fixed number of iterations. If we never hit saved_ptr,
+ * assume the test passes.
+ */
+ for (iter = 0; iter < maxiter; iter++) {
+ buf = kmem_cache_alloc(c, GFP_KERNEL);
+ used_objects[iter] = buf;
+ if (buf == saved_ptr) {
+ fail = memcmp(buf_contents, buf, size);
+ for (i = 0; i <= iter; i++)
+ kmem_cache_free(c, used_objects[i]);
+ goto free_out;
+ }
+ }
+
+free_out:
+ kmem_cache_destroy(c);
+ kfree(buf_contents);
+ kfree(used_objects);
+out:
+ *total_failures += fail;
+ return 1;
+}
+
+/*
+ * Test kmem_cache allocation by creating caches of different sizes, with and
+ * without constructors, with and without SLAB_TYPESAFE_BY_RCU.
+ */
+static int __init test_kmemcache(int *total_failures)
+{
+ int failures = 0, num_tests = 0;
+ int i, flags, size;
+ bool ctor, rcu, zero;
+
+ for (i = 0; i < 10; i++) {
+ size = 8 << i;
+ for (flags = 0; flags < 8; flags++) {
+ ctor = flags & 1;
+ rcu = flags & 2;
+ zero = flags & 4;
+ if (ctor & zero)
+ continue;
+ num_tests += do_kmem_cache_size(size, ctor, rcu, zero,
+ &failures);
+ }
+ }
+ REPORT_FAILURES_IN_FN();
+ *total_failures += failures;
+ return num_tests;
+}
+
+/* Test the behavior of SLAB_TYPESAFE_BY_RCU caches of different sizes. */
+static int __init test_rcu_persistent(int *total_failures)
+{
+ int failures = 0, num_tests = 0;
+ int i, size;
+
+ for (i = 0; i < 10; i++) {
+ size = 8 << i;
+ num_tests += do_kmem_cache_rcu_persistent(size, &failures);
+ }
+ REPORT_FAILURES_IN_FN();
+ *total_failures += failures;
+ return num_tests;
+}
+
+/*
+ * Run the tests. Each test function returns the number of executed tests and
+ * updates |failures| with the number of failed tests.
+ */
+static int __init test_meminit_init(void)
+{
+ int failures = 0, num_tests = 0;
+
+ num_tests += test_pages(&failures);
+ num_tests += test_kvmalloc(&failures);
+ num_tests += test_kmemcache(&failures);
+ num_tests += test_rcu_persistent(&failures);
+
+ if (failures == 0)
+ pr_info("all %d tests passed!\n", num_tests);
+ else
+ pr_info("failures: %d out of %d\n", failures, num_tests);
+
+ return failures ? -EINVAL : 0;
+}
+module_init(test_meminit_init);
+
+MODULE_LICENSE("GPL");
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related
* [PATCH v6 2/3] mm: init: report memory auto-initialization features at boot time
From: Alexander Potapenko @ 2019-06-06 16:48 UTC (permalink / raw)
To: Andrew Morton, Christoph Lameter
Cc: Alexander Potapenko, Kees Cook, Dmitry Vyukov, James Morris,
Jann Horn, Kostya Serebryany, Laura Abbott, Mark Rutland,
Masahiro Yamada, Matthew Wilcox, Nick Desaulniers, Randy Dunlap,
Sandeep Patil, Serge E. Hallyn, Souptick Joarder, Marco Elver,
Kaiwan N Billimoria, kernel-hardening, linux-mm,
linux-security-module
In-Reply-To: <20190606164845.179427-1-glider@google.com>
Print the currently enabled stack and heap initialization modes.
Stack initialization is enabled by a config flag, while heap
initialization is configured at boot time with defaults being set
in the config. It's more convenient for the user to have all information
about these hardening measures in one place.
The possible options for stack are:
- "all" for CONFIG_INIT_STACK_ALL;
- "byref_all" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL;
- "byref" for CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF;
- "__user" for CONFIG_GCC_PLUGIN_STRUCTLEAK_USER;
- "off" otherwise.
Depending on the values of init_on_alloc and init_on_free boottime
options we also report "heap alloc" and "heap free" as "on"/"off".
In the init_on_free mode initializing pages at boot time may take some
time, so print a notice about that as well.
Signed-off-by: Alexander Potapenko <glider@google.com>
Suggested-by: Kees Cook <keescook@chromium.org>
To: Andrew Morton <akpm@linux-foundation.org>
To: Christoph Lameter <cl@linux.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Souptick Joarder <jrdr.linux@gmail.com>
Cc: Marco Elver <elver@google.com>
Cc: Kaiwan N Billimoria <kaiwan@kaiwantech.com>
Cc: kernel-hardening@lists.openwall.com
Cc: linux-mm@kvack.org
Cc: linux-security-module@vger.kernel.org
---
v6:
- update patch description, fixed message about clearing memory
---
init/main.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/init/main.c b/init/main.c
index 66a196c5e4c3..e68ef1f181f9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -520,6 +520,29 @@ static inline void initcall_debug_enable(void)
}
#endif
+/* Report memory auto-initialization states for this boot. */
+void __init report_meminit(void)
+{
+ const char *stack;
+
+ if (IS_ENABLED(CONFIG_INIT_STACK_ALL))
+ stack = "all";
+ else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL))
+ stack = "byref_all";
+ else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF))
+ stack = "byref";
+ else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_USER))
+ stack = "__user";
+ else
+ stack = "off";
+
+ pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s\n",
+ stack, want_init_on_alloc(GFP_KERNEL) ? "on" : "off",
+ want_init_on_free() ? "on" : "off");
+ if (want_init_on_free())
+ pr_info("mem auto-init: clearing system memory may take some time...\n");
+}
+
/*
* Set up kernel memory allocators
*/
@@ -530,6 +553,7 @@ static void __init mm_init(void)
* bigger than MAX_ORDER unless SPARSEMEM.
*/
page_ext_init_flatmem();
+ report_meminit();
mem_init();
kmem_cache_init();
pgtable_init();
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related
* [PATCH v6 1/3] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options
From: Alexander Potapenko @ 2019-06-06 16:48 UTC (permalink / raw)
To: Andrew Morton, Christoph Lameter, Kees Cook
Cc: Alexander Potapenko, Masahiro Yamada, Michal Hocko, James Morris,
Serge E. Hallyn, Nick Desaulniers, Kostya Serebryany,
Dmitry Vyukov, Sandeep Patil, Laura Abbott, Randy Dunlap,
Jann Horn, Mark Rutland, Marco Elver, linux-mm,
linux-security-module, kernel-hardening
In-Reply-To: <20190606164845.179427-1-glider@google.com>
The new options are needed to prevent possible information leaks and
make control-flow bugs that depend on uninitialized values more
deterministic.
init_on_alloc=1 makes the kernel initialize newly allocated pages and heap
objects with zeroes. Initialization is done at allocation time at the
places where checks for __GFP_ZERO are performed.
init_on_free=1 makes the kernel initialize freed pages and heap objects
with zeroes upon their deletion. This helps to ensure sensitive data
doesn't leak via use-after-free accesses.
Both init_on_alloc=1 and init_on_free=1 guarantee that the allocator
returns zeroed memory. The two exceptions are slab caches with
constructors and SLAB_TYPESAFE_BY_RCU flag. Those are never
zero-initialized to preserve their semantics.
Both init_on_alloc and init_on_free default to zero, but those defaults
can be overridden with CONFIG_INIT_ON_ALLOC_DEFAULT_ON and
CONFIG_INIT_ON_FREE_DEFAULT_ON.
Slowdown for the new features compared to init_on_free=0,
init_on_alloc=0:
hackbench, init_on_free=1: +7.62% sys time (st.err 0.74%)
hackbench, init_on_alloc=1: +7.75% sys time (st.err 2.14%)
Linux build with -j12, init_on_free=1: +8.38% wall time (st.err 0.39%)
Linux build with -j12, init_on_free=1: +24.42% sys time (st.err 0.52%)
Linux build with -j12, init_on_alloc=1: -0.13% wall time (st.err 0.42%)
Linux build with -j12, init_on_alloc=1: +0.57% sys time (st.err 0.40%)
The slowdown for init_on_free=0, init_on_alloc=0 compared to the
baseline is within the standard error.
The new features are also going to pave the way for hardware memory
tagging (e.g. arm64's MTE), which will require both on_alloc and on_free
hooks to set the tags for heap objects. With MTE, tagging will have the
same cost as memory initialization.
Although init_on_free is rather costly, there are paranoid use-cases where
in-memory data lifetime is desired to be minimized. There are various
arguments for/against the realism of the associated threat models, but
given that we'll need the infrastructre for MTE anyway, and there are
people who want wipe-on-free behavior no matter what the performance cost,
it seems reasonable to include it in this series.
Signed-off-by: Alexander Potapenko <glider@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
To: Christoph Lameter <cl@linux.com>
To: Kees Cook <keescook@chromium.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Jann Horn <jannh@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-mm@kvack.org
Cc: linux-security-module@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
v2:
- unconditionally initialize pages in kernel_init_free_pages()
- comment from Randy Dunlap: drop 'default false' lines from Kconfig.hardening
v3:
- don't call kernel_init_free_pages() from memblock_free_pages()
- adopted some Kees' comments for the patch description
v4:
- use NULL instead of 0 in slab_alloc_node() (found by kbuild test robot)
- don't write to NULL object in slab_alloc_node() (found by Android
testing)
v5:
- adjusted documentation wording as suggested by Kees
- disable SLAB_POISON if auto-initialization is on
- don't wipe RCU cache allocations made without __GFP_ZERO
- dropped SLOB support
---
.../admin-guide/kernel-parameters.txt | 9 +++
drivers/infiniband/core/uverbs_ioctl.c | 2 +-
include/linux/mm.h | 22 +++++++
kernel/kexec_core.c | 2 +-
mm/dmapool.c | 2 +-
mm/page_alloc.c | 63 ++++++++++++++++---
mm/slab.c | 16 ++++-
mm/slab.h | 19 ++++++
mm/slub.c | 33 ++++++++--
net/core/sock.c | 2 +-
security/Kconfig.hardening | 29 +++++++++
11 files changed, 180 insertions(+), 19 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 138f6664b2e2..84ee1121a2b9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1673,6 +1673,15 @@
initrd= [BOOT] Specify the location of the initial ramdisk
+ init_on_alloc= [MM] Fill newly allocated pages and heap objects with
+ zeroes.
+ Format: 0 | 1
+ Default set by CONFIG_INIT_ON_ALLOC_DEFAULT_ON.
+
+ init_on_free= [MM] Fill freed pages and heap objects with zeroes.
+ Format: 0 | 1
+ Default set by CONFIG_INIT_ON_FREE_DEFAULT_ON.
+
init_pkru= [x86] Specify the default memory protection keys rights
register contents for all processes. 0x55555554 by
default (disallow access to all but pkey 0). Can
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index 829b0c6944d8..61758201d9b2 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -127,7 +127,7 @@ __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
res = (void *)pbundle->internal_buffer + pbundle->internal_used;
pbundle->internal_used =
ALIGN(new_used, sizeof(*pbundle->internal_buffer));
- if (flags & __GFP_ZERO)
+ if (want_init_on_alloc(flags))
memset(res, 0, size);
return res;
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834ac32b7..7733a341c0c4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2685,6 +2685,28 @@ static inline void kernel_poison_pages(struct page *page, int numpages,
int enable) { }
#endif
+#ifdef CONFIG_INIT_ON_ALLOC_DEFAULT_ON
+DECLARE_STATIC_KEY_TRUE(init_on_alloc);
+#else
+DECLARE_STATIC_KEY_FALSE(init_on_alloc);
+#endif
+static inline bool want_init_on_alloc(gfp_t flags)
+{
+ if (static_branch_unlikely(&init_on_alloc))
+ return true;
+ return flags & __GFP_ZERO;
+}
+
+#ifdef CONFIG_INIT_ON_FREE_DEFAULT_ON
+DECLARE_STATIC_KEY_TRUE(init_on_free);
+#else
+DECLARE_STATIC_KEY_FALSE(init_on_free);
+#endif
+static inline bool want_init_on_free(void)
+{
+ return static_branch_unlikely(&init_on_free);
+}
+
extern bool _debug_pagealloc_enabled;
static inline bool debug_pagealloc_enabled(void)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index fd5c95ff9251..2f75dd0d0d81 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -315,7 +315,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
arch_kexec_post_alloc_pages(page_address(pages), count,
gfp_mask);
- if (gfp_mask & __GFP_ZERO)
+ if (want_init_on_alloc(gfp_mask))
for (i = 0; i < count; i++)
clear_highpage(pages + i);
}
diff --git a/mm/dmapool.c b/mm/dmapool.c
index 76a160083506..493d151067cb 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -381,7 +381,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
#endif
spin_unlock_irqrestore(&pool->lock, flags);
- if (mem_flags & __GFP_ZERO)
+ if (want_init_on_alloc(mem_flags))
memset(retval, 0, pool->size);
return retval;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d66bc8abe0af..50a3b104a491 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -136,6 +136,48 @@ unsigned long totalcma_pages __read_mostly;
int percpu_pagelist_fraction;
gfp_t gfp_allowed_mask __read_mostly = GFP_BOOT_MASK;
+#ifdef CONFIG_INIT_ON_ALLOC_DEFAULT_ON
+DEFINE_STATIC_KEY_TRUE(init_on_alloc);
+#else
+DEFINE_STATIC_KEY_FALSE(init_on_alloc);
+#endif
+#ifdef CONFIG_INIT_ON_FREE_DEFAULT_ON
+DEFINE_STATIC_KEY_TRUE(init_on_free);
+#else
+DEFINE_STATIC_KEY_FALSE(init_on_free);
+#endif
+
+static int __init early_init_on_alloc(char *buf)
+{
+ int ret;
+ bool bool_result;
+
+ if (!buf)
+ return -EINVAL;
+ ret = kstrtobool(buf, &bool_result);
+ if (bool_result)
+ static_branch_enable(&init_on_alloc);
+ else
+ static_branch_disable(&init_on_alloc);
+ return ret;
+}
+early_param("init_on_alloc", early_init_on_alloc);
+
+static int __init early_init_on_free(char *buf)
+{
+ int ret;
+ bool bool_result;
+
+ if (!buf)
+ return -EINVAL;
+ ret = kstrtobool(buf, &bool_result);
+ if (bool_result)
+ static_branch_enable(&init_on_free);
+ else
+ static_branch_disable(&init_on_free);
+ return ret;
+}
+early_param("init_on_free", early_init_on_free);
/*
* A cached value of the page's pageblock's migratetype, used when the page is
@@ -1090,6 +1132,14 @@ static int free_tail_pages_check(struct page *head_page, struct page *page)
return ret;
}
+static void kernel_init_free_pages(struct page *page, int numpages)
+{
+ int i;
+
+ for (i = 0; i < numpages; i++)
+ clear_highpage(page + i);
+}
+
static __always_inline bool free_pages_prepare(struct page *page,
unsigned int order, bool check_free)
{
@@ -1142,6 +1192,8 @@ static __always_inline bool free_pages_prepare(struct page *page,
}
arch_free_page(page, order);
kernel_poison_pages(page, 1 << order, 0);
+ if (want_init_on_free())
+ kernel_init_free_pages(page, 1 << order);
if (debug_pagealloc_enabled())
kernel_map_pages(page, 1 << order, 0);
@@ -2020,8 +2072,8 @@ static inline int check_new_page(struct page *page)
static inline bool free_pages_prezeroed(void)
{
- return IS_ENABLED(CONFIG_PAGE_POISONING_ZERO) &&
- page_poisoning_enabled();
+ return (IS_ENABLED(CONFIG_PAGE_POISONING_ZERO) &&
+ page_poisoning_enabled()) || want_init_on_free();
}
#ifdef CONFIG_DEBUG_VM
@@ -2075,13 +2127,10 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
unsigned int alloc_flags)
{
- int i;
-
post_alloc_hook(page, order, gfp_flags);
- if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO))
- for (i = 0; i < (1 << order); i++)
- clear_highpage(page + i);
+ if (!free_pages_prezeroed() && want_init_on_alloc(gfp_flags))
+ kernel_init_free_pages(page, 1 << order);
if (order && (gfp_flags & __GFP_COMP))
prep_compound_page(page, order);
diff --git a/mm/slab.c b/mm/slab.c
index f7117ad9b3a3..98a89d7c922d 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1830,6 +1830,14 @@ static bool set_objfreelist_slab_cache(struct kmem_cache *cachep,
cachep->num = 0;
+ /*
+ * If slab auto-initialization on free is enabled, store the freelist
+ * off-slab, so that its contents don't end up in one of the allocated
+ * objects.
+ */
+ if (unlikely(slab_want_init_on_free(cachep)))
+ return false;
+
if (cachep->ctor || flags & SLAB_TYPESAFE_BY_RCU)
return false;
@@ -3263,7 +3271,7 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
local_irq_restore(save_flags);
ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
- if (unlikely(flags & __GFP_ZERO) && ptr)
+ if (unlikely(slab_want_init_on_alloc(flags, cachep)) && ptr)
memset(ptr, 0, cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &ptr);
@@ -3320,7 +3328,7 @@ slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
prefetchw(objp);
- if (unlikely(flags & __GFP_ZERO) && objp)
+ if (unlikely(slab_want_init_on_alloc(flags, cachep)) && objp)
memset(objp, 0, cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &objp);
@@ -3441,6 +3449,8 @@ void ___cache_free(struct kmem_cache *cachep, void *objp,
struct array_cache *ac = cpu_cache_get(cachep);
check_irq_off();
+ if (unlikely(slab_want_init_on_free(cachep)))
+ memset(objp, 0, cachep->object_size);
kmemleak_free_recursive(objp, cachep->flags);
objp = cache_free_debugcheck(cachep, objp, caller);
@@ -3528,7 +3538,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
cache_alloc_debugcheck_after_bulk(s, flags, size, p, _RET_IP_);
/* Clear memory outside IRQ disabled section */
- if (unlikely(flags & __GFP_ZERO))
+ if (unlikely(slab_want_init_on_alloc(flags, s)))
for (i = 0; i < size; i++)
memset(p[i], 0, s->object_size);
diff --git a/mm/slab.h b/mm/slab.h
index 43ac818b8592..31032d488b29 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -524,4 +524,23 @@ static inline int cache_random_seq_create(struct kmem_cache *cachep,
static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
#endif /* CONFIG_SLAB_FREELIST_RANDOM */
+static inline bool slab_want_init_on_alloc(gfp_t flags, struct kmem_cache *c)
+{
+ if (static_branch_unlikely(&init_on_alloc)) {
+ if (c->ctor)
+ return false;
+ if (c->flags & SLAB_TYPESAFE_BY_RCU)
+ return flags & __GFP_ZERO;
+ return true;
+ }
+ return flags & __GFP_ZERO;
+}
+
+static inline bool slab_want_init_on_free(struct kmem_cache *c)
+{
+ if (static_branch_unlikely(&init_on_free))
+ return !(c->ctor || (c->flags & SLAB_TYPESAFE_BY_RCU));
+ return false;
+}
+
#endif /* MM_SLAB_H */
diff --git a/mm/slub.c b/mm/slub.c
index cd04dbd2b5d0..9c4a8b9a955c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1279,6 +1279,12 @@ static int __init setup_slub_debug(char *str)
if (*str == ',')
slub_debug_slabs = str + 1;
out:
+ if ((static_branch_unlikely(&init_on_alloc) ||
+ static_branch_unlikely(&init_on_free)) &&
+ (slub_debug & SLAB_POISON)) {
+ pr_warn("disabling SLAB_POISON: can't be used together with memory auto-initialization\n");
+ slub_debug &= ~SLAB_POISON;
+ }
return 1;
}
@@ -1424,6 +1430,19 @@ static __always_inline bool slab_free_hook(struct kmem_cache *s, void *x)
static inline bool slab_free_freelist_hook(struct kmem_cache *s,
void **head, void **tail)
{
+
+ void *object;
+ void *next = *head;
+ void *old_tail = *tail ? *tail : *head;
+
+ if (slab_want_init_on_free(s))
+ do {
+ object = next;
+ next = get_freepointer(s, object);
+ memset(object, 0, s->size);
+ set_freepointer(s, object, next);
+ } while (object != old_tail);
+
/*
* Compiler cannot detect this function can be removed if slab_free_hook()
* evaluates to nothing. Thus, catch all relevant config debug options here.
@@ -1433,9 +1452,7 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s,
defined(CONFIG_DEBUG_OBJECTS_FREE) || \
defined(CONFIG_KASAN)
- void *object;
- void *next = *head;
- void *old_tail = *tail ? *tail : *head;
+ next = *head;
/* Head and tail of the reconstructed freelist */
*head = NULL;
@@ -2741,8 +2758,14 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
prefetch_freepointer(s, next_object);
stat(s, ALLOC_FASTPATH);
}
+ /*
+ * If the object has been wiped upon free, make sure it's fully
+ * initialized by zeroing out freelist pointer.
+ */
+ if (unlikely(slab_want_init_on_free(s)) && object)
+ *(void **)object = NULL;
- if (unlikely(gfpflags & __GFP_ZERO) && object)
+ if (unlikely(slab_want_init_on_alloc(gfpflags, s)) && object)
memset(object, 0, s->object_size);
slab_post_alloc_hook(s, gfpflags, 1, &object);
@@ -3163,7 +3186,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
local_irq_enable();
/* Clear memory outside IRQ disabled fastpath loop */
- if (unlikely(flags & __GFP_ZERO)) {
+ if (unlikely(slab_want_init_on_alloc(flags, s))) {
int j;
for (j = 0; j < i; j++)
diff --git a/net/core/sock.c b/net/core/sock.c
index 75b1c950b49f..9ceb90c875bc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1602,7 +1602,7 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
if (!sk)
return sk;
- if (priority & __GFP_ZERO)
+ if (want_init_on_alloc(priority))
sk_prot_clear_nulls(sk, prot->obj_size);
} else
sk = kmalloc(prot->obj_size, priority);
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index c6cb2d9b2905..a1ffe2eb4d5f 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -160,6 +160,35 @@ config STACKLEAK_RUNTIME_DISABLE
runtime to control kernel stack erasing for kernels built with
CONFIG_GCC_PLUGIN_STACKLEAK.
+config INIT_ON_ALLOC_DEFAULT_ON
+ bool "Enable heap memory zeroing on allocation by default"
+ help
+ This has the effect of setting "init_on_alloc=1" on the kernel
+ command line. This can be disabled with "init_on_alloc=0".
+ When "init_on_alloc" is enabled, all page allocator and slab
+ allocator memory will be zeroed when allocated, eliminating
+ many kinds of "uninitialized heap memory" flaws, especially
+ heap content exposures. The performance impact varies by
+ workload, but most cases see <1% impact. Some synthetic
+ workloads have measured as high as 7%.
+
+config INIT_ON_FREE_DEFAULT_ON
+ bool "Enable heap memory zeroing on free by default"
+ help
+ This has the effect of setting "init_on_free=1" on the kernel
+ command line. This can be disabled with "init_on_free=0".
+ Similar to "init_on_alloc", when "init_on_free" is enabled,
+ all page allocator and slab allocator memory will be zeroed
+ when freed, eliminating many kinds of "uninitialized heap memory"
+ flaws, especially heap content exposures. The primary difference
+ with "init_on_free" is that data lifetime in memory is reduced,
+ as anything freed is wiped immediately, making live forensics or
+ cold boot memory attacks unable to recover freed memory contents.
+ The performance impact varies by workload, but is more expensive
+ than "init_on_alloc" due to the negative cache effects of
+ touching "cold" memory areas. Most cases see 3-5% impact. Some
+ synthetic workloads have measured as high as 8%.
+
endmenu
endmenu
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related
* Re: [PATCH 09/10] usb: Add USB subsystem notifications [ver #3]
From: Felipe Balbi @ 2019-06-07 6:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern
Cc: David Howells, viro, linux-usb, raven, linux-fsdevel, linux-api,
linux-block, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20190606153150.GB28997@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
Hi,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Thu, Jun 06, 2019 at 10:55:24AM -0400, Alan Stern wrote:
>> On Thu, 6 Jun 2019, Greg Kroah-Hartman wrote:
>>
>> > On Thu, Jun 06, 2019 at 10:24:18AM -0400, Alan Stern wrote:
>> > > On Thu, 6 Jun 2019, David Howells wrote:
>> > >
>> > > > Add a USB subsystem notification mechanism whereby notifications about
>> > > > hardware events such as device connection, disconnection, reset and I/O
>> > > > errors, can be reported to a monitoring process asynchronously.
>> > >
>> > > USB I/O errors covers an awfully large and vague field. Do we really
>> > > want to include them? I'm doubtful.
>> >
>> > See the other patch on the linux-usb list that wanted to start adding
>> > KOBJ_CHANGE notifications about USB "i/o errors".
>>
>> That patch wanted to add notifications only for enumeration failures
>> (assuming you're talking about the patch from Eugeniu Rosca), not I/O
>> errors in general.
>
> Yes, sorry, I was thinking that as a "I/O failed in enumeration" :)
>
>> > So for "severe" issues, yes, we should do this, but perhaps not for all
>> > of the "normal" things we see when a device is yanked out of the system
>> > and the like.
>>
>> Then what counts as a "severe" issue? Anything besides enumeration
>> failure?
>
> Not that I can think of at the moment, other than the other recently
> added KOBJ_CHANGE issue. I'm sure we have other "hard failure" issues
> in the USB stack that people will want exposed over time.
From an XHCI standpoint, Transaction Errors might be one thing. They
happen rarely and are a strong indication that the bus itself is
bad. Either bad cable, misbehaving PHYs, improper power management, etc.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] LSM: switch to blocking policy update notifiers
From: Paul Moore @ 2019-06-07 5:19 UTC (permalink / raw)
To: James Morris
Cc: Casey Schaufler, Janne Karhunen, Stephen Smalley, Mimi Zohar,
linux-integrity, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1906071043160.21512@namei.org>
On Thu, Jun 6, 2019 at 8:45 PM James Morris <jmorris@namei.org> wrote:
> On Wed, 5 Jun 2019, Paul Moore wrote:
> > On Wed, Jun 5, 2019 at 1:05 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > > On 6/5/2019 9:51 AM, Janne Karhunen wrote:
> > >
> > > One hook with an added "bool blocking" argument, if
> > > that's the only difference?
> >
> > I think there is value in keeping a similar convention to the notifier
> > code on which this is based, see include/linux/notifier.h.
>
> Although this doesn't seem to be what other users in the kernel are doing.
How many of them potentially have the need for both blocking and
non-blocking notifiers? I didn't go through the entire list of
callers, but it seems all that I looked at used only one type. The
simple fact that we started with one type of notifier for the LSM, and
we are now switching to the other (and getting lucky that it is safe
to do so for the existing callers) seems to lend some weight to the
argument we may need both and adding "block"/"blocking"/etc. to the
name has value.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/2] LSM: switch to blocking policy update notifiers
From: James Morris @ 2019-06-07 0:45 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, Janne Karhunen, Stephen Smalley, Mimi Zohar,
linux-integrity, linux-security-module
In-Reply-To: <CAHC9VhTzwPoxYPxYWn15ZQQwM6Q3wGJSRybb-zu_ZDA1xU6ziQ@mail.gmail.com>
On Wed, 5 Jun 2019, Paul Moore wrote:
> On Wed, Jun 5, 2019 at 1:05 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > On 6/5/2019 9:51 AM, Janne Karhunen wrote:
> >
> > One hook with an added "bool blocking" argument, if
> > that's the only difference?
>
> I think there is value in keeping a similar convention to the notifier
> code on which this is based, see include/linux/notifier.h.
>
Although this doesn't seem to be what other users in the kernel are doing.
Probably the less code churn the better in this case.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [GIT PULL] apparmor bug fixes for v5.3-rc4
From: John Johansen @ 2019-06-07 0:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKLM, open list:SECURITY SUBSYSTEM
Hi Linus,
Can you please pull the following bug fixes for apparmor
Thanks!
- John
The following changes since commit b8a5afa418c1f5c8d7814ef829a88e60ae52f618:
net: correct zerocopy refcnt with udp MSG_MORE (2019-05-31 06:40:10 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2019-06-06
for you to fetch changes up to dd60c38193b1cd8bc1cbde1425881cd5227ef466:
apparmor: enforce nullbyte at end of tag string (2019-05-31 06:50:00 -0700)
----------------------------------------------------------------
+ Bug Fixes
- Fix PROFILE_MEDIATES for untrusted input
- enforce nullbyte at end of tag string
----------------------------------------------------------------
Jann Horn (1):
apparmor: enforce nullbyte at end of tag string
John Johansen (1):
apparmor: fix PROFILE_MEDIATES for untrusted input
security/apparmor/include/policy.h | 11 ++++++++++-
security/apparmor/policy_unpack.c | 2 +-
2 files changed, 11 insertions(+), 2 deletions(-)
^ permalink raw reply
* [PATCH v7 2/3] add a new ima template field buf
From: Prakhar Srivastava @ 2019-06-07 0:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190607002330.2999-1-prsriva02@gmail.com>
A buffer(kexec cmdline args) measured into ima cannot be
appraised without already being aware of the buffer contents.
Since hashes are non-reversible, raw buffer is needed for
validation or regenerating hash for appraisal/attestation.
This patch adds support to ima to allow store/read the
buffer contents in HEX.
- Add two new fields to ima_event_data to hold the buf and
buf_len [Suggested by Roberto]
- Add a new temaplte field 'buf' to be used to store/read
the buffer data.[Suggested by Mimi]
- Updated process_buffer_meaurement to add the buffer to
ima_event_data. process_buffer_measurement added in
"Add a new ima hook ima_kexec_cmdline to measure cmdline args"
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Documentation/security/IMA-templates.rst | 4 ++--
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_api.c | 4 ++--
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 2 ++
security/integrity/ima/ima_template.c | 2 ++
security/integrity/ima/ima_template_lib.c | 20 ++++++++++++++++++++
security/integrity/ima/ima_template_lib.h | 4 ++++
8 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..3e78ce3591db 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,8 +69,8 @@ descriptors by adding their identifier to the format string
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
-
+ - 'sig': the file signature;
+ - 'buf': the buffer data that was used to generate the hash without size limitations;
Below, there is the list of defined template descriptors:
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a4ad1270bffa..16110180545c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -65,6 +65,8 @@ struct ima_event_data {
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
const char *violation;
+ const void *buf;
+ int buf_len;
};
/* IMA template field data definition */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index ea7d8cbf712f..83ca99d65e4b 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -140,7 +140,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
struct ima_template_entry *entry;
struct inode *inode = file_inode(file);
struct ima_event_data event_data = {iint, file, filename, NULL, 0,
- cause};
+ cause, NULL, 0};
int violation = 1;
int result;
@@ -296,7 +296,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
struct inode *inode = file_inode(file);
struct ima_template_entry *entry;
struct ima_event_data event_data = {iint, file, filename, xattr_value,
- xattr_len, NULL};
+ xattr_len, NULL, NULL, 0};
int violation = 0;
if (iint->measured_pcrs & (0x1 << pcr))
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 993d0f1915ff..c8591406c0e2 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -50,7 +50,7 @@ static int __init ima_add_boot_aggregate(void)
struct ima_template_entry *entry;
struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
- NULL, 0, NULL};
+ NULL, 0, NULL, NULL, 0};
int result = -ENOMEM;
int violation = 0;
struct {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index e4f301381ffb..9308d664f074 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -641,6 +641,8 @@ static void process_buffer_measurement(const void *buf, int size,
memset(&hash, 0, sizeof(hash));
event_data.filename = eventname;
+ event_data.buf = buf;
+ event_data.buf_len = size;
iint->ima_hash = &hash.hdr;
iint->ima_hash->algo = ima_hash_algo;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e6e892f31cbd..8c40de18a0aa 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -43,6 +43,8 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "buf", .field_init = ima_eventbuf_init,
+ .field_show = ima_show_template_buf},
};
#define MAX_TEMPLATE_NAME_LEN 15
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..43d1404141c1 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -162,6 +162,12 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
}
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data)
+{
+ ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
+}
+
/**
* ima_parse_buf() - Parses lengths and data from an input buffer
* @bufstartp: Buffer start address.
@@ -389,3 +395,17 @@ int ima_eventsig_init(struct ima_event_data *event_data,
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
DATA_FMT_HEX, field_data);
}
+
+/*
+ * ima_eventbuf_init - include the buffer(kexec-cmldine) as part of the
+ * template data.
+ */
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ if ((!event_data->buf) || (event_data->buf_len == 0))
+ return 0;
+
+ return ima_write_template_field_data(event_data->buf, event_data->buf_len,
+ DATA_FMT_HEX, field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..f0178bc60c55 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data);
int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
int maxfields, struct ima_field_data *fields, int *curfields,
unsigned long *len_mask, int enforce_mask, char *bufname);
@@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
--
2.19.1
^ permalink raw reply related
* [PATCH v7 3/3] call ima_kexec_cmdline to measure the cmdline args
From: Prakhar Srivastava @ 2019-06-07 0:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190607002330.2999-1-prsriva02@gmail.com>
During soft reboot(kexec_file_load) boot cmdline args
are not measured.Thus the new kernel on load boots with
an assumption of cold reboot.
This patch makes a call to the ima hook ima_kexec_cmdline,
added in "Add a new ima hook ima_kexec_cmdline to measure
cmdline args"
to measure the boot cmdline args into the ima log.
- call ima_kexec_cmdline from kexec_file_load.
- move the call ima_add_kexec_buffer after the cmdline
args have been measured.
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
kernel/kexec_file.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 072b6ee55e3f..ed4727586fc3 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
return ret;
image->kernel_buf_len = size;
- /* IMA needs to pass the measurement list to the next kernel. */
- ima_add_kexec_buffer(image);
-
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
@@ -241,8 +238,13 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
ret = -EINVAL;
goto out;
}
+
+ ima_kexec_cmdline(image->cmdline_buf, image->cmdline_buf_len - 1);
}
+ /* IMA needs to pass the measurement list to the next kernel. */
+ ima_add_kexec_buffer(image);
+
/* Call arch image load handlers */
ldata = arch_kexec_kernel_image_load(image);
--
2.19.1
^ permalink raw reply related
* [PATCH v7 1/3] Add a new ima hook ima_kexec_cmdline to measure cmdline args
From: Prakhar Srivastava @ 2019-06-07 0:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190607002330.2999-1-prsriva02@gmail.com>
This patch adds support in ima to measure kexec cmdline args
during soft reboot kexec_file_load.
- A new ima hook ima_kexec_cmdline is defined to be called by the
kexec code.
- A new function process_buffer_measurement is defined to measure
the buffer hash into the ima log.
- A new func policy KEXEC_CMDLINE is defined to control the
measurement.[Suggested by Mimi]
Hash computation can be tested using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
Documentation/ABI/testing/ima_policy | 1 +
include/linux/ima.h | 2 +
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 1 +
security/integrity/ima/ima_main.c | 77 ++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 ++++
6 files changed, 91 insertions(+)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b383c1763610..fc376a323908 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -28,6 +28,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
+ [KEXEC_CMDLINE]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
diff --git a/include/linux/ima.h b/include/linux/ima.h
index fd9f7cf4cdf5..b42f5a006042 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_kexec_cmdline(const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -92,6 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
return;
}
+static inline void ima_kexec_cmdline(const void *buf, int size) {}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 18b48a6d0b80..a4ad1270bffa 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -185,6 +185,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(KEXEC_KERNEL_CHECK) \
hook(KEXEC_INITRAMFS_CHECK) \
hook(POLICY_CHECK) \
+ hook(KEXEC_CMDLINE) \
hook(MAX_CHECK)
#define __ima_hook_enumify(ENUM) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 78eb11c7ac07..ea7d8cbf712f 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -176,6 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
+ * | KEXEC_CMDLINE
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index af341a80118f..e4f301381ffb 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -605,6 +605,83 @@ int ima_load_data(enum kernel_load_data_id id)
return 0;
}
+/*
+ * process_buffer_measurement - Measure the buffer to ima log.
+ * @buf: pointer to the buffer that needs to be added to the log.
+ * @size: size of buffer(in bytes).
+ * @eventname: event name to be used for the buffer entry.
+ * @cred: a pointer to a credentials structure for user validation.
+ * @secid: the secid of the task to be validated.
+ *
+ * Based on policy, the buffer is measured into the ima log.
+ */
+static void process_buffer_measurement(const void *buf, int size,
+ const char *eventname, const struct cred *cred,
+ u32 secid)
+{
+ int ret = 0;
+ struct ima_template_entry *entry = NULL;
+ struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
+ struct ima_event_data event_data = {.iint = iint};
+ struct ima_template_desc *template_desc = NULL;
+ struct {
+ struct ima_digest_data hdr;
+ char digest[IMA_MAX_DIGEST_SIZE];
+ } hash;
+ int violation = 0;
+ int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+ int action = 0;
+
+ action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
+ &template_desc);
+ if (!(action & IMA_MEASURE))
+ goto out;
+
+ memset(iint, 0, sizeof(*iint));
+ memset(&hash, 0, sizeof(hash));
+
+ event_data.filename = eventname;
+
+ iint->ima_hash = &hash.hdr;
+ iint->ima_hash->algo = ima_hash_algo;
+ iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+
+ ret = ima_calc_buffer_hash(buf, size, iint->ima_hash);
+ if (ret < 0)
+ goto out;
+
+ ret = ima_alloc_init_template(&event_data, &entry, template_desc);
+ if (ret < 0)
+ goto out;
+
+ if (action & IMA_MEASURE)
+ ret = ima_store_template(entry, violation, NULL, buf, pcr);
+
+ if (ret < 0)
+ ima_free_template_entry(entry);
+
+out:
+ return;
+}
+
+/**
+ * ima_kexec_cmdline - measure kexec cmdline boot args
+ * @buf: pointer to buffer
+ * @size: size of buffer
+ *
+ * Buffers can only be measured, not appraised.
+ */
+void ima_kexec_cmdline(const void *buf, int size)
+{
+ u32 secid;
+
+ if (buf && size != 0) {
+ security_task_getsecid(current, &secid);
+ process_buffer_measurement(buf, size, "kexec-cmdline",
+ current_cred(), secid);
+ }
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd9b01881d17..98e351e13557 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -292,6 +292,13 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
+ /* only incase of KEXEC_CMDLINE, inode is NULL */
+ if (func == KEXEC_CMDLINE) {
+ if ((rule->flags & IMA_FUNC) &&
+ (rule->func == func) && (!inode))
+ return true;
+ return false;
+ }
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
@@ -880,6 +887,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = KEXEC_INITRAMFS_CHECK;
else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
entry->func = POLICY_CHECK;
+ else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
+ entry->func = KEXEC_CMDLINE;
else
result = -EINVAL;
if (!result)
--
2.19.1
^ permalink raw reply related
* [PATCH v7 0/3] add new ima hook ima_kexec_cmdline to measure kexec boot cmdline args
From: Prakhar Srivastava @ 2019-06-07 0:23 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
The motive behind the patch series is to measure the boot cmdline args
used for soft reboot/kexec case.
For secure boot attestation, it is necessary to measure the kernel
command line and the kernel version. For cold boot, the boot loader
can be enhanced to measure these parameters.
(https://mjg59.dreamwidth.org/48897.html)
However, for attestation across soft reboot boundary, these values
also need to be measured during kexec_file_load.
Currently for Kexec(kexec_file_load)/soft reboot scenario the boot cmdline
args for the next kernel are not measured. For
normal case of boot/hardreboot the cmdline args are measured into the TPM.
The hash of boot command line is calculated and added to the current
running kernel's measurement list.
On a soft reboot like kexec, no cmdline arguments measurement takes place.
To achive the above the patch series does the following
-adds a new ima hook: ima_kexec_cmdline which measures the cmdline args
into the ima log, behind a new ima policy entry KEXEC_CMDLINE.
-since the cmldine args cannot be appraised, a new template field(buf) is
added. The template field contains the buffer passed(cmldine args), which
can be used to appraise/attest at a later stage.
-call the ima_kexec_cmdline(...) hook from kexec_file_load call.
The ima logs need to be carried over to the next kernel, which will be followed
up by other patchsets for x86_64 and arm64.
The kexec cmdline hash can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
Changelog:
V7:
- rebased to next-queued-testing
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-queued-testing
V6:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
add new fields to ima_event_data to store/read buffer data.
[suggested by Roberto]
-call ima_kexec_cmdline from kexec_file_load path
v5:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
-call ima_kexec_cmdline from kexec_file_load path
v4:
- per feedback from LSM community, removed the LSM hook and renamed the
IMA policy to KEXEC_CMDLINE
v3: (rebase changes to next-general)
- Add policy checks for buffer[suggested by Mimi Zohar]
- use the IMA_XATTR to add buffer
- Add kexec_cmdline used for kexec file load
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
v2:
- Add policy checks for buffer[suggested by Mimi Zohar]
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
- use the IMA_XATTR to add buffer instead of sig template
v1:
-Add kconfigs to control the ima_buffer_check
-measure the cmdline args suffixed with the kernel file name
-add the buffer to the template sig field.
Prakhar Srivastava (3):
Add a new ima hook ima_kexec_cmdline to measure cmdline args
add a new ima template field buf
call ima_kexec_cmdline to measure the cmdline args
Documentation/ABI/testing/ima_policy | 1 +
Documentation/security/IMA-templates.rst | 2 +-
include/linux/ima.h | 2 +
kernel/kexec_file.c | 8 ++-
security/integrity/ima/ima.h | 3 +
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 80 +++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 +++
security/integrity/ima/ima_template.c | 2 +
security/integrity/ima/ima_template_lib.c | 20 ++++++
security/integrity/ima/ima_template_lib.h | 4 ++
12 files changed, 131 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH 22/58] Audit: Change audit_sig_sid to audit_sig_lsm
From: Kees Cook @ 2019-06-06 22:53 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds
In-Reply-To: <5010ae20-ce00-be1e-0c2b-7568282b6b39@schaufler-ca.com>
On Thu, Jun 06, 2019 at 02:06:44PM -0700, Casey Schaufler wrote:
> I'd rather describe what's in it than how it's used.
Yeah, good point. :)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 10/10] Add sample notification program [ver #3]
From: David Howells @ 2019-06-06 22:52 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: dhowells, viro, raven, linux-fsdevel, linux-api, linux-block,
keyrings, linux-security-module, linux-kernel, Eugeniu Rosca
In-Reply-To: <20190606212140.GA25664@vmlxhi-102.adit-jv.com>
Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> How about arm64? Do you intend to enable cross-compilation?
There's no guarantee that a cross-compiler can actually build userspace apps,
though I haven't intended to encode anything against that in the Makefile.
> > + asm ("lfence" : : : "memory" );
> [..]
> > + asm("mfence" ::: "memory");
>
> FWIW, trying to cross-compile it returned:
>
> aarch64-linux-gnu-gcc -I../../usr/include -I../../include watch_test.c -o watch_test
> /tmp/ccDXYynm.s: Assembler messages:
> /tmp/ccDXYynm.s:471: Error: unknown mnemonic `lfence' -- `lfence'
> /tmp/ccDXYynm.s:568: Error: unknown mnemonic `mfence' -- `mfence'
Yeah. I need to use C-standard __atomic_* stuff.
David
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: David Howells @ 2019-06-06 22:50 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, Andy Lutomirski, Casey Schaufler, Stephen Smalley,
Al Viro, Greg Kroah-Hartman, USB list, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <D2BD8FEB-5DF5-449B-AF81-83BA65E0E643@amacapital.net>
Andy Lutomirski <luto@amacapital.net> wrote:
> They can call fsinfo() anyway, or just read /proc/self/mounts. As far as I’m
> concerned, if you have CAP_SYS_ADMIN over a mount namespace and LSM policy
> lets you mount things, the of course you can get information to basically
> anyone who can use that mount namespace.
And automounts? You don't need CAP_SYS_ADMIN to trigger one of those, but
they still generate events. On the other hand, you need CSA to mount
something that has automounts in the first place, and if you're particularly
concerned about security, you probably don't want the processes you might be
suspicious of having access to things that contain automounts (typically
network filesystems).
David
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Andy Lutomirski @ 2019-06-06 22:42 UTC (permalink / raw)
To: David Howells
Cc: Andy Lutomirski, Casey Schaufler, Stephen Smalley, Al Viro,
Greg Kroah-Hartman, USB list, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <30567.1559860681@warthog.procyon.org.uk>
> On Jun 6, 2019, at 3:38 PM, David Howells <dhowells@redhat.com> wrote:
>
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> I mean: are there cases where some action generates a notification but does
>> not otherwise have an effect visible to the users who can receive the
>> notification. It looks like the answer is probably “no”, which is good.
>
> mount_notify(). You can get a notification that someone altered the mount
> topology (eg. by mounting something). A process receiving a notification
> could then use fsinfo(), say, to reread the mount topology tree, find out
> where the new mount is and wander over there to have a look - assuming they
> have the permissions for pathwalk to succeed.
>
>
They can call fsinfo() anyway, or just read /proc/self/mounts. As far as I’m concerned, if you have CAP_SYS_ADMIN over a mount namespace and LSM policy lets you mount things, the of course you can get information to basically anyone who can use that mount namespace.
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: David Howells @ 2019-06-06 22:38 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, Andy Lutomirski, Casey Schaufler, Stephen Smalley,
Al Viro, Greg Kroah-Hartman, USB list, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <AD7898AE-B92C-4DE6-B895-7116FEDB3091@amacapital.net>
Andy Lutomirski <luto@amacapital.net> wrote:
> I mean: are there cases where some action generates a notification but does
> not otherwise have an effect visible to the users who can receive the
> notification. It looks like the answer is probably “no”, which is good.
mount_notify(). You can get a notification that someone altered the mount
topology (eg. by mounting something). A process receiving a notification
could then use fsinfo(), say, to reread the mount topology tree, find out
where the new mount is and wander over there to have a look - assuming they
have the permissions for pathwalk to succeed.
David
^ permalink raw reply
* Re: [PATCH 2/2] ima: use the lsm policy update notifier
From: Mimi Zohar @ 2019-06-06 22:28 UTC (permalink / raw)
To: Janne Karhunen, sds, paul; +Cc: linux-integrity, linux-security-module
In-Reply-To: <1559858345.4278.163.camel@linux.ibm.com>
Hi Janne,
One more comment below ...
> > +
> > +static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
> > +{
> > + struct ima_rule_entry *nentry;
> > + int i, result;
> > +
> > + nentry = kmalloc(sizeof(*nentry), GFP_KERNEL);
> > + if (!nentry)
> > + return NULL;
> > +
> > + memcpy(nentry, entry, sizeof(*nentry));
> > + nentry->fsname = NULL;
> > + for (i = 0; i < MAX_LSM_RULES; i++) {
> > + nentry->lsm[i].rule = NULL;
> > + nentry->lsm[i].args_p = NULL;
> > + }
I don't think this loop is necessary. Either use kzalloc() or move
the initialization to inside the loop below.
> > +
> > + if (entry->fsname) {
> > + nentry->fsname = kstrdup(entry->fsname, GFP_KERNEL);
> > + if (!nentry->fsname)
> > + goto out_err;
> > + }
> > + for (i = 0; i < MAX_LSM_RULES; i++) {
> > + if (!entry->lsm[i].rule)
> > + continue;
To here.
> > +
> > + nentry->lsm[i].type = entry->lsm[i].type;
> > + nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p,
> > + GFP_KERNEL);
> > + if (!nentry->lsm[i].args_p)
> > + goto out_err;
If the memory allocation fails, then nentry will be freed anyway.
thanks,
Mimid
^ permalink raw reply
* Re: [PATCH 2/2] ima: use the lsm policy update notifier
From: Mimi Zohar @ 2019-06-06 21:59 UTC (permalink / raw)
To: Janne Karhunen, sds, paul; +Cc: linux-integrity, linux-security-module
In-Reply-To: <20190605083606.4209-2-janne.karhunen@gmail.com>
Hi Janne,
On Wed, 2019-06-05 at 11:36 +0300, Janne Karhunen wrote:
> Don't do lazy policy updates while running the rule matching,
> run the updates as they happen.
>
> Depends on commit 2d1d5cee66d1 ("LSM: switch to blocking policy update notifiers")
>
> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Thanks! Just a couple of minor things. Comments inline below.
> ---
> security/integrity/ima/ima.h | 2 +
> security/integrity/ima/ima_main.c | 8 ++
> security/integrity/ima/ima_policy.c | 124 +++++++++++++++++++++++-----
> 3 files changed, 114 insertions(+), 20 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d213e835c498..2203451862d4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void);
> int ima_init_template(void);
> void ima_init_template_list(void);
> int __init ima_init_digests(void);
> +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data);
>
> /*
> * used to protect h_table and sha_table
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index f16353b5097e..9e3ea8a3f2db 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -43,6 +43,10 @@ int ima_appraise;
> int ima_hash_algo = HASH_ALGO_SHA1;
> static int hash_setup_done;
>
> +static struct notifier_block ima_lsm_policy_notifier = {
> + .notifier_call = ima_lsm_policy_change,
> +};
> +
> static int __init hash_setup(char *str)
> {
> struct ima_template_desc *template_desc = ima_template_desc_current();
> @@ -621,6 +625,10 @@ static int __init init_ima(void)
> error = ima_init();
> }
>
> + error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier);
> + if (error)
> + pr_warn("Couldn't register LSM notifier, error %d\n", error);
> +
> if (!error)
> ima_update_policy_flag();
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 1cc822a59054..7129dc4cd396 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -249,31 +249,121 @@ static int __init default_appraise_policy_setup(char *str)
> }
> __setup("ima_appraise_tcb", default_appraise_policy_setup);
>
> +static void ima_lsm_free_rule(struct ima_rule_entry *entry)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_LSM_RULES; i++) {
> + kfree(entry->lsm[i].rule);
> + kfree(entry->lsm[i].args_p);
> + }
> + kfree(entry->fsname);
> + kfree(entry);
> +}
Matthew's patch, which adds per policy template format support, adds a
"template" field to entry. In case anyone wants to backport this
patch, it might be simpler to make the change as a separate patch.
> +
> +static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
> +{
> + struct ima_rule_entry *nentry;
> + int i, result;
> +
> + nentry = kmalloc(sizeof(*nentry), GFP_KERNEL);
> + if (!nentry)
> + return NULL;
> +
> + memcpy(nentry, entry, sizeof(*nentry));
> + nentry->fsname = NULL;
> + for (i = 0; i < MAX_LSM_RULES; i++) {
> + nentry->lsm[i].rule = NULL;
> + nentry->lsm[i].args_p = NULL;
> + }
> +
> + if (entry->fsname) {
> + nentry->fsname = kstrdup(entry->fsname, GFP_KERNEL);
> + if (!nentry->fsname)
> + goto out_err;
> + }
> + for (i = 0; i < MAX_LSM_RULES; i++) {
> + if (!entry->lsm[i].rule)
> + continue;
> +
> + nentry->lsm[i].type = entry->lsm[i].type;
> + nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p,
> + GFP_KERNEL);
> + if (!nentry->lsm[i].args_p)
> + goto out_err;
> +
> + result = security_filter_rule_init(nentry->lsm[i].type,
> + Audit_equal,
> + nentry->lsm[i].args_p,
> + &nentry->lsm[i].rule);
> + if (result == -EINVAL)
> + pr_warn("ima: rule for LSM \'%d\' is invalid\n",
> + entry->lsm[i].type);
If LSM labels can come and go, then perhaps instead of saying
"invalid" say "undefined" or "missing".
> +
> + }
> + return nentry;
> +
> +out_err:
> + ima_lsm_free_rule(nentry);
> + return NULL;
> +}
> +
> +static int ima_lsm_update_rule(struct ima_rule_entry *entry)
> +{
> + struct ima_rule_entry *nentry;
> +
> + nentry = ima_lsm_copy_rule(entry);
> + if (!nentry)
> + return -ENOMEM;
> +
> + list_replace_rcu(&entry->list, &nentry->list);
> + synchronize_rcu();
> + ima_lsm_free_rule(entry);
> +
> + return 0;
> +}
> +
> /*
> * The LSM policy can be reloaded, leaving the IMA LSM based rules referring
> * to the old, stale LSM policy. Update the IMA LSM based rules to reflect
> - * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if
> - * they don't.
> + * the reloaded LSM policy.
> */
> static void ima_lsm_update_rules(void)
> {
> - struct ima_rule_entry *entry;
> - int result;
> - int i;
> + struct ima_rule_entry *entry, *e;
> + int i, result, needs_update;
>
> - list_for_each_entry(entry, &ima_policy_rules, list) {
> + list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
> + needs_update = 0;
> for (i = 0; i < MAX_LSM_RULES; i++) {
> - if (!entry->lsm[i].rule)
> - continue;
> - result = security_filter_rule_init(entry->lsm[i].type,
> - Audit_equal,
> - entry->lsm[i].args_p,
> - &entry->lsm[i].rule);
> - BUG_ON(!entry->lsm[i].rule);
> + if (entry->lsm[i].rule) {
> + needs_update = 1;
> + break;
> + }
> + }
> + if (!needs_update)
> + continue;
> +
> + result = ima_lsm_update_rule(entry);
> + if (result) {
> + pr_err("ima: lsm rule update error %d\n",
> + result);
No need for separate line.
Mimi
> + return;
> }
> }
> }
>
> +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data)
> +{
> + if (event != LSM_POLICY_CHANGE)
> + return NOTIFY_DONE;
> +
> + ima_lsm_update_rules();
> + return NOTIFY_OK;
> +}
> +
> /**
> * ima_match_rules - determine whether an inode matches the measure rule.
> * @rule: a pointer to a rule
> @@ -327,11 +417,10 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> for (i = 0; i < MAX_LSM_RULES; i++) {
> int rc = 0;
> u32 osid;
> - int retried = 0;
>
> if (!rule->lsm[i].rule)
> continue;
> -retry:
> +
> switch (i) {
> case LSM_OBJ_USER:
> case LSM_OBJ_ROLE:
> @@ -352,11 +441,6 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> default:
> break;
> }
> - if ((rc < 0) && (!retried)) {
> - retried = 1;
> - ima_lsm_update_rules();
> - goto retry;
> - }
> if (!rc)
> return false;
> }
^ permalink raw reply
* Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
From: Andy Lutomirski @ 2019-06-06 21:54 UTC (permalink / raw)
To: David Howells
Cc: Andy Lutomirski, Casey Schaufler, Stephen Smalley, Al Viro,
Greg Kroah-Hartman, USB list, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, Paul Moore
In-Reply-To: <23611.1559855827@warthog.procyon.org.uk>
> On Jun 6, 2019, at 2:17 PM, David Howells <dhowells@redhat.com> wrote:
>
> Andy Lutomirski <luto@kernel.org> wrote:
>
>>>> You are allowing arbitrary information flow between T and W above. Who
>>>> cares about notifications?
>>>
>>> I do. If Watched object is /dev/null no data flow is possible.
>>> There are many objects on a modern Linux system for which this
>>> is true. Even if it's "just a file" the existence of one path
>>> for data to flow does not justify ignoring the rules for other
>>> data paths.
>>
>> Aha!
>>
>> Even ignoring security, writes to things like /dev/null should
>> probably not trigger notifications to people who are watching
>> /dev/null. (There are probably lots of things like this: /dev/zero,
>> /dev/urandom, etc.)
>
> Even writes to /dev/null might generate access notifications; leastways,
> vfs_read() will call fsnotify_access() afterwards on success.
Hmm. I can see this being an issue, but I guess not with your patch set.
>
> Whether or not you can set marks on open device files is another matter.
>
>> David, are there any notification types that have this issue in your
>> patchset? If so, is there a straightforward way to fix it?
>
> I'm not sure what issue you're referring to specifically. Do you mean whether
> writes to device files generate notifications?
I mean: are there cases where some action generates a notification but does not otherwise have an effect visible to the users who can receive the notification. It looks like the answer is probably “no”, which is good.
Casey, is this good enough for you, or is there still an issue?
^ permalink raw reply
* Re: [PATCH 10/10] Add sample notification program [ver #3]
From: Eugeniu Rosca @ 2019-06-06 21:21 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel, Eugeniu Rosca, Eugeniu Rosca
In-Reply-To: <155981421379.17513.13158528501056454772.stgit@warthog.procyon.org.uk>
Hi David,
On Thu, Jun 06, 2019 at 10:43:33AM +0100, David Howells wrote:
[..]
> diff --git a/samples/watch_queue/Makefile b/samples/watch_queue/Makefile
> new file mode 100644
> index 000000000000..42b694430d0f
> --- /dev/null
> +++ b/samples/watch_queue/Makefile
> @@ -0,0 +1,9 @@
> +# List of programs to build
> +hostprogs-y := watch_test
> +
> +# Tell kbuild to always build the programs
> +always := $(hostprogs-y)
> +
> +HOSTCFLAGS_watch_test.o += -I$(objtree)/usr/include
How about arm64? Do you intend to enable cross-compilation?
> +
> +HOSTLOADLIBES_watch_test += -lkeyutils
> diff --git a/samples/watch_queue/watch_test.c b/samples/watch_queue/watch_test.c
> new file mode 100644
> index 000000000000..893a5380f792
> --- /dev/null
> +++ b/samples/watch_queue/watch_test.c
[..]
> + asm ("lfence" : : : "memory" );
[..]
> + asm("mfence" ::: "memory");
FWIW, trying to cross-compile it returned:
aarch64-linux-gnu-gcc -I../../usr/include -I../../include watch_test.c -o watch_test
/tmp/ccDXYynm.s: Assembler messages:
/tmp/ccDXYynm.s:471: Error: unknown mnemonic `lfence' -- `lfence'
/tmp/ccDXYynm.s:568: Error: unknown mnemonic `mfence' -- `mfence'
<builtin>: recipe for target 'watch_test' failed
make: *** [watch_test] Error 1
--
Best Regards,
Eugeniu.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox