27 Jul, 2011
1 commit
-
Signed-off-by: Al Viro
24 Feb, 2011
1 commit
-
The original ima_must_measure() function based its results on cached
iint information, which required an iint be allocated for all files.
Currently, an iint is allocated only for files in policy. As a result,
for those files in policy, ima_must_measure() is now called twice: once
to determine if the inode is in the measurement policy and, the second
time, to determine if it needs to be measured/re-measured.The second call to ima_must_measure() unnecessarily checks to see if
the file is in policy. As we already know the file is in policy, this
patch removes the second unnecessary call to ima_must_measure(), removes
the vestige iint parameter, and just checks the iint directly to determine
if the inode has been measured or needs to be measured/re-measured.Signed-off-by: Mimi Zohar
Acked-by: Eric Paris
10 Feb, 2011
3 commits
-
Now that i_readcount is maintained by the VFS layer, remove the
imbalance checking in IMA. Cleans up the IMA code nicely.Signed-off-by: Mimi Zohar
Acked-by: Eric Paris -
ima_counts_get() updated the readcount and invalidated the PCR,
as necessary. Only update the i_readcount in the VFS layer.
Move the PCR invalidation checks to ima_file_check(), where it
belongs.Maintaining the i_readcount in the VFS layer, will allow other
subsystems to use i_readcount.Signed-off-by: Mimi Zohar
Acked-by: Eric Paris -
Convert the inode's i_readcount from an unsigned int to atomic.
Signed-off-by: Mimi Zohar
Acked-by: Eric Paris
04 Jan, 2011
1 commit
-
If security_filter_rule_init() doesn't return a rule, then not everything
is as fine as the return code implies.This bug only occurs when the LSM (eg. SELinux) is disabled at runtime.
Adding an empty LSM rule causes ima_match_rules() to always succeed,
ignoring any remaining rules.default IMA TCB policy:
# PROC_SUPER_MAGIC
dont_measure fsmagic=0x9fa0
# SYSFS_MAGIC
dont_measure fsmagic=0x62656572
# DEBUGFS_MAGIC
dont_measure fsmagic=0x64626720
# TMPFS_MAGIC
dont_measure fsmagic=0x01021994
# SECURITYFS_MAGIC
dont_measure fsmagic=0x73636673< LSM specific rule >
dont_measure obj_type=var_log_tmeasure func=BPRM_CHECK
measure func=FILE_MMAP mask=MAY_EXEC
measure func=FILE_CHECK mask=MAY_READ uid=0Thus without the patch, with the boot parameters 'tcb selinux=0', adding
the above 'dont_measure obj_type=var_log_t' rule to the default IMA TCB
measurement policy, would result in nothing being measured. The patch
prevents the default TCB policy from being replaced.Signed-off-by: Mimi Zohar
Cc: James Morris
Acked-by: Serge Hallyn
Cc: David Safford
Cc:
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
27 Oct, 2010
11 commits
-
Current logic looks like this:
rc = ima_must_measure(NULL, inode, MAY_READ, FILE_CHECK);
if (rc < 0)
goto out;if (mode & FMODE_WRITE) {
if (inode->i_readcount)
send_tomtou = true;
goto out;
}if (atomic_read(&inode->i_writecount) > 0)
send_writers = true;Lets assume we have a policy which states that all files opened for read
by root must be measured.Lets assume the file has permissions 777.
Lets assume that root has the given file open for read.
Lets assume that a non-root process opens the file write.
The non-root process will get to ima_counts_get() and will check the
ima_must_measure(). Since it is not supposed to measure it will goto
out.We should check the i_readcount no matter what since we might be causing
a ToMToU voilation!This is close to correct, but still not quite perfect. The situation
could have been that root, which was interested in the mesurement opened
and closed the file and another process which is not interested in the
measurement is the one holding the i_readcount ATM. This is just overly
strict on ToMToU violations, which is better than not strict enough...Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
Currently for every removed inode IMA must take a global lock and search
the IMA rbtree looking for an associated integrity structure. Instead
we explicitly mark an inode when we add an integrity structure so we
only have to take the global lock and do the removal if it exists.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
Since finding a struct ima_iint_cache requires a valid struct inode, and
the struct ima_iint_cache is supposed to have the same lifetime as a
struct inode (technically they die together but don't need to be created
at the same time) we don't have to worry about the ima_iint_cache
outliving or dieing before the inode. So the refcnt isn't useful. Just
get rid of it and free the structure when the inode is freed.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
IMA always allocates an integrity structure to hold information about
every inode, but only needed this structure to track the number of
readers and writers currently accessing a given inode. Since that
information was moved into struct inode instead of the integrity struct
this patch stops allocating the integrity stucture until it is needed.
Thus greatly reducing memory usage.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
IMA currently allocated an inode integrity structure for every inode in
core. This stucture is about 120 bytes long. Most files however
(especially on a system which doesn't make use of IMA) will never need
any of this space. The problem is that if IMA is enabled we need to
know information about the number of readers and the number of writers
for every inode on the box. At the moment we collect that information
in the per inode iint structure and waste the rest of the space. This
patch moves those counters into the struct inode so we can eventually
stop allocating an IMA integrity structure except when absolutely
needed.This patch does the minimum needed to move the location of the data.
Further cleanups, especially the location of counter updates, may still
be possible.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
IMA tracks the number of struct files which are holding a given inode
readonly and the number which are holding the inode write or r/w. It
needs this information so when a new reader or writer comes in it can
tell if this new file will be able to invalidate results it already made
about existing files.aka if a task is holding a struct file open RO, IMA measured the file
and recorded those measurements and then a task opens the file RW IMA
needs to note in the logs that the old measurement may not be correct.
It's called a "Time of Measure Time of Use" (ToMToU) issue. The same is
true is a RO file is opened to an inode which has an open writer. We
cannot, with any validity, measure the file in question since it could
be changing.This patch attempts to use the i_writecount field to track writers. The
i_writecount field actually embeds more information in it's value than
IMA needs but it should work for our purposes and allow us to shrink the
struct inode even more.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
Currently IMA used the iint->mutex to protect the i_readcount and
i_writecount. This patch uses the inode->i_lock since we are going to
start using in inode objects and that is the most appropriate lock.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
The IMA flags is an unsigned long but there is only 1 flag defined.
Lets save a little space and make it a char. This packs nicely next to
the array of u8's.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
Currently IMA uses 2 longs in struct inode. To save space (and as it
seems impossible to overflow 32 bits) we switch these to unsigned int.
The switch to unsigned does require slightly different checks for
underflow, but it isn't complex.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
The opencount was used to help debugging to make sure that everything
which created a struct file also correctly made the IMA calls. Since we
moved all of that into the VFS this isn't as necessary. We should be
able to get the same amount of debugging out of just the reader and
write count.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds -
The IMA code needs to store the number of tasks which have an open fd
granting permission to write a file even when IMA is not in use. It
needs this information in order to be enabled at a later point in time
without losing it's integrity garantees.At the moment that means we store a little bit of data about every inode
in a cache. We use a radix tree key'd on the inode's memory address.
Dave Chinner pointed out that a radix tree is a terrible data structure
for such a sparse key space. This patch switches to using an rbtree
which should be more efficient.Bug report from Dave:
"I just noticed that slabtop was reporting an awfully high usage of
radix tree nodes:OBJS ACTIVE USE OBJ SIZE SLABS OBJ/SLAB CACHE SIZE NAME
4200331 2778082 66% 0.55K 144839 29 2317424K radix_tree_node
2321500 2060290 88% 1.00K 72581 32 2322592K xfs_inode
2235648 2069791 92% 0.12K 69864 32 279456K iint_cacheThat is, 2.7M radix tree nodes are allocated, and the cache itself is
consuming 2.3GB of RAM. I know that the XFS inodei caches are indexed
by radix tree node, but for 2 million cached inodes that would mean a
density of 1 inode per radix tree node, which for a system with 16M
inodes in the filsystems is an impossibly low density. The worst I've
seen in a production system like kernel.org is about 20-25% density,
which would mean about 150-200k radix tree nodes for that many inodes.
So it's not the inode cache.So I looked up what the iint_cache was. It appears to used for
storing per-inode IMA information, and uses a radix tree for indexing.
It uses the *address* of the struct inode as the indexing key. That
means the key space is extremely sparse - for XFS the struct inode
addresses are approximately 1000 bytes apart, which means the closest
the radix tree index keys get is ~1000. Which means that there is a
single entry per radix tree leaf node, so the radix tree is using
roughly 550 bytes for every 120byte structure being cached. For the
above example, it's probably wasting close to 1GB of RAM...."Reported-by: Dave Chinner
Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Linus Torvalds
08 Sep, 2010
1 commit
-
commit 8262bb85da allocated the inode integrity struct (iint) before any
inodes were created. Only after IMA was initialized in late_initcall were
the counters updated. This patch updates the counters, whether or not IMA
has been initialized, to resolve 'imbalance' messages.This patch fixes the bug as reported in bugzilla: 15673. When the i915
is builtin, the ring_buffer is initialized before IMA, causing the
imbalance message on suspend.Reported-by: Thomas Meyer
Signed-off-by: Mimi Zohar
Tested-by: Thomas Meyer
Tested-by: David Safford
Cc: Stable Kernel
Signed-off-by: James Morris
02 Aug, 2010
1 commit
-
The default for llseek will change to no_llseek,
so securityfs users need to add explicit .llseek
assignments. Since we're dealing with regular
files from a VFS perspective, use generic_file_llseek.Signed-off-by: Arnd Bergmann
Cc: Mimi Zohar
Signed-off-by: James Morris
22 May, 2010
1 commit
-
Of the three uses of kref_set in the kernel:
One really should be kref_put as the code is letting go of a
reference,
Two really should be kref_init because the kref is being
initialised.This suggests that making kref_set available encourages bad code.
So fix the three uses and remove kref_set completely.Signed-off-by: NeilBrown
Acked-by: Mimi Zohar
Acked-by: Serge Hallyn
Signed-off-by: Greg Kroah-Hartman
17 May, 2010
1 commit
-
The ACPI dependency moved to the TPM, where it belongs. Although
IMA per-se does not require access to the bios measurement log,
verifying the IMA boot aggregate does, which requires ACPI.This patch prereq's 'TPM: ACPI/PNP dependency removal'
http://lkml.org/lkml/2010/5/4/378.Signed-off-by: Mimi Zohar
Reported-by: Jean-Christophe Dubois
Acked-by: Serge Hallyn
Tested-by: Serge Hallyn
Signed-off-by: James Morris
07 May, 2010
1 commit
-
This reverts commit a674fa46c79ffa37995bd1c8e4daa2b3be5a95ae.
Previous revert was a prereq.
Signed-off-by: James Morris
06 May, 2010
1 commit
05 May, 2010
1 commit
-
The ACPI dependency moved to the TPM, where it belongs. Although
IMA per-se does not require access to the bios measurement log,
verifying the IMA boot aggregate does, which requires ACPI.This patch prereq's 'TPM: ACPI/PNP dependency removal'
http://lkml.org/lkml/2010/5/4/378.Signed-off-by: Mimi Zohar
Reported-by: Jean-Christophe Dubois
Acked-by: Serge Hallyn
Tested-by: Serge Hallyn
Signed-off-by: James Morris
23 Apr, 2010
1 commit
-
As an example IMA emits a warning when it can't find a TPM chip:
"No TPM chip found, activating TPM-bypass!"
This patch prefaces that message with IMA so we know what subsystem is
bypassing the TPM. Do this for all pr_info and pr_err messages.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris
21 Apr, 2010
8 commits
-
integrity_audit_msg() uses "integrity:" in the audit message. This
violates the (loosely defined) audit system requirements that everything be
a key=value pair and it doesn't provide additional information. This can
be obviously gleaned from the message type. Just drop it.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
Convert all of the places IMA calls audit_log_format with %s into
audit_log_untrusted_string(). This is going to cause them all to get
quoted, but it should make audit log injection harder.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
IMA policy load parser will reject any policies with a comment. This patch
will allow the parser to just ignore lines which start with a #. This is not
very robust. # can ONLY be used at the very beginning of a line. Inline
comments are not allowed.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
IMA parser will fail if whitespace is used in any way other than a single
space. Using a tab or even using 2 spaces in a row will result in a policy
being rejected. This patch makes the kernel ignore whitespace a bit better.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
Currently the ima policy load code will print what it doesn't understand
but really I think it should reject any policy it doesn't understand. This
patch makes it so!Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
ima_parse_rule currently sets entry->action = -1 and then later tests
if (entry->action == UNKNOWN). It is true that UNKNOWN == -1 but actually
setting it to UNKNOWN makes a lot more sense in case things change in the
future.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
IMA will accept rules which specify things twice and will only pay
attention to the last one. We should reject such rules.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
Currently IMA will only accept one rule per write(). This patch allows IMA to
accept writes which contain multiple rules but only processes one rule per
write. \n is used as the delimiter between rules. IMA will return a short
write indicating that it only accepted up to the first \n.This allows simple userspace utilities like cat to be used to load an IMA
policy instead of needing a special userspace utility that understood 'one
write per rule'Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris
30 Mar, 2010
1 commit
-
…it slab.h inclusion from percpu.h
percpu.h is included by sched.h and module.h and thus ends up being
included when building most .c files. percpu.h includes slab.h which
in turn includes gfp.h making everything defined by the two files
universally available and complicating inclusion dependencies.percpu.h -> slab.h dependency is about to be removed. Prepare for
this change by updating users of gfp and slab facilities include those
headers directly instead of assuming availability. As this conversion
needs to touch large number of source files, the following script is
used as the basis of conversion.http://userweb.kernel.org/~tj/misc/slabh-sweep.py
The script does the followings.
* Scan files for gfp and slab usages and update includes such that
only the necessary includes are there. ie. if only gfp is used,
gfp.h, if slab is used, slab.h.* When the script inserts a new include, it looks at the include
blocks and try to put the new include such that its order conforms
to its surrounding. It's put in the include block which contains
core kernel includes, in the same order that the rest are ordered -
alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
doesn't seem to be any matching order.* If the script can't find a place to put a new include (mostly
because the file doesn't have fitting include block), it prints out
an error message indicating which .h file needs to be added to the
file.The conversion was done in the following steps.
1. The initial automatic conversion of all .c files updated slightly
over 4000 files, deleting around 700 includes and adding ~480 gfp.h
and ~3000 slab.h inclusions. The script emitted errors for ~400
files.2. Each error was manually checked. Some didn't need the inclusion,
some needed manual addition while adding it to implementation .h or
embedding .c file was more appropriate for others. This step added
inclusions to around 150 files.3. The script was run again and the output was compared to the edits
from #2 to make sure no file was left behind.4. Several build tests were done and a couple of problems were fixed.
e.g. lib/decompress_*.c used malloc/free() wrappers around slab
APIs requiring slab.h to be added manually.5. The script was run on all .h files but without automatically
editing them as sprinkling gfp.h and slab.h inclusions around .h
files could easily lead to inclusion dependency hell. Most gfp.h
inclusion directives were ignored as stuff from gfp.h was usually
wildly available and often used in preprocessor macros. Each
slab.h inclusion directive was examined and added manually as
necessary.6. percpu.h was updated not to include slab.h.
7. Build test were done on the following configurations and failures
were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
distributed build env didn't work with gcov compiles) and a few
more options had to be turned off depending on archs to make things
build (like ipr on powerpc/64 which failed due to missing writeq).* x86 and x86_64 UP and SMP allmodconfig and a custom test config.
* powerpc and powerpc64 SMP allmodconfig
* sparc and sparc64 SMP allmodconfig
* ia64 SMP allmodconfig
* s390 SMP allmodconfig
* alpha SMP allmodconfig
* um on x86_64 SMP allmodconfig8. percpu.h modifications were reverted so that it could be applied as
a separate patch and serve as bisection point.Given the fact that I had only a couple of failures from tests on step
6, I'm fairly confident about the coverage of this conversion patch.
If there is a breakage, it's likely to be something in one of the arch
headers which should be easily discoverable easily on most builds of
the specific arch.Signed-off-by: Tejun Heo <tj@kernel.org>
Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
10 Mar, 2010
1 commit
-
As noted by checkpatch.pl, __func__ should be used instead of gcc
specific __FUNCTION__.Signed-off-by: H Hartley Sweeten
Signed-off-by: James Morris
25 Feb, 2010
1 commit
-
If radix_tree_preload is failed in ima_inode_alloc, we don't need
radix_tree_preload_end because kernel is alread preempt enabledSigned-off-by: Xiaotian Feng
Signed-off-by: Mimi Zohar
Signed-off-by: James Morris
07 Feb, 2010
4 commits
-
With the movement of the ima hooks functions were renamed from *path* to
*file* since they always deal with struct file. This patch renames some of
the ima internal flags to make them consistent with the rest of the code.Signed-off-by: Mimi Zohar
Signed-off-by: Eric Paris
Signed-off-by: Al Viro -
ima_path_check actually deals with files! call it ima_file_check instead.
Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Al Viro -
ima wants to create an inode information struct (iint) when inodes are
allocated. This means that at least the part of ima which does this
allocation (the allocation is filled with information later) should
before any inodes are created. To accomplish this we split the ima
initialization routine placing the kmem cache allocator inside a
security_initcall() function. Since this makes use of radix trees we also
need to make sure that is initialized before security_initcall().Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: Al Viro -
The "Untangling ima mess, part 2 with counters" patch messed
up the counters. Based on conversations with Al Viro, this patch
streamlines ima_path_check() by removing the counter maintaince.
The counters are now updated independently, from measuring the file,
in __dentry_open() and alloc_file() by calling ima_counts_get().
ima_path_check() is called from nfsd and do_filp_open().
It also did not measure all files that should have been measured.
Reason: ima_path_check() got bogus value passed as mask.
[AV: mea culpa]
[AV: add missing nfsd bits]Signed-off-by: Mimi Zohar
Signed-off-by: Al Viro