On 1/23/2015 8:20 AM, Jim Davis wrote:
> Building with the attached random configuration file,
>
> security/smack/smack_netfilter.c: In function ‘smack_ipv4_output’:
> security/smack/smack_netfilter.c:55:6: error: ‘struct sk_buff’ has no
> member named ‘secmark’
> skb->secmark = skp->smk_secid;
> ^
> make[2]: *** [security/smack/smack_netfilter.o] Error 1
The existing Makefile used the wrong configuration option to
determine if smack_netfilter should be built. This sets it right.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
We hit use after free on dereferncing pointer to task_smack struct in
smk_of_task() called from smack_task_to_inode().
task_security() macro uses task_cred_xxx() to get pointer to the task_smack.
task_cred_xxx() could be used only for non-pointer members of task's
credentials. It cannot be used for pointer members since what they point
to may disapper after dropping RCU read lock.
Mainly task_security() used this way:
smk_of_task(task_security(p))
Intead of this introduce function smk_of_task_struct() which
takes task_struct as argument and returns pointer to smk_known struct
and do this under RCU read lock.
Bogus task_security() macro is not used anymore, so remove it.
KASan's report for this:
AddressSanitizer: use after free in smack_task_to_inode+0x50/0x70 at addr c4635600
=============================================================================
BUG kmalloc-64 (Tainted: PO): kasan error
-----------------------------------------------------------------------------
Disabling lock debugging due to kernel taint
INFO: Allocated in new_task_smack+0x44/0xd8 age=39 cpu=0 pid=1866
kmem_cache_alloc_trace+0x88/0x1bc
new_task_smack+0x44/0xd8
smack_cred_prepare+0x48/0x21c
security_prepare_creds+0x44/0x4c
prepare_creds+0xdc/0x110
smack_setprocattr+0x104/0x150
security_setprocattr+0x4c/0x54
proc_pid_attr_write+0x12c/0x194
vfs_write+0x1b0/0x370
SyS_write+0x5c/0x94
ret_fast_syscall+0x0/0x48
INFO: Freed in smack_cred_free+0xc4/0xd0 age=27 cpu=0 pid=1564
kfree+0x270/0x290
smack_cred_free+0xc4/0xd0
security_cred_free+0x34/0x3c
put_cred_rcu+0x58/0xcc
rcu_process_callbacks+0x738/0x998
__do_softirq+0x264/0x4cc
do_softirq+0x94/0xf4
irq_exit+0xbc/0x120
handle_IRQ+0x104/0x134
gic_handle_irq+0x70/0xac
__irq_svc+0x44/0x78
_raw_spin_unlock+0x18/0x48
sync_inodes_sb+0x17c/0x1d8
sync_filesystem+0xac/0xfc
vdfs_file_fsync+0x90/0xc0
vfs_fsync_range+0x74/0x7c
INFO: Slab 0xd3b23f50 objects=32 used=31 fp=0xc4635600 flags=0x4080
INFO: Object 0xc4635600 @offset=5632 fp=0x (null)
Bytes b4 c46355f0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
Object c4635600: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object c4635610: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object c4635620: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object c4635630: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
Redzone c4635640: bb bb bb bb ....
Padding c46356e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
Padding c46356f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
CPU: 5 PID: 834 Comm: launchpad_prelo Tainted: PBO 3.10.30 #1
Backtrace:
[<c00233a4>] (dump_backtrace+0x0/0x158) from [<c0023dec>] (show_stack+0x20/0x24)
r7:c4634010 r6:d3b23f50 r5:c4635600 r4:d1002140
[<c0023dcc>] (show_stack+0x0/0x24) from [<c06d6d7c>] (dump_stack+0x20/0x28)
[<c06d6d5c>] (dump_stack+0x0/0x28) from [<c01c1d50>] (print_trailer+0x124/0x144)
[<c01c1c2c>] (print_trailer+0x0/0x144) from [<c01c1e88>] (object_err+0x3c/0x44)
r7:c4635600 r6:d1002140 r5:d3b23f50 r4:c4635600
[<c01c1e4c>] (object_err+0x0/0x44) from [<c01cac18>] (kasan_report_error+0x2b8/0x538)
r6:d1002140 r5:d3b23f50 r4:c6429cf8 r3:c09e1aa7
[<c01ca960>] (kasan_report_error+0x0/0x538) from [<c01c9430>] (__asan_load4+0xd4/0xf8)
[<c01c935c>] (__asan_load4+0x0/0xf8) from [<c031e168>] (smack_task_to_inode+0x50/0x70)
r5:c4635600 r4:ca9da000
[<c031e118>] (smack_task_to_inode+0x0/0x70) from [<c031af64>] (security_task_to_inode+0x3c/0x44)
r5:cca25e80 r4:c0ba9780
[<c031af28>] (security_task_to_inode+0x0/0x44) from [<c023d614>] (pid_revalidate+0x124/0x178)
r6:00000000 r5:cca25e80 r4:cbabe3c0 r3:00008124
[<c023d4f0>] (pid_revalidate+0x0/0x178) from [<c01db98c>] (lookup_fast+0x35c/0x43y4)
r9:c6429efc r8:00000101 r7:c079d940 r6:c6429e90 r5:c6429ed8 r4:c83c4148
[<c01db630>] (lookup_fast+0x0/0x434) from [<c01deec8>] (do_last.isra.24+0x1c0/0x1108)
[<c01ded08>] (do_last.isra.24+0x0/0x1108) from [<c01dff04>] (path_openat.isra.25+0xf4/0x648)
[<c01dfe10>] (path_openat.isra.25+0x0/0x648) from [<c01e1458>] (do_filp_open+0x3c/0x88)
[<c01e141c>] (do_filp_open+0x0/0x88) from [<c01ccb28>] (do_sys_open+0xf0/0x198)
r7:00000001 r6:c0ea2180 r5:0000000b r4:00000000
[<c01cca38>] (do_sys_open+0x0/0x198) from [<c01ccc00>] (SyS_open+0x30/0x34)
[<c01ccbd0>] (SyS_open+0x0/0x34) from [<c001db80>] (ret_fast_syscall+0x0/0x48)
Read of size 4 by thread T834:
Memory state around the buggy address:
c4635380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
c4635400: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
c4635480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
c4635500: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
c4635580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>c4635600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
c4635680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
c4635700: 00 00 00 00 04 fc fc fc fc fc fc fc fc fc fc fc
c4635780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
c4635800: 00 00 00 00 00 00 04 fc fc fc fc fc fc fc fc fc
c4635880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
Cc: <stable@vger.kernel.org>
During UDS connection check, both sides are checked for write access to
the other side. But only the first check is performed with audit support.
The second one didn't produce any audit logs. This simple patch fixes that.
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
Smack uses CIPSO to label internet packets and thus provide
for access control on delivery of packets. The netfilter facility
was not used to allow for Smack to work properly without netfilter
configuration. Smack does not need netfilter, however there are
cases where it would be handy.
As a side effect, the labeling of local IPv4 packets can be optimized
and the handling of local IPv6 packets is just all out better.
The best part is that the netfilter tools use "contexts" that
are just strings, and they work just as well for Smack as they
do for SELinux.
All of the conditional compilation for IPv6 was implemented
by Rafal Krypa <r.krypa@samsung.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
This is one of those cases where you look at code you did
years ago and wonder what you might have been thinking.
There are a number of LSM hooks that work off of file pointers,
and most of them really want the security data from the inode.
Some, however, really want the security context that the process
had when the file was opened. The difference went undetected in
Smack until it started getting used in a real system with real
testing. At that point it was clear that something was amiss.
This patch corrects the misuse of the f_security value in several
of the hooks. The behavior will not usually be any different, as
the process had to be able to open the file in the first place, and
the old check almost always succeeded, as will the new, but for
different reasons.
Thanks to the Samsung Tizen development team that identified this.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The 54e70ec5eb commit introduced a
bidirectional check that should have checked for mutual WRITE access
between two labels. Due to a typo subject's OUT label is checked with
object's OUT. Should be OUT to IN.
Signed-off-by: Zbigniew Jasinski <z.jasinski@samsung.com>
Files created with __shmem_file_stup() appear to have somewhat fake
dentries which make them look like root directories and not get
the label the current process or ("*") star meant for tmpfs files.
Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
In principle if this function was called with "value" == NULL and "len"
not NULL it could return different results for the "len" compared to a
case where "name" was not NULL. This is a hypothetical case that does
not exist in the kernel, but it's a logic bug nonetheless.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
Pull security layer updates from James Morris:
"In terms of changes, there's general maintenance to the Smack,
SELinux, and integrity code.
The IMA code adds a new kconfig option, IMA_APPRAISE_SIGNED_INIT,
which allows IMA appraisal to require signatures. Support for reading
keys from rootfs before init is call is also added"
* 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (23 commits)
selinux: Remove security_ops extern
security: smack: fix out-of-bounds access in smk_parse_smack()
VFS: refactor vfs_read()
ima: require signature based appraisal
integrity: provide a hook to load keys when rootfs is ready
ima: load x509 certificate from the kernel
integrity: provide a function to load x509 certificate from the kernel
integrity: define a new function integrity_read_file()
Security: smack: replace kzalloc with kmem_cache for inode_smack
Smack: Lock mode for the floor and hat labels
ima: added support for new kernel cmdline parameter ima_template_fmt
ima: allocate field pointers array on demand in template_desc_init_fields()
ima: don't allocate a copy of template_fmt in template_desc_init_fields()
ima: display template format in meas. list if template name length is zero
ima: added error messages to template-related functions
ima: use atomic bit operations to protect policy update interface
ima: ignore empty and with whitespaces policy lines
ima: no need to allocate entry for comment
ima: report policy load status
ima: use path names cache
...
The patch use kmem_cache to allocate/free inode_smack since they are
alloced in high volumes making it a perfect case for kmem_cache.
As per analysis, 24 bytes of memory is wasted per allocation due
to internal fragmentation. With kmem_cache, this can be avoided.
Accounting of memory allocation is below :
total slack net count-alloc/free caller
Before (with kzalloc)
1919872 719952 1919872 29998/0 new_inode_smack+0x14
After (with kmem_cache)
1201680 0 1201680 30042/0 new_inode_smack+0x18
>From above data, we found that 719952 bytes(~700 KB) of memory is
saved on allocation of 29998 smack inodes.
Signed-off-by: Rohit <rohit.kr@samsung.com>
The lock access mode allows setting a read lock on a file
for with the process has only read access. The floor label is
defined to make it easy to have the basic system installed such
that everyone can read it. Once there's a desire to read lock
(rationally or otherwise) a floor file a rule needs to get set.
This happens all the time, so make the floor label a little bit
more special and allow everyone lock access, too. By implication,
give processes with the hat label (hat can read everything)
lock access as well. This reduces clutter in the Smack rule set.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Pull security subsystem updates from James Morris.
Mostly ima, selinux, smack and key handling updates.
* 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (65 commits)
integrity: do zero padding of the key id
KEYS: output last portion of fingerprint in /proc/keys
KEYS: strip 'id:' from ca_keyid
KEYS: use swapped SKID for performing partial matching
KEYS: Restore partial ID matching functionality for asymmetric keys
X.509: If available, use the raw subjKeyId to form the key description
KEYS: handle error code encoded in pointer
selinux: normalize audit log formatting
selinux: cleanup error reporting in selinux_nlmsg_perm()
KEYS: Check hex2bin()'s return when generating an asymmetric key ID
ima: detect violations for mmaped files
ima: fix race condition on ima_rdwr_violation_check and process_measurement
ima: added ima_policy_flag variable
ima: return an error code from ima_add_boot_aggregate()
ima: provide 'ima_appraise=log' kernel option
ima: move keyring initialization to ima_init()
PKCS#7: Handle PKCS#7 messages that contain no X.509 certs
PKCS#7: Better handling of unsupported crypto
KEYS: Overhaul key identification when searching for asymmetric keys
KEYS: Implement binary asymmetric key ID handling
...
security_file_set_fowner always returns 0, so make it f_setown and
__f_setown void return functions and fix up the error handling in the
callers.
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Smack used to use a mix of smack_known struct and char* throughout its
APIs and implementation. This patch unifies the behaviour and makes it
store and operate exclusively on smack_known struct pointers when managing
labels.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
Conflicts:
security/smack/smack_access.c
security/smack/smack_lsm.c
The 54e70ec5eb commit introduced a
bidirectional check that should have checked for mutual WRITE access
between two labels. Due to a typo the second check was incorrect.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
People keep asking me for permissive mode, and I keep saying "no".
Permissive mode is wrong for more reasons than I can enumerate,
but the compelling one is that it's once on, never off.
Nonetheless, there is an argument to be made for running a
process with lots of permissions, logging which are required,
and then locking the process down. There wasn't a way to do
that with Smack, but this provides it.
The notion is that you start out by giving the process an
appropriate Smack label, such as "ATBirds". You create rules
with a wide range of access and the "b" mode. On Tizen it
might be:
ATBirds System rwxalb
ATBirds User rwxalb
ATBirds _ rwxalb
User ATBirds wb
System ATBirds wb
Accesses that fail will generate audit records. Accesses
that succeed because of rules marked with a "b" generate
log messages identifying the rule, the program and as much
object information as is convenient.
When the system is properly configured and the programs
brought in line with the labeling scheme the "b" mode can
be removed from the rules. When the system is ready for
production the facility can be configured out.
This provides the developer the convenience of permissive
mode without creating a system that looks like it is
enforcing a policy while it is not.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
While opening with CAP_MAC_OVERRIDE file label is not set.
Other calls may access it after CAP_MAC_OVERRIDE is dropped from process.
Signed-off-by: Marcin Niesluchowski <m.niesluchow@samsung.com>
Values of extended attributes are stored as binary blobs. NULL-termination
of them isn't required. It just wastes disk space and confuses command-line
tools like getfattr because they have to print that zero byte at the end.
This patch removes terminating zero byte from initial security label in
smack_inode_init_security and cuts it out in function smack_inode_getsecurity
which is used by syscall getxattr. This change seems completely safe, because
function smk_parse_smack ignores everything after first zero byte.
Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" file
And after writing zero-length string into smackfs files syslog and onlycp:
# python -c 'import os; os.write(1, "")' > /smack/syslog
The problem is caused by brain-damaged logic in function smk_parse_smack()
which takes pointer to buffer and its length but if length below or equal zero
it thinks that the buffer is zero-terminated. Unfortunately callers of this
function are widely used and proper fix requires serious refactoring.
Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Security operation ->inode_listsecurity is used for generating list of
available extended attributes for syscall listxattr. Currently it's used
only in nfs4 or if filesystem doesn't provide i_op->listxattr.
The list is the set of NULL-terminated names, one after the other.
This method must include zero byte at the and into result.
Also this function must return length even if string does not fit into
output buffer or it is NULL, see similar method in selinux and man listxattr.
Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Historically the NetLabel LSM secattr catmap functions and data
structures have had very long names which makes a mess of the NetLabel
code and anyone who uses NetLabel. This patch renames the catmap
functions and structures from "*_secattr_catmap_*" to just "*_catmap_*"
which improves things greatly.
There are no substantial code or logic changes in this patch.
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The NetLabel secattr catmap functions, and the SELinux import/export
glue routines, were broken in many horrible ways and the SELinux glue
code fiddled with the NetLabel catmap structures in ways that we
probably shouldn't allow. At some point this "worked", but that was
likely due to a bit of dumb luck and sub-par testing (both inflicted
by yours truly). This patch corrects these problems by basically
gutting the code in favor of something less obtuse and restoring the
NetLabel abstractions in the SELinux catmap glue code.
Everything is working now, and if it decides to break itself in the
future this code will be much easier to debug than the code it
replaces.
One noteworthy side effect of the changes is that it is no longer
necessary to allocate a NetLabel catmap before calling one of the
NetLabel APIs to set a bit in the catmap. NetLabel will automatically
allocate the catmap nodes when needed, resulting in less allocations
when the lowest bit is greater than 255 and less code in the LSMs.
Cc: stable@vger.kernel.org
Reported-by: Christian Evans <frodox@zoho.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The NetLabel category (catmap) functions have a problem in that they
assume categories will be set in an increasing manner, e.g. the next
category set will always be larger than the last. Unfortunately, this
is not a valid assumption and could result in problems when attempting
to set categories less than the startbit in the lowest catmap node.
In some cases kernel panics and other nasties can result.
This patch corrects the problem by checking for this and allocating a
new catmap node instance and placing it at the front of the list.
Cc: stable@vger.kernel.org
Reported-by: Christian Evans <frodox@zoho.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The cgroup filesystem isn't ready for an LSM to
properly use extented attributes. This patch makes
files created in the cgroup filesystem usable by
a system running Smack and systemd.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Smack believes that many of the operatons that can
be performed on an open file descriptor are read operations.
The fstat and lseek system calls are examples.
An implication of this is that files shouldn't be open
if the task doesn't have read access even if it has
write access and the file is being opened write only.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Smack IPC policy requires that the sender have write access
to the receiver. UDS streams don't do per-packet checks. The
only check is done at connect time. The existing code checks
if the connecting process can write to the other, but not the
other way around. This change adds a check that the other end
can write to the connecting process.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schuafler <casey@schaufler-ca.com>
Sam Henderson points out that removing the SMACK64TRANSMUTE
attribute from a directory does not result in the directory
transmuting. This is because the inode flag indicating that
the directory is transmuting isn't cleared. The fix is a tad
less than trivial because smk_task and smk_mmap should have
been broken out, too.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The function `smack_inode_post_setxattr` is called each
time that a setxattr is done, for any value of name.
The kernel allow to put value==NULL when size==0
to set an empty attribute value. The systematic
call to smk_import_entry was causing the dereference
of a NULL pointer hence a KERNEL PANIC!
The problem can be produced easily by issuing the
command `setfattr -n user.data file` under bash prompt
when SMACK is active.
Moving the call to smk_import_entry as proposed by this
patch is correcting the behaviour because the function
smack_inode_post_setxattr is called for the SMACK's
attributes only if the function smack_inode_setxattr validated
the value and its size (what will not be the case when size==0).
It also has a benefical effect to not fill the smack hash
with garbage values coming from any extended attribute
write.
Change-Id: Iaf0039c2be9bccb6cee11c24a3b44d209101fe47
Signed-off-by: José Bollo <jose.bollo@open.eurogiciel.org>
1. In order to remove any SMACK extended attribute from a file, a user
should have CAP_MAC_ADMIN capability. But user without having this
capability is able to remove SMACK64MMAP security attribute.
2. While validating size and value of smack extended attribute in
smack_inode_setsecurity hook, wrong error code is returned.
Signed-off-by: Pankaj Kumar <pamkaj.k2@samsung.com>
Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com>
This allows to limit ptrace beyond the regular smack access rules.
It adds a smackfs/ptrace interface that allows smack to be configured
to require equal smack labels for PTRACE_MODE_ATTACH access.
See the changes in Documentation/security/Smack.txt below for details.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
The decision whether we can trace a process is made in the following
functions:
smack_ptrace_traceme()
smack_ptrace_access_check()
smack_bprm_set_creds() (in case the proces is traced)
This patch unifies all those decisions by introducing one function that
checks whether ptrace is allowed: smk_ptrace_rule_check().
This makes possible to actually trace with TRACEME where first the
TRACEME itself must be allowed and then exec() on a traced process.
Additional bugs fixed:
- The decision is made according to the mode parameter that is now correctly
translated from PTRACE_MODE_* to MAY_* instead of being treated 1:1.
PTRACE_MODE_READ requires MAY_READ.
PTRACE_MODE_ATTACH requires MAY_READWRITE.
- Add a smack audit log in case of exec() refused by bprm_set_creds().
- Honor the PTRACE_MODE_NOAUDIT flag and don't put smack audit info
in case this flag is set.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
The order of subject/object is currently reversed in
smack_ptrace_traceme(). It is currently checked if the tracee has a
capability to trace tracer and according to this rule a decision is made
whether the tracer will be allowed to trace tracee.
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
For any keyring access type SMACK always used MAY_READWRITE access check.
It prevents reading the key with label "_", which should be allowed for anyone.
This patch changes default access check to MAY_READ and use MAY_READWRITE in only
appropriate cases.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Move the flags representing required permission to linux/key.h as the perm
parameter of security_key_permission() is in terms of them - and not the
permissions mask flags used in key->perm.
Whilst we're at it:
(1) Rename them to be KEY_NEED_xxx rather than KEY_xxx to avoid collisions
with symbols in uapi/linux/input.h.
(2) Don't use key_perm_t for a mask of required permissions, but rather limit
it to the permissions mask attached to the key and arguments related
directly to that.
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Pull audit update from Eric Paris:
"Again we stayed pretty well contained inside the audit system.
Venturing out was fixing a couple of function prototypes which were
inconsistent (didn't hurt anything, but we used the same value as an
int, uint, u32, and I think even a long in a couple of places).
We also made a couple of minor changes to when a couple of LSMs called
the audit system. We hoped to add aarch64 audit support this go
round, but it wasn't ready.
I'm disappearing on vacation on Thursday. I should have internet
access, but it'll be spotty. If anything goes wrong please be sure to
cc rgb@redhat.com. He'll make fixing things his top priority"
* git://git.infradead.org/users/eparis/audit: (50 commits)
audit: whitespace fix in kernel-parameters.txt
audit: fix location of __net_initdata for audit_net_ops
audit: remove pr_info for every network namespace
audit: Modify a set of system calls in audit class definitions
audit: Convert int limit uses to u32
audit: Use more current logging style
audit: Use hex_byte_pack_upper
audit: correct a type mismatch in audit_syscall_exit()
audit: reorder AUDIT_TTY_SET arguments
audit: rework AUDIT_TTY_SET to only grab spin_lock once
audit: remove needless switch in AUDIT_SET
audit: use define's for audit version
audit: documentation of audit= kernel parameter
audit: wait_for_auditd rework for readability
audit: update MAINTAINERS
audit: log task info on feature change
audit: fix incorrect set of audit_sock
audit: print error message when fail to create audit socket
audit: fix dangling keywords in audit_log_set_loginuid() output
audit: log on errors from filter user rules
...
Remove the call to audit_log() (which call audit_log_start()) and deal with
the errors in the caller, logging only once if the condition is met. Calling
audit_log_start() in this location makes buffer allocation and locking more
complicated in the calling tree (audit_filter_user()).
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
Eric Paris politely points out:
Inside smack_file_receive() it seems like you are initting the audit
field with LSM_AUDIT_DATA_TASK. And then use
smk_ad_setfield_u_fs_path().
Seems like LSM_AUDIT_DATA_PATH would make more sense. (and depending
on how it's used fix a crash...)
He is correct. This puts things in order.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The mount restrictions imposed by Smack rely heavily on the
use of the filesystem "floor", which is the label that all
processes writing to the filesystem must have access to. It
turns out that while the "floor" notion is sound, it has yet
to be fully implemented and has never been used.
The sb_mount and sb_umount hooks only make sense if the
filesystem floor is used actively, and it isn't. They can
be reintroduced if a rational restriction comes up. Until
then, they get removed.
The sb_kern_mount hook is required for the option processing.
It is too permissive in the case of unprivileged mounts,
effectively bypassing the CAP_MAC_ADMIN restrictions if
any of the smack options are specified. Unprivileged mounts
are no longer allowed to set Smack filesystem options.
Additionally, the root and default values are set to the
label of the caller, in keeping with the policy that objects
get the label of their creator.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
smk_write_change_rule() is calling capable rather than
the more correct smack_privileged(). This allows for setting
rules in violation of the onlycap facility. This is the
simple repair.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The syslog control requires that the calling proccess
have the floor ("_") Smack label. Tizen does not run any
processes except for kernel helpers with the floor label.
This changes allows the admin to configure a specific
label for syslog. The default value is the star ("*")
label, effectively removing the restriction. The value
can be set using smackfs/syslog for anyone who wants
a more restrictive behavior.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Smack prohibits processes from using the star ("*") and web ("@") labels
because we don't want files with those labels getting created implicitly.
All setting of those labels should be done explicitly. The trouble is that
there is no check for these labels in the processing of SMACK64EXEC. That
is repaired.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
This is a regression caused by f7112e6c. When either subject or
object is not found the answer for access should be no. This
patch fixes the situation. '0' is written back instead of failing
with -EINVAL.
v2: cosmetic style fixes
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
When the ptrace security hooks were split the addition of
a mode parameter was not taken advantage of in the Smack
ptrace access check. This changes the access check from
always looking for read and write access to using the
passed mode. This will make use of /proc much happier.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Linux file locking does not follow the same rules
as other mechanisms. Even though it is a write operation
a process can set a read lock on files which it has open
only for read access. Two programs with read access to
a file can use read locks to communicate.
This is not acceptable in a Mandatory Access Control
environment. Smack treats setting a read lock as the
write operation that it is. Unfortunately, many programs
assume that setting a read lock is a read operation.
These programs are unhappy in the Smack environment.
This patch introduces a new access mode (lock) to address
this problem. A process with lock access to a file can
set a read lock. A process with write access to a file can
set a read lock or a write lock. This prevents a situation
where processes are granted write access just so they can
set read locks.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Pull security subsystem updates from James Morris:
"Nothing major for this kernel, just maintenance updates"
* 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (21 commits)
apparmor: add the ability to report a sha1 hash of loaded policy
apparmor: export set of capabilities supported by the apparmor module
apparmor: add the profile introspection file to interface
apparmor: add an optional profile attachment string for profiles
apparmor: add interface files for profiles and namespaces
apparmor: allow setting any profile into the unconfined state
apparmor: make free_profile available outside of policy.c
apparmor: rework namespace free path
apparmor: update how unconfined is handled
apparmor: change how profile replacement update is done
apparmor: convert profile lists to RCU based locking
apparmor: provide base for multiple profiles to be replaced at once
apparmor: add a features/policy dir to interface
apparmor: enable users to query whether apparmor is enabled
apparmor: remove minimum size check for vmalloc()
Smack: parse multiple rules per write to load2, up to PAGE_SIZE-1 bytes
Smack: network label match fix
security: smack: add a hash table to quicken smk_find_entry()
security: smack: fix memleak in smk_write_rules_list()
xattr: Constify ->name member of "struct xattr".
...
Smack interface for loading rules has always parsed only single rule from
data written to it. This requires user program to call one write() per
each rule it wants to load.
This change makes it possible to write multiple rules, separated by new
line character. Smack will load at most PAGE_SIZE-1 characters and properly
return number of processed bytes. In case when user buffer is larger, it
will be additionally truncated. All characters after last \n will not get
parsed to avoid partial rule near input buffer boundary.
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
The original implementation of the Smack IPv6 port based
local controls works most of the time using a sockaddr as
a temporary variable, but not always as it overflows in
some circumstances. The correct data is a sockaddr_in6.
A struct sockaddr isn't as large as a struct sockaddr_in6.
There would need to be casting one way or the other. This
patch gets it the right way.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
The Smack code that matches incoming CIPSO tags with Smack labels
reaches through the NetLabel interfaces and compares the network
data with the CIPSO header associated with a Smack label. This was
done in a ill advised attempt to optimize performance. It works
so long as the categories fit in a single capset, but this isn't
always the case.
This patch changes the Smack code to use the appropriate NetLabel
interfaces to compare the incoming CIPSO header with the CIPSO
header associated with a label. It will always match the CIPSO
headers correctly.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Accepted for the smack-next tree after changing the number of
slots from 128 to 16.
This patch adds a hash table to quicken searching of a smack label by its name.
Basically, the patch improves performance of SMACK initialization. Parsing of
rules involves translation from a string to a smack_known (aka label) entity
which is done in smk_find_entry().
The current implementation of the function iterates over a global list of
smack_known resulting in O(N) complexity for smk_find_entry(). The total
complexity of SMACK initialization becomes O(rules * labels). Therefore it
scales quadratically with a complexity of a system.
Applying the patch reduced the complexity of smk_find_entry() to O(1) as long
as number of label is in hundreds. If the number of labels is increased please
update SMACK_HASH_SLOTS constant defined in security/smack/smack.h. Introducing
the configuration of this constant with Kconfig or cmdline might be a good
idea.
The size of the hash table was adjusted experimentally. The rule set used by
TIZEN contains circa 17K rules for 500 labels. The table above contains
results of SMACK initialization using 'time smackctl apply' bash command.
The 'Ref' is a kernel without this patch applied. The consecutive values
refers to value of SMACK_HASH_SLOTS. Every measurement was repeated three
times to reduce noise.
| Ref | 1 | 2 | 4 | 8 | 16 | 32 | 64 | 128 | 256 | 512
--------------------------------------------------------------------------------------------
Run1 | 1.156 | 1.096 | 0.883 | 0.764 | 0.692 | 0.667 | 0.649 | 0.633 | 0.634 | 0.629 | 0.620
Run2 | 1.156 | 1.111 | 0.885 | 0.764 | 0.694 | 0.661 | 0.649 | 0.651 | 0.634 | 0.638 | 0.623
Run3 | 1.160 | 1.107 | 0.886 | 0.764 | 0.694 | 0.671 | 0.661 | 0.638 | 0.631 | 0.624 | 0.638
AVG | 1.157 | 1.105 | 0.885 | 0.764 | 0.693 | 0.666 | 0.653 | 0.641 | 0.633 | 0.630 | 0.627
Surprisingly, a single hlist is slightly faster than a double-linked list.
The speed-up saturates near 64 slots. Therefore I chose value 128 to provide
some margin if more labels were used.
It looks that IO becomes a new bottleneck.
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
The smack_parsed_rule structure is allocated. If a rule is successfully
installed then the last reference to the object is lost. This patch fixes this
leak. Moreover smack_parsed_rule is allocated on stack because it no longer
needed ofter smk_write_rules_list() is finished.
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Since everybody sets kstrdup()ed constant string to "struct xattr"->name but
nobody modifies "struct xattr"->name , we can omit kstrdup() and its failure
checking by constifying ->name member of "struct xattr".
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Joel Becker <jlbec@evilplan.org> [ocfs2]
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Reviewed-by: Paul Moore <paul@paul-moore.com>
Tested-by: Paul Moore <paul@paul-moore.com>
Acked-by: Eric Paris <eparis@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Feature highlights include:
- Add basic client support for NFSv4.2
- Add basic client support for Labeled NFS (selinux for NFSv4.2)
- Fix the use of credentials in NFSv4.1 stateful operations, and
add support for NFSv4.1 state protection.
Bugfix highlights:
- Fix another NFSv4 open state recovery race
- Fix an NFSv4.1 back channel session regression
- Various rpc_pipefs races
- Fix another issue with NFSv3 auth negotiation
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
iQIcBAABAgAGBQJR2vsSAAoJEGcL54qWCgDyWBIP/AqlpBBAblxbNQ1Bl/0m1Pdb
iKH961qgM4U1BzK0svGtHTZqkovpm4o/VbkbKBT5mQ4g6SbbsJ/AsS1plCyfnIZi
bdnKNJyj6zg0NsAkJ3vKWqd4BTaP+icdSfEIlRKQxAPESewN7b5B3OWgY4KdYmnk
q5BP25anC1ryxVycSY67ux8S2IKXVSRZeCZv+RO21rvZ2G0bV5y7t8Om28ztxEnU
RKrHgQHgaaktR7i8QVO0sbiWq3iqLa3GPkUvFLwWGr8PQJtTkYY0QwYSrsV3N4rY
hYpMRUZFHpZ8UG5YvBT6xyOy/XaGwMGKSfZjB9/YG4QVju+tTy50U1JbTil5PEWY
GHWYF68aurIeUkXrhSv8AVnOnhir0mISx5ou/SV7p0QoAZ92V6kq+LkPrW520qlc
z8ILh3j28pN3ZUCIEArcaZhYCt48uO2hwBi5TqevQyyGRsXFGbN1moD5jvHkllft
Fi0XGuCBdvhrzFRZcsEl+PDq7fT8lXUK2BHe8oR5jz9PhUp+jpEl9m/eg3RsjJjN
DuxsHye2U4chScdnRtLBQvpFtdINvWX/Gy8Bi7kdE5tsQySvOa+rdwuBc7h88PHC
+4xI2iX3z4O1+GpsAe/T9+pjW689jEilS+eVDRVEGl6yHGn9q8PYOayjPjwbJHxS
R2mLTRhKu1DKguTzO13f
=wGjn
-----END PGP SIGNATURE-----
Merge tag 'nfs-for-3.11-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
Pull NFS client updates from Trond Myklebust:
"Feature highlights include:
- Add basic client support for NFSv4.2
- Add basic client support for Labeled NFS (selinux for NFSv4.2)
- Fix the use of credentials in NFSv4.1 stateful operations, and add
support for NFSv4.1 state protection.
Bugfix highlights:
- Fix another NFSv4 open state recovery race
- Fix an NFSv4.1 back channel session regression
- Various rpc_pipefs races
- Fix another issue with NFSv3 auth negotiation
Please note that Labeled NFS does require some additional support from
the security subsystem. The relevant changesets have all been
reviewed and acked by James Morris."
* tag 'nfs-for-3.11-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs: (54 commits)
NFS: Set NFS_CS_MIGRATION for NFSv4 mounts
NFSv4.1 Refactor nfs4_init_session and nfs4_init_channel_attrs
nfs: have NFSv3 try server-specified auth flavors in turn
nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it
nfs: move server_authlist into nfs_try_mount_request
nfs: refactor "need_mount" code out of nfs_try_mount
SUNRPC: PipeFS MOUNT notification optimization for dying clients
SUNRPC: split client creation routine into setup and registration
SUNRPC: fix races on PipeFS UMOUNT notifications
SUNRPC: fix races on PipeFS MOUNT notifications
NFSv4.1 use pnfs_device maxcount for the objectlayout gdia_maxcount
NFSv4.1 use pnfs_device maxcount for the blocklayout gdia_maxcount
NFSv4.1 Fix gdia_maxcount calculation to fit in ca_maxresponsesize
NFS: Improve legacy idmapping fallback
NFSv4.1 end back channel session draining
NFS: Apply v4.1 capabilities to v4.2
NFSv4.1: Clean up layout segment comparison helper names
NFSv4.1: layout segment comparison helpers should take 'const' parameters
NFSv4: Move the DNS resolver into the NFSv4 module
rpc_pipefs: only set rpc_dentry_ops if d_op isn't already set
...
The interface to request security labels from user space is the xattr
interface. When requesting the security label from an NFS server it is
important to make sure the requested xattr actually is a MAC label. This allows
us to make sure that we get the desired semantics from the attribute instead of
something else such as capabilities or a time based LSM.
Acked-by: Eric Paris <eparis@redhat.com>
Acked-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Matthew N. Dodd <Matthew.Dodd@sparta.com>
Signed-off-by: Miguel Rodel Felipe <Rodel_FM@dsi.a-star.edu.sg>
Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene@dsi.a-star.edu.sg>
Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG@dsi.a-star.edu.sg>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Bug report: https://tizendev.org/bugs/browse/TDIS-3891
The reason is userspace libsmack only use "smackfs/cipso2" long-label interface,
but the code's logical is still for orginal fixed length label. Now update
smack_cipso_apply() to support flexible label (<=256 including tailing '\0')
There is also a bug in kernel/security/smack/smackfs.c:
When smk_set_cipso() parsing the CIPSO setting from userspace, the offset of
CIPSO level should be "strlen(label)+1" instead of "strlen(label)"
Signed-off-by: Passion,Zhao <passion.zhao@intel.com>
Suppliment the smkfsroot mount option with another, smkfstransmute,
that does the same thing but also marks the root inode as
transmutting. This allows a freshly created filesystem to
be mounted with a transmutting heirarchy.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Each Smack label that the kernel has seen is added to a
list of labels. The list of access rules for a given subject
label hangs off of the label list entry for the label.
This patch changes the structures that contain subject
labels to point at the label list entry rather that the
label itself. Doing so removes a label list lookup in
smk_access() that was accounting for the largest single
chunk of Smack overhead.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Smack does not provide access controls on IPv6 communications.
This patch introduces a mechanism for maintaining Smack lables
for local IPv6 communications. It is based on labeling local ports.
The behavior should be compatible with any future "real" IPv6
support as it provides no interfaces for users to manipulate
the labeling. Remote IPv6 connections use the ambient label
the same way that unlabeled IPv4 packets are treated.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
As reported for linux-next: Tree for Apr 2 (smack)
Add the required include for smackfs.c
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: James Morris <james.l.morris@oracle.com>
This patch fixes kernel Oops because of wrong common_audit_data type
in smack_inode_unlink() and smack_inode_rmdir().
When SMACK security module is enabled and SMACK logging is on (/smack/logging
is not zero) and you try to delete the file which
1) you cannot delete due to SMACK rules and logging of failures is on
or
2) you can delete and logging of success is on,
you will see following:
Unable to handle kernel NULL pointer dereference at virtual address 000002d7
[<...>] (strlen+0x0/0x28)
[<...>] (audit_log_untrustedstring+0x14/0x28)
[<...>] (common_lsm_audit+0x108/0x6ac)
[<...>] (smack_log+0xc4/0xe4)
[<...>] (smk_curacc+0x80/0x10c)
[<...>] (smack_inode_unlink+0x74/0x80)
[<...>] (security_inode_unlink+0x2c/0x30)
[<...>] (vfs_unlink+0x7c/0x100)
[<...>] (do_unlinkat+0x144/0x16c)
The function smack_inode_unlink() (and smack_inode_rmdir()) need
to log two structures of different types. First of all it does:
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
This will set common audit data type to LSM_AUDIT_DATA_DENTRY
and store dentry for auditing (by function smk_curacc(), which in turn calls
dump_common_audit_data(), which is actually uses provided data and logs it).
/*
* You need write access to the thing you're unlinking
*/
rc = smk_curacc(smk_of_inode(ip), MAY_WRITE, &ad);
if (rc == 0) {
/*
* You also need write access to the containing directory
*/
Then this function wants to log anoter data:
smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
smk_ad_setfield_u_fs_inode(&ad, dir);
The function sets inode field, but don't change common_audit_data type.
rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
}
So the dump_common_audit() function incorrectly interprets inode structure
as dentry, and Oops will happen.
This patch reinitializes common_audit_data structures with correct type.
Also I removed unneeded
smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
initialization, because both dentry and inode pointers are stored
in the same union.
Signed-off-by: Igor Zhbanov <i.zhbanov@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Rule modifications are enabled via /smack/change-rule. Format is as follows:
"Subject Object rwaxt rwaxt"
First two strings are subject and object labels up to 255 characters.
Third string contains permissions to enable.
Fourth string contains permissions to disable.
All unmentioned permissions will be left unchanged.
If no rule previously existed, it will be created.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
This fixes audit logs for granting or denial of permissions to show
information about transmute bit.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
Special file /smack/revoke-subject will silently accept labels that are not
present on the subject label list. Nothing has to be done for such labels,
as there are no rules for them to revoke.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
Pull security subsystem updates from James Morris:
"A quiet cycle for the security subsystem with just a few maintenance
updates."
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security:
Smack: create a sysfs mount point for smackfs
Smack: use select not depends in Kconfig
Yama: remove locking from delete path
Yama: add RCU to drop read locking
drivers/char/tpm: remove tasklet and cleanup
KEYS: Use keyring_alloc() to create special keyrings
KEYS: Reduce initial permissions on keys
KEYS: Make the session and process keyrings per-thread
seccomp: Make syscall skipping and nr changes more consistent
key: Fix resource leak
keys: Fix unreachable code
KEYS: Add payload preparsing opportunity prior to key instantiate or update
There are a number of "conventions" for where to put LSM filesystems.
Smack adheres to none of them. Create a mount point at /sys/fs/smackfs
for mounting smackfs so that Smack can be conventional.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The components NETLABEL and SECURITY_NETWORK are required by
Smack. Using "depends" in Kconfig hides the Smack option
if the user hasn't figured out that they need to be enabled
while using make menuconfig. Using select is a better choice.
Because select is not recursive depends on NET and SECURITY
are added. The reflects similar usage in TOMOYO and AppArmor.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The data structure allocations being done in prepare_creds
are duplicated in smack_setprocattr. This results in the
structure allocated in prepare_creds being orphaned and
never freed. The duplicate code is removed from
smack_setprocattr.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Add /smack/revoke-subject special file. Writing a SMACK label to this file will
set the access to '-' for all access rules with that subject label.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
On 12/20/2011 11:20 PM, Jarkko Sakkinen wrote:
> Allow SIGCHLD to be passed to child process without
> explicit policy. This will help to keep the access
> control policy simple and easily maintainable with
> complex applications that require use of multiple
> security contexts. It will also help to keep them
> as isolated as possible.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>
I have a slightly different version that applies to the
current smack-next tree.
Allow SIGCHLD to be passed to child process without
explicit policy. This will help to keep the access
control policy simple and easily maintainable with
complex applications that require use of multiple
security contexts. It will also help to keep them
as isolated as possible.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
security/smack/smack_lsm.c | 37 ++++++++-----------------------------
1 files changed, 8 insertions(+), 29 deletions(-)
Consider the input case of a rule that consists entirely of non space
symbols followed by a \0. Say 64 + \0
In this case strlen(data) = 64
kzalloc of subject and object are 64 byte objects
sscanfdata, "%s %s %s", subject, ...)
will put 65 bytes into subject.
Signed-off-by: Alan Cox <alan@linux.intel.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: stable@vger.kernel.org
Signed-off-by: James Morris <james.l.morris@oracle.com>
This patch removes empty rules (i.e. with access set to '-') from the
rule list presented to user space.
Smack by design never removes labels nor rules from its lists. Access
for a rule may be set to '-' to effectively disable it. Such rules would
show up in the listing generated when /smack/load or /smack/load2 is
read. This may cause clutter if many rules were disabled.
As a rule with access set to '-' is equivalent to no rule at all, they
may be safely hidden from the listing.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Some of the bounds checking used on the /smack/access
interface was lost when support for long labels was
added. No kernel access checks are affected, however
this is a case where /smack/access could be used
incorrectly and fail to detect the error. This patch
reintroduces the original checks.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Smack is integrated with the POSIX capabilities scheme,
using the capabilities CAP_MAC_OVERRIDE and CAP_MAC_ADMIN to
determine if a process is allowed to ignore Smack checks or
change Smack related data respectively. Smack provides an
additional restriction that if an onlycap value is set
by writing to /smack/onlycap only tasks with that Smack
label are allowed to use CAP_MAC_OVERRIDE.
This change adds CAP_MAC_ADMIN as a capability that is affected
by the onlycap mechanism.
Targeted for git://git.gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
In January of 2012 Al Viro pointed out three bits of code that
he titled "new_inode_smack bogosities". This patch repairs these
errors.
1. smack_sb_kern_mount() included a NULL check that is impossible.
The check and NULL case are removed.
2. smack_kb_kern_mount() included pointless locking. The locking is
removed. Since this is the only place that lock was used the lock
is removed from the superblock_smack structure.
3. smk_fill_super() incorrectly and unnecessarily set the Smack label
for the smackfs root inode. The assignment has been removed.
Targeted for git://gitorious.org/smack-next/kernel.git
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
V4 updated to current linux-security#next
Targeted for git://gitorious.org/smack-next/kernel.git
Modern application runtime environments like to use
naming schemes that are structured and generated without
human intervention. Even though the Smack limit of 23
characters for a label name is perfectly rational for
human use there have been complaints that the limit is
a problem in environments where names are composed from
a set or sources, including vendor, author, distribution
channel and application name. Names like
softwarehouse-pgwodehouse-coolappstore-mellowmuskrats
are becoming harder to avoid. This patch introduces long
label support in Smack. Labels are now limited to 255
characters instead of the old 23.
The primary reason for limiting the labels to 23 characters
was so they could be directly contained in CIPSO category sets.
This is still done were possible, but for labels that are too
large a mapping is required. This is perfectly safe for communication
that stays "on the box" and doesn't require much coordination
between boxes beyond what would have been required to keep label
names consistent.
The bulk of this patch is in smackfs, adding and updating
administrative interfaces. Because existing APIs can't be
changed new ones that do much the same things as old ones
have been introduced.
The Smack specific CIPSO data representation has been removed
and replaced with the data format used by netlabel. The CIPSO
header is now computed when a label is imported rather than
on use. This results in improved IP performance. The smack
label is now allocated separately from the containing structure,
allowing for larger strings.
Four new /smack interfaces have been introduced as four
of the old interfaces strictly required labels be specified
in fixed length arrays.
The access interface is supplemented with the check interface:
access "Subject Object rwxat"
access2 "Subject Object rwaxt"
The load interface is supplemented with the rules interface:
load "Subject Object rwxat"
load2 "Subject Object rwaxt"
The load-self interface is supplemented with the self-rules interface:
load-self "Subject Object rwxat"
load-self2 "Subject Object rwaxt"
The cipso interface is supplemented with the wire interface:
cipso "Subject lvl cnt c1 c2 ..."
cipso2 "Subject lvl cnt c1 c2 ..."
The old interfaces are maintained for compatibility.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Dave Chinner wrote:
> Yes, because you have no idea what the calling context is except
> for the fact that is from somewhere inside filesystem code and the
> filesystem could be holding locks. Therefore, GFP_NOFS is really the
> only really safe way to allocate memory here.
I see. Thank you.
I'm not sure, but can call trace happen where somewhere inside network
filesystem or stackable filesystem code with locks held invokes operations that
involves GFP_KENREL memory allocation outside that filesystem?
----------
[PATCH] SMACK: Fix incorrect GFP_KERNEL usage.
new_inode_smack() which can be called from smack_inode_alloc_security() needs
to use GFP_NOFS like SELinux's inode_alloc_security() does, for
security_inode_alloc() is called from inode_init_always() and
inode_init_always() is called from xfs_inode_alloc() which is using GFP_NOFS.
smack_inode_init_security() needs to use GFP_NOFS like
selinux_inode_init_security() does, for initxattrs() callback function (e.g.
btrfs_initxattrs()) which is called from security_inode_init_security() is
using GFP_NOFS.
smack_audit_rule_match() needs to use GFP_ATOMIC, for
security_audit_rule_match() can be called from audit_filter_user_rules() and
audit_filter_user_rules() is called from audit_filter_user() with RCU read lock
held.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Casey Schaufler <cschaufler@cschaufler-intel.(none)>
The transmuting directory feature of Smack requires that
the transmuting attribute be explicitly set in all cases.
It seems the users of this facility would expect that the
transmuting attribute be inherited by subdirectories that
are created in a transmuting directory. This does not seem
to add any additional complexity to the understanding of
how the system works.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
A kernel with Smack enabled will fail if tmpfs has xattr support.
Move the initialization of predefined Smack label
list entries to the LSM initialization from the
smackfs setup. This became an issue when tmpfs
acquired xattr support, but was never correct.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
This fixes builds where CONFIG_AUDIT is not defined and
CONFIG_SECURITY_SMACK=y.
This got introduced by the stack-usage reducation commit 48c62af68a
("LSM: shrink the common_audit_data data union").
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
It isn't needed. If you don't set the type of the data associated with
that type it is a pretty obvious programming bug. So why waste the cycles?
Signed-off-by: Eric Paris <eparis@redhat.com>
It just bloats the audit data structure for no good reason, since the
only time those fields are filled are just before calling the
common_lsm_audit() function, which is also the only user of those
fields.
So just make them be the arguments to common_lsm_audit(), rather than
bloating that structure that is passed around everywhere, and is
initialized in hot paths.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
After shrinking the common_audit_data stack usage for private LSM data I'm
not going to shrink the data union. To do this I'm going to move anything
larger than 2 void * ptrs to it's own structure and require it to be declared
separately on the calling stack. Thus hot paths which don't need more than
a couple pointer don't have to declare space to hold large unneeded
structures. I could get this down to one void * by dealing with the key
struct and the struct path. We'll see if that is helpful after taking care of
networking.
Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Linus found that the gigantic size of the common audit data caused a big
perf hit on something as simple as running stat() in a loop. This patch
requires LSMs to declare the LSM specific portion separately rather than
doing it in a union. Thus each LSM can be responsible for shrinking their
portion and don't have to pay a penalty just because other LSMs have a
bigger space requirement.
Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Commit 272cd7a8c6 introduced
a change to the way rule lists are handled and reported in
the smackfs filesystem. One of the issues addressed had to
do with the termination of read requests on /smack/load.
This change introduced a error in /smack/cipso, which shares
some of the same list processing code.
This patch updates all the file access list handling in
smackfs to use the code introduced for /smack/load.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Allow query access as a normal user removing the need
for CAP_MAC_ADMIN. Give RW access to /smack/access
for UGO. Do not import smack labels in access check.
Signed-off-by: Jarkko Sakkinen <jarkko.j.sakkinen@gmail.com>
Signed-off-by: Casey Schaufler <cschaufler@cschaufler-intel.(none)>
Forgot to update simple_transaction_set() to take terminator
character into account.
Signed-off-by: Jarkko Sakkinen <jarkko.j.sakkinen@gmail.com>
Signed-off-by: Casey Schaufler <cschaufler@cschaufler-intel.(none)>
On some build configurations PER_CLEAR_ON_SETID symbol was not
found when compiling smack_lsm.c. This patch fixes the issue by
explicitly doing #include <linux/personality.h>.
Signed-off-by: Jarkko Sakkinen <jarkko.j.sakkinen@gmail.com>
Signed-off-by: Casey Schaufler <cschaufler@cschaufler-intel.(none)>
Small fix for the output of access SmackFS file. Use string
is instead of byte. Makes it easier to extend API if it is
needed.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>
This patch is targeted for the smack-next tree.
This patch takes advantage of the recent changes for performance
and points the packet labels on UDS connect at the output label of
the far side. This makes getsockopt(...SO_PEERCRED...) function
properly. Without this change the getsockopt does not provide any
information.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
There are a number of comments in the Smack code that
are either malformed or include code. This patch cleans
them up.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Al Viro pointed out that the processing of fcntl done
by Smack appeared poorly designed. He was right. There
are three things that required change. Most obviously,
the list of commands that really imply writing is limited
to those involving file locking and signal handling.
The initialization if the file security blob was
incomplete, requiring use of a heretofore unused LSM hook.
Finally, the audit information coming from a helper
masked the identity of the LSM hook. This patch corrects
all three of these defects.
This is targeted for the smack-next tree pending comments.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
This patch is targeted for the smack-next tree.
Smack access checks suffer from two significant performance
issues. In cases where there are large numbers of rules the
search of the single list of rules is wasteful. Comparing the
string values of the smack labels is less efficient than a
numeric comparison would.
These changes take advantage of the Smack label list, which
maintains the mapping of Smack labels to secids and optional
CIPSO labels. Because the labels are kept perpetually, an
access check can be done strictly based on the address of the
label in the list without ever looking at the label itself.
Rather than keeping one global list of rules the rules with
a particular subject label can be based off of that label
list entry. The access check need never look at entries that
do not use the current subject label.
This requires that packets coming off the network with
CIPSO direct Smack labels that have never been seen before
be treated carefully. The only case where they could be
delivered is where the receiving socket has an IPIN star
label, so that case is explicitly addressed.
On a system with 39,800 rules (200 labels in all permutations)
a system with this patch runs an access speed test in 5% of
the time of the old version. That should be a best case
improvement. If all of the rules are associated with the
same subject label and all of the accesses are for processes
with that label (unlikely) the improvement is about 30%.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Adds a new file into SmackFS called 'access'. Wanted
Smack permission is written into /smack/access.
After that result can be read from the opened file.
If access applies result contains 1 and otherwise
0. File access is protected from race conditions
by using simple_transaction_get()/set() API.
Fixes from the previous version:
- Removed smack.h changes, refactoring left-over
from previous version.
- Removed #include <linux/smack.h>, refactoring
left-over from previous version.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>
Signed-off-by: Casey Schaufler <cschaufler@cschaufler-intel.(none)>
My @hp.com will no longer be valid starting August 5, 2011 so an update is
necessary. My new email address is employer independent so we don't have
to worry about doing this again any time soon.
Signed-off-by: Paul Moore <paul.moore@hp.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
smack_file_lock has a struct path, so use that instead of only the
dentry.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
This patch separates and audit message that only contains a dentry from
one that contains a full path. This allows us to make it harder to
misuse the interfaces or for the interfaces to be implemented wrong.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
The lsm common audit code has wacky contortions making sure which pieces
of information are set based on if it was given a path, dentry, or
inode. Split this into path and inode to get rid of some of the code
complexity.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Right now all RCU walks fall back to reference walk when CONFIG_SECURITY
is enabled, even though just the standard capability module is active.
This is because security_inode_exec_permission unconditionally fails
RCU walks.
Move this decision to the low level security module. This requires
passing the RCU flags down the security hook. This way at least
the capability module and a few easy cases in selinux/smack work
with RCU walks with CONFIG_SECURITY=y
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
Right now all RCU walks fall back to reference walk when CONFIG_SECURITY
is enabled, even though just the standard capability module is active.
This is because security_inode_exec_permission unconditionally fails
RCU walks.
Move this decision to the low level security module. This requires
passing the RCU flags down the security hook. This way at least
the capability module and a few easy cases in selinux/smack work
with RCU walks with CONFIG_SECURITY=y
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The mmap policy enforcement checks the access of the
SMACK64MMAP subject against the current subject incorrectly.
The check as written works correctly only if the access
rules involved have the same access. This is the common
case, so initial testing did not find a problem.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
The mmap policy enforcement was not properly handling the
interaction between the global and local rule lists.
Instead of going through one and then the other, which
missed the important case where a rule specified that
there should be no access, combine the access limitations
where there is a rule in each list.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
SELinux would like to implement a new labeling behavior of newly created
inodes. We currently label new inodes based on the parent and the creating
process. This new behavior would also take into account the name of the
new object when deciding the new label. This is not the (supposed) full path,
just the last component of the path.
This is very useful because creating /etc/shadow is different than creating
/etc/passwd but the kernel hooks are unable to differentiate these
operations. We currently require that userspace realize it is doing some
difficult operation like that and than userspace jumps through SELinux hoops
to get things set up correctly. This patch does not implement new
behavior, that is obviously contained in a seperate SELinux patch, but it
does pass the needed name down to the correct LSM hook. If no such name
exists it is fine to pass NULL.
Signed-off-by: Eric Paris <eparis@redhat.com>
In the embedded world there are often situations
where libraries are updated from a variety of sources,
for a variety of reasons, and with any number of
security characteristics. These differences
might include privilege required for a given library
provided interface to function properly, as occurs
from time to time in graphics libraries. There are
also cases where it is important to limit use of
libraries based on the provider of the library and
the security aware application may make choices
based on that criteria.
These issues are addressed by providing an additional
Smack label that may optionally be assigned to an object,
the SMACK64MMAP attribute. An mmap operation is allowed
if there is no such attribute.
If there is a SMACK64MMAP attribute the mmap is permitted
only if a subject with that label has all of the access
permitted a subject with the current task label.
Security aware applications may from time to time
wish to reduce their "privilege" to avoid accidental use
of privilege. One case where this arises is the
environment in which multiple sources provide libraries
to perform the same functions. An application may know
that it should eschew services made available from a
particular vendor, or of a particular version.
In support of this a secondary list of Smack rules has
been added that is local to the task. This list is
consulted only in the case where the global list has
approved access. It can only further restrict access.
Unlike the global last, if no entry is found on the
local list access is granted. An application can add
entries to its own list by writing to /smack/load-self.
The changes appear large as they involve refactoring
the list handling to accomodate there being more
than one rule list.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Conflicts:
security/smack/smack_lsm.c
Verified and added fix by Stephen Rothwell <sfr@canb.auug.org.au>
Ok'd by Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
unix_release() can asynchornously set socket->sk to NULL, and
it does so without holding the unix_state_lock() on "other"
during stream connects.
However, the reverse mapping, sk->sk_socket, is only transitioned
to NULL under the unix_state_lock().
Therefore make the security hooks follow the reverse mapping instead
of the forward mapping.
Reported-by: Jeremy Fitzhardinge <jeremy@goop.org>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
In a situation where Smack access rules allow processes
with multiple labels to write to a directory it is easy
to get into a situation where the directory gets cluttered
with files that the owner can't deal with because while
they could be written to the directory a process at the
label of the directory can't write them. This is generally
the desired behavior, but when it isn't it is a real
issue.
This patch introduces a new attribute SMACK64TRANSMUTE that
instructs Smack to create the file with the label of the directory
under certain circumstances.
A new access mode, "t" for transmute, is made available to
Smack access rules, which are expanded from "rwxa" to "rwxat".
If a file is created in a directory marked as transmutable
and if access was granted to perform the operation by a rule
that included the transmute mode, then the file gets the
Smack label of the directory instead of the Smack label of the
creating process.
Note that this is equivalent to creating an empty file at the
label of the directory and then having the other process write
to it. The transmute scheme requires that both the access rule
allows transmutation and that the directory be explicitly marked.
Signed-off-by: Jarkko Sakkinen <ext-jarkko.2.sakkinen@nokia.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
SMACK64EXEC. It defines label that is used while task is
running.
Exception: in smack_task_wait() child task is checked
for write access to parent task using label inherited
from the task that forked it.
Fixed issues from previous submit:
- SMACK64EXEC was not read when SMACK64 was not set.
- inode security blob was not updated after setting
SMACK64EXEC
- inode security blob was not updated when removing
SMACK64EXEC
This patch addresses a number of long standing issues
with the way Smack treats UNIX domain sockets.
All access control was being done based on the label of
the file system object. This is inconsistant with the
internet domain, in which access is done based on the
IPIN and IPOUT attributes of the socket. As a result
of the inode label policy it was not possible to use
a UDS socket for label cognizant services, including
dbus and the X11 server.
Support for SCM_PEERSEC on UDS sockets is also provided.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
The addition of CONFIG_SECURITY_DMESG_RESTRICT resulted in a build
failure when CONFIG_PRINTK=n. This is because the capabilities code
which used the new option was built even though the variable in question
didn't exist.
The patch here fixes this by moving the capabilities checks out of the
LSM and into the caller. All (known) LSMs should have been calling the
capabilities hook already so it actually makes the code organization
better to eliminate the hook altogether.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: James Morris <jmorris@namei.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* 'llseek' of git://git.kernel.org/pub/scm/linux/kernel/git/arnd/bkl:
vfs: make no_llseek the default
vfs: don't use BKL in default_llseek
llseek: automatically add .llseek fop
libfs: use generic_file_llseek for simple_attr
mac80211: disallow seeks in minstrel debug code
lirc: make chardev nonseekable
viotape: use noop_llseek
raw: use explicit llseek file operations
ibmasmfs: use generic_file_llseek
spufs: use llseek in all file operations
arm/omap: use generic_file_llseek in iommu_debug
lkdtm: use generic_file_llseek in debugfs
net/wireless: use generic_file_llseek in debugfs
drm: use noop_llseek
With the (long ago) interface change to have the secid_to_secctx functions
do the string allocation instead of having the caller do the allocation we
lost the ability to query the security server for the length of the
upcoming string. The SECMARK code would like to allocate a netlink skb
with enough length to hold the string but it is just too unclean to do the
string allocation twice or to do the allocation the first time and hold
onto the string and slen. This patch adds the ability to call
security_secid_to_secctx() with a NULL data pointer and it will just set
the slen pointer.
Signed-off-by: Eric Paris <eparis@redhat.com>
Reviewed-by: Paul Moore <paul.moore@hp.com>
Signed-off-by: James Morris <jmorris@namei.org>
All security modules shouldn't change sched_param parameter of
security_task_setscheduler(). This is not only meaningless, but also
make a harmful result if caller pass a static variable.
This patch remove policy and sched_param parameter from
security_task_setscheduler() becuase none of security module is
using it.
Cc: James Morris <jmorris@namei.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: James Morris <jmorris@namei.org>
All file_operations should get a .llseek operation so we can make
nonseekable_open the default for future file operations without a
.llseek pointer.
The three cases that we can automatically detect are no_llseek, seq_lseek
and default_llseek. For cases where we can we can automatically prove that
the file offset is always ignored, we use noop_llseek, which maintains
the current behavior of not returning an error from a seek.
New drivers should normally not use noop_llseek but instead use no_llseek
and call nonseekable_open at open time. Existing drivers can be converted
to do the same when the maintainer knows for certain that no user code
relies on calling seek on the device file.
The generated code is often incorrectly indented and right now contains
comments that clarify for each added line why a specific variant was
chosen. In the version that gets submitted upstream, the comments will
be gone and I will manually fix the indentation, because there does not
seem to be a way to do that using coccinelle.
Some amount of new code is currently sitting in linux-next that should get
the same modifications, which I will do at the end of the merge window.
Many thanks to Julia Lawall for helping me learn to write a semantic
patch that does all this.
===== begin semantic patch =====
// This adds an llseek= method to all file operations,
// as a preparation for making no_llseek the default.
//
// The rules are
// - use no_llseek explicitly if we do nonseekable_open
// - use seq_lseek for sequential files
// - use default_llseek if we know we access f_pos
// - use noop_llseek if we know we don't access f_pos,
// but we still want to allow users to call lseek
//
@ open1 exists @
identifier nested_open;
@@
nested_open(...)
{
<+...
nonseekable_open(...)
...+>
}
@ open exists@
identifier open_f;
identifier i, f;
identifier open1.nested_open;
@@
int open_f(struct inode *i, struct file *f)
{
<+...
(
nonseekable_open(...)
|
nested_open(...)
)
...+>
}
@ read disable optional_qualifier exists @
identifier read_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
expression E;
identifier func;
@@
ssize_t read_f(struct file *f, char *p, size_t s, loff_t *off)
{
<+...
(
*off = E
|
*off += E
|
func(..., off, ...)
|
E = *off
)
...+>
}
@ read_no_fpos disable optional_qualifier exists @
identifier read_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
@@
ssize_t read_f(struct file *f, char *p, size_t s, loff_t *off)
{
... when != off
}
@ write @
identifier write_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
expression E;
identifier func;
@@
ssize_t write_f(struct file *f, const char *p, size_t s, loff_t *off)
{
<+...
(
*off = E
|
*off += E
|
func(..., off, ...)
|
E = *off
)
...+>
}
@ write_no_fpos @
identifier write_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
@@
ssize_t write_f(struct file *f, const char *p, size_t s, loff_t *off)
{
... when != off
}
@ fops0 @
identifier fops;
@@
struct file_operations fops = {
...
};
@ has_llseek depends on fops0 @
identifier fops0.fops;
identifier llseek_f;
@@
struct file_operations fops = {
...
.llseek = llseek_f,
...
};
@ has_read depends on fops0 @
identifier fops0.fops;
identifier read_f;
@@
struct file_operations fops = {
...
.read = read_f,
...
};
@ has_write depends on fops0 @
identifier fops0.fops;
identifier write_f;
@@
struct file_operations fops = {
...
.write = write_f,
...
};
@ has_open depends on fops0 @
identifier fops0.fops;
identifier open_f;
@@
struct file_operations fops = {
...
.open = open_f,
...
};
// use no_llseek if we call nonseekable_open
////////////////////////////////////////////
@ nonseekable1 depends on !has_llseek && has_open @
identifier fops0.fops;
identifier nso ~= "nonseekable_open";
@@
struct file_operations fops = {
... .open = nso, ...
+.llseek = no_llseek, /* nonseekable */
};
@ nonseekable2 depends on !has_llseek @
identifier fops0.fops;
identifier open.open_f;
@@
struct file_operations fops = {
... .open = open_f, ...
+.llseek = no_llseek, /* open uses nonseekable */
};
// use seq_lseek for sequential files
/////////////////////////////////////
@ seq depends on !has_llseek @
identifier fops0.fops;
identifier sr ~= "seq_read";
@@
struct file_operations fops = {
... .read = sr, ...
+.llseek = seq_lseek, /* we have seq_read */
};
// use default_llseek if there is a readdir
///////////////////////////////////////////
@ fops1 depends on !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier readdir_e;
@@
// any other fop is used that changes pos
struct file_operations fops = {
... .readdir = readdir_e, ...
+.llseek = default_llseek, /* readdir is present */
};
// use default_llseek if at least one of read/write touches f_pos
/////////////////////////////////////////////////////////////////
@ fops2 depends on !fops1 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read.read_f;
@@
// read fops use offset
struct file_operations fops = {
... .read = read_f, ...
+.llseek = default_llseek, /* read accesses f_pos */
};
@ fops3 depends on !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier write.write_f;
@@
// write fops use offset
struct file_operations fops = {
... .write = write_f, ...
+ .llseek = default_llseek, /* write accesses f_pos */
};
// Use noop_llseek if neither read nor write accesses f_pos
///////////////////////////////////////////////////////////
@ fops4 depends on !fops1 && !fops2 && !fops3 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read_no_fpos.read_f;
identifier write_no_fpos.write_f;
@@
// write fops use offset
struct file_operations fops = {
...
.write = write_f,
.read = read_f,
...
+.llseek = noop_llseek, /* read and write both use no f_pos */
};
@ depends on has_write && !has_read && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier write_no_fpos.write_f;
@@
struct file_operations fops = {
... .write = write_f, ...
+.llseek = noop_llseek, /* write uses no f_pos */
};
@ depends on has_read && !has_write && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read_no_fpos.read_f;
@@
struct file_operations fops = {
... .read = read_f, ...
+.llseek = noop_llseek, /* read uses no f_pos */
};
@ depends on !has_read && !has_write && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
@@
struct file_operations fops = {
...
+.llseek = noop_llseek, /* no read or write fn */
};
===== End semantic patch =====
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Julia Lawall <julia@diku.dk>
Cc: Christoph Hellwig <hch@infradead.org>
SELinux needs to pass the MAY_ACCESS flag so it can handle auditting
correctly. Presently the masking of MAY_* flags is done in the VFS. In
order to allow LSMs to decide what flags they care about and what flags
they don't just pass them all and the each LSM mask off what they don't
need. This patch should contain no functional changes to either the VFS or
any LSM.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Stephen D. Smalley <sds@tycho.nsa.gov>
Signed-off-by: James Morris <jmorris@namei.org>
This patch removes some unneeded code for if opt_dentry is null because
that can never happen.
The function dereferences "opt_dentry" earlier when it checks
"if (opt_dentry->d_parent == opt_dentry) {". That code was added in
2008.
This function called from security_d_instantiate(). I checked all the
places which call security_d_instantiate() and dentry is always non-null.
I also checked the selinux version of this hook and there is a comment
which says that dentry should be non-null if called from
d_instantiate().
Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
This is an unused hook in SMACK so remove it.
Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
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 allmodconfig
8. 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>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (52 commits)
init: Open /dev/console from rootfs
mqueue: fix typo "failues" -> "failures"
mqueue: only set error codes if they are really necessary
mqueue: simplify do_open() error handling
mqueue: apply mathematics distributivity on mq_bytes calculation
mqueue: remove unneeded info->messages initialization
mqueue: fix mq_open() file descriptor leak on user-space processes
fix race in d_splice_alias()
set S_DEAD on unlink() and non-directory rename() victims
vfs: add NOFOLLOW flag to umount(2)
get rid of ->mnt_parent in tomoyo/realpath
hppfs can use existing proc_mnt, no need for do_kern_mount() in there
Mirror MS_KERNMOUNT in ->mnt_flags
get rid of useless vfsmount_lock use in put_mnt_ns()
Take vfsmount_lock to fs/internal.h
get rid of insanity with namespace roots in tomoyo
take check for new events in namespace (guts of mounts_poll()) to namespace.c
Don't mess with generic_permission() under ->d_lock in hpfs
sanitize const/signedness for udf
nilfs: sanitize const/signedness in dealing with ->d_name.name
...
Fix up fairly trivial (famous last words...) conflicts in
drivers/infiniband/core/uverbs_main.c and security/tomoyo/realpath.c
(mnt,mnt_mountpoint) pair is conceptually wrong; if you want
to use it for generating pathname and for nothing else *and*
if you know that vfsmount tree is unchanging, you can get
away with that, but the right solution for that is (mnt,mnt_root).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Casey Schaufler
Andrey Ryabinin
Rafal Krypa
Zbigniew Jasinski
Łukasz Stelmach
Lukasz Pawelczyk
Linus Torvalds
Al Viro
Rohit
Jeff Layton
Marcin Niesluchowski
Konstantin Khlebnikov
James Morris
Paul Moore
Toralf Förster
José Bollo
Pankaj Kumar
Lukasz Pawelczyk
Dmitry Kasatkin
David Howells
Richard Guy Briggs
Jarkko Sakkinen
Tomasz Stanislawski
Tetsuo Handa
David Quigley
Passion,Zhao
Igor Zhbanov
Jarkko Sakkinen
Alan Cox
Kees Cook
Eric Paris
Al Viro
Jarkko Sakkinen
Jarkko Sakkinen
Paul Moore
James Morris
Andi Kleen
Lucas De Marchi
Shan Wei
David S. Miller
Jarkko Sakkinen
KOSAKI Motohiro
Arnd Bergmann
Mimi Zohar
Dan Carpenter
Uwe Kleine-König
Tejun Heo