aboutsummaryrefslogtreecommitdiffstats
path: root/security/apparmor/lib.c (follow)
AgeCommit message (Collapse)AuthorFilesLines
2018-10-03apparmor: Fix uninitialized value in aa_split_fqnameZubin Mithra1-2/+4
Syzkaller reported a OOB-read with the stacktrace below. This occurs inside __aa_lookupn_ns as `n` is not initialized. `n` is obtained from aa_splitn_fqname. In cases where `name` is invalid, aa_splitn_fqname returns without initializing `ns_name` and `ns_len`. Fix this by always initializing `ns_name` and `ns_len`. __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430 memcmp+0xe3/0x160 lib/string.c:861 strnstr+0x4b/0x70 lib/string.c:934 __aa_lookupn_ns+0xc1/0x570 security/apparmor/policy_ns.c:209 aa_lookupn_ns+0x88/0x1e0 security/apparmor/policy_ns.c:240 aa_fqlookupn_profile+0x1b9/0x1010 security/apparmor/policy.c:468 fqlookupn_profile+0x80/0xc0 security/apparmor/label.c:1844 aa_label_strn_parse+0xa3a/0x1230 security/apparmor/label.c:1908 aa_label_parse+0x42/0x50 security/apparmor/label.c:1943 aa_change_profile+0x513/0x3510 security/apparmor/domain.c:1362 apparmor_setprocattr+0xaa4/0x1150 security/apparmor/lsm.c:658 security_setprocattr+0x66/0xc0 security/security.c:1298 proc_pid_attr_write+0x301/0x540 fs/proc/base.c:2555 __vfs_write+0x119/0x9f0 fs/read_write.c:485 vfs_write+0x1fc/0x560 fs/read_write.c:549 ksys_write+0x101/0x260 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Fixes: 3b0aaf5866bf ("apparmor: add lib fn to find the "split" for fqnames") Reported-by: syzbot+61e4b490d9d2da591b50@syzkaller.appspotmail.com Signed-off-by: Zubin Mithra <zsm@chromium.org> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-07-19apparmor: Check buffer bounds when mapping permissions maskTyler Hicks1-4/+13
Don't read past the end of the buffer containing permissions characters or write past the end of the destination string. Detected by CoverityScan CID#1415361, 1415376 ("Out-of-bounds access") Fixes: e53cfe6c7caa ("apparmor: rework perm mapping to a slightly broader set") Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-05-03apparmor: fix typo "type"Zygmunt Krynicki1-1/+1
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com> Acked-by: Christian Boltz <apparmor@cboltz.de> Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-03-13apparmor: add base infastructure for socket mediationJohn Johansen1-2/+3
version 2 - Force an abi break. Network mediation will only be available in v8 abi complaint policy. Provide a basic mediation of sockets. This is not a full net mediation but just whether a spcific family of socket can be used by an application, along with setting up some basic infrastructure for network mediation to follow. the user space rule hav the basic form of NETWORK RULE = [ QUALIFIERS ] 'network' [ DOMAIN ] [ TYPE | PROTOCOL ] DOMAIN = ( 'inet' | 'ax25' | 'ipx' | 'appletalk' | 'netrom' | 'bridge' | 'atmpvc' | 'x25' | 'inet6' | 'rose' | 'netbeui' | 'security' | 'key' | 'packet' | 'ash' | 'econet' | 'atmsvc' | 'sna' | 'irda' | 'pppox' | 'wanpipe' | 'bluetooth' | 'netlink' | 'unix' | 'rds' | 'llc' | 'can' | 'tipc' | 'iucv' | 'rxrpc' | 'isdn' | 'phonet' | 'ieee802154' | 'caif' | 'alg' | 'nfc' | 'vsock' | 'mpls' | 'ib' | 'kcm' ) ',' TYPE = ( 'stream' | 'dgram' | 'seqpacket' | 'rdm' | 'raw' | 'packet' ) PROTOCOL = ( 'tcp' | 'udp' | 'icmp' ) eg. network, network inet, Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Seth Arnold <seth.arnold@canonical.com>
2017-11-21apparmor: remove unused redundant variable stopColin Ian King1-3/+0
The boolean variable 'stop' is being set but never read. This is a redundant variable and can be removed. Cleans up clang warning: Value stored to 'stop' is never read Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-11-21apparmor: initialized returned struct aa_permsArnd Bergmann1-8/+5
gcc-4.4 points out suspicious code in compute_mnt_perms, where the aa_perms structure is only partially initialized before getting returned: security/apparmor/mount.c: In function 'compute_mnt_perms': security/apparmor/mount.c:227: error: 'perms.prompt' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.hide' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.cond' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.complain' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.stop' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.deny' is used uninitialized in this function Returning or assigning partially initialized structures is a bit tricky, in particular it is explicitly allowed in c99 to assign a partially initialized structure to another, as long as only members are read that have been initialized earlier. Looking at what various compilers do here, the version that produced the warning copied uninitialized stack data, while newer versions (and also clang) either set the other members to zero or don't update the parts of the return buffer that are not modified in the temporary structure, but they never warn about this. In case of apparmor, it seems better to be a little safer and always initialize the aa_perms structure. Most users already do that, this changes the remaining ones, including the one instance that I got the warning for. Fixes: fa488437d0f9 ("apparmor: add mount mediation") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Seth Arnold <seth.arnold@canonical.com> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-10-26Revert "apparmor: add base infastructure for socket mediation"Linus Torvalds1-3/+2
This reverts commit 651e28c5537abb39076d3949fb7618536f1d242e. This caused a regression: "The specific problem is that dnsmasq refuses to start on openSUSE Leap 42.2. The specific cause is that and attempt to open a PF_LOCAL socket gets EACCES. This means that networking doesn't function on a system with a 4.14-rc2 system." Sadly, the developers involved seemed to be in denial for several weeks about this, delaying the revert. This has not been a good release for the security subsystem, and this area needs to change development practices. Reported-and-bisected-by: James Bottomley <James.Bottomley@hansenpartnership.com> Tracked-by: Thorsten Leemhuis <regressions@leemhuis.info> Cc: John Johansen <john.johansen@canonical.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Seth Arnold <seth.arnold@canonical.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2017-09-22apparmor: add base infastructure for socket mediationJohn Johansen1-2/+3
Provide a basic mediation of sockets. This is not a full net mediation but just whether a spcific family of socket can be used by an application, along with setting up some basic infrastructure for network mediation to follow. the user space rule hav the basic form of NETWORK RULE = [ QUALIFIERS ] 'network' [ DOMAIN ] [ TYPE | PROTOCOL ] DOMAIN = ( 'inet' | 'ax25' | 'ipx' | 'appletalk' | 'netrom' | 'bridge' | 'atmpvc' | 'x25' | 'inet6' | 'rose' | 'netbeui' | 'security' | 'key' | 'packet' | 'ash' | 'econet' | 'atmsvc' | 'sna' | 'irda' | 'pppox' | 'wanpipe' | 'bluetooth' | 'netlink' | 'unix' | 'rds' | 'llc' | 'can' | 'tipc' | 'iucv' | 'rxrpc' | 'isdn' | 'phonet' | 'ieee802154' | 'caif' | 'alg' | 'nfc' | 'vsock' | 'mpls' | 'ib' | 'kcm' ) ',' TYPE = ( 'stream' | 'dgram' | 'seqpacket' | 'rdm' | 'raw' | 'packet' ) PROTOCOL = ( 'tcp' | 'udp' | 'icmp' ) eg. network, network inet, Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Seth Arnold <seth.arnold@canonical.com>
2017-06-10apparmor: switch from profiles to using labels on contextsJohn Johansen1-0/+163
Begin the actual switch to using domain labels by storing them on the context and converting the label to a singular profile where possible. Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-10apparmor: share profile name on replacementJohn Johansen1-8/+32
The profile names are the same, leverage this. Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-10apparmor: switch from file_perms to aa_permsJohn Johansen1-0/+1
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-10apparmor: add gerneric permissions struct and support fnsJohn Johansen1-0/+102
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-09apparmor: rework perm mapping to a slightly broader setJohn Johansen1-0/+59
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-08apparmor: move permissions into their own file to be more easily sharedJohn Johansen1-0/+1
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-06-08apparmor: make internal lib fn skipn_spaces available to the rest of apparmorJohn Johansen1-1/+1
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-05-08mm: introduce kv[mz]alloc helpersMichal Hocko1-30/+0
Patch series "kvmalloc", v5. There are many open coded kmalloc with vmalloc fallback instances in the tree. Most of them are not careful enough or simply do not care about the underlying semantic of the kmalloc/page allocator which means that a) some vmalloc fallbacks are basically unreachable because the kmalloc part will keep retrying until it succeeds b) the page allocator can invoke a really disruptive steps like the OOM killer to move forward which doesn't sound appropriate when we consider that the vmalloc fallback is available. As it can be seen implementing kvmalloc requires quite an intimate knowledge if the page allocator and the memory reclaim internals which strongly suggests that a helper should be implemented in the memory subsystem proper. Most callers, I could find, have been converted to use the helper instead. This is patch 6. There are some more relying on __GFP_REPEAT in the networking stack which I have converted as well and Eric Dumazet was not opposed [2] to convert them as well. [1] http://lkml.kernel.org/r/20170130094940.13546-1-mhocko@kernel.org [2] http://lkml.kernel.org/r/1485273626.16328.301.camel@edumazet-glaptop3.roam.corp.google.com This patch (of 9): Using kmalloc with the vmalloc fallback for larger allocations is a common pattern in the kernel code. Yet we do not have any common helper for that and so users have invented their own helpers. Some of them are really creative when doing so. Let's just add kv[mz]alloc and make sure it is implemented properly. This implementation makes sure to not make a large memory pressure for > PAGE_SZE requests (__GFP_NORETRY) and also to not warn about allocation failures. This also rules out the OOM killer as the vmalloc is a more approapriate fallback than a disruptive user visible action. This patch also changes some existing users and removes helpers which are specific for them. In some cases this is not possible (e.g. ext4_kvmalloc, libcfs_kvzalloc) because those seems to be broken and require GFP_NO{FS,IO} context which is not vmalloc compatible in general (note that the page table allocation is GFP_KERNEL). Those need to be fixed separately. While we are at it, document that __vmalloc{_node} about unsupported gfp mask because there seems to be a lot of confusion out there. kvmalloc_node will warn about GFP_KERNEL incompatible (which are not superset) flags to catch new abusers. Existing ones would have to die slowly. [sfr@canb.auug.org.au: f2fs fixup] Link: http://lkml.kernel.org/r/20170320163735.332e64b7@canb.auug.org.au Link: http://lkml.kernel.org/r/20170306103032.2540-2-mhocko@kernel.org Signed-off-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> Reviewed-by: Andreas Dilger <adilger@dilger.ca> [ext4 part] Acked-by: Vlastimil Babka <vbabka@suse.cz> Cc: John Hubbard <jhubbard@nvidia.com> Cc: David Miller <davem@davemloft.net> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2017-04-07apparmor: fix boolreturn.cocci warningskbuild test robot1-2/+2
security/apparmor/lib.c:132:9-10: WARNING: return of 0/1 in function 'aa_policy_init' with return type bool Return statements in functions returning bool should use true/false instead of 1/0. Generated by: scripts/coccinelle/misc/boolreturn.cocci Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com>
2017-01-16apparmor: change aad apparmor_audit_data macro to a fn macroJohn Johansen1-5/+3
The aad macro can replace aad strings when it is not intended to. Switch to a fn macro so it is only applied when intended. Also at the same time cleanup audit_data initialization by putting common boiler plate behind a macro, and dropping the gfp_t parameter which will become useless. Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: update policy_destroy to use new debug assertsJohn Johansen1-9/+2
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: pass gfp param into aa_policy_init()John Johansen1-4/+4
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: constify policy name and hnameJohn Johansen1-1/+1
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: rename hname_tail to basenameJohn Johansen1-1/+1
Rename to the shorter and more familiar shell cmd name Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: add lib fn to find the "split" for fqnamesJohn Johansen1-0/+53
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: split out shared policy_XXX fns to libJohn Johansen1-0/+52
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-16apparmor: move lib definitions into separate lib includeJohn Johansen1-1/+1
Signed-off-by: John Johansen <john.johansen@canonical.com>
2017-01-15AppArmor: Use GFP_KERNEL for __aa_kvmalloc().Tetsuo Handa1-1/+2
Calling kmalloc(GFP_NOIO) with order == PAGE_ALLOC_COSTLY_ORDER is not recommended because it might fall into infinite retry loop without invoking the OOM killer. Since aa_dfa_unpack() is the only caller of kvzalloc() and aa_dfa_unpack() which is calling kvzalloc() via unpack_table() is doing kzalloc(GFP_KERNEL), it is safe to use GFP_KERNEL from __aa_kvmalloc(). Since aa_simple_write_to_buffer() is the only caller of kvmalloc() and aa_simple_write_to_buffer() is calling copy_from_user() which is GFP_KERNEL context (see memdup_user_nul()), it is safe to use GFP_KERNEL from __aa_kvmalloc(). Therefore, replace GFP_NOIO with GFP_KERNEL. Also, since we have vmalloc() fallback, add __GFP_NORETRY so that we don't invoke the OOM killer by kmalloc(GFP_KERNEL) with order == PAGE_ALLOC_COSTLY_ORDER. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: John Johansen <john.johansen@canonical.com>
2014-05-06nick kvfree() from apparmorAl Viro1-14/+0
too many places open-code it Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2013-08-14apparmor: remove minimum size check for vmalloc()Tetsuo Handa1-5/+0
This is a follow-up to commit b5b3ee6c "apparmor: no need to delay vfree()". Since vmalloc() will do "size = PAGE_ALIGN(size);", we don't need to check for "size >= sizeof(struct work_struct)". Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: John Johansen <john.johansen@canonical.com>
2013-05-12apparmor: no need to delay vfree()Al Viro1-21/+3
vfree() can be called from interrupt contexts now Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Acked-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com>
2013-04-28apparmor: fix fully qualified name parsingJohn Johansen1-2/+4
currently apparmor name parsing is only correctly handling :<NS>:<profile> but :<NS>://<profile> is also a valid form and what is exported to userspace. Signed-off-by: John Johansen <john.johansen@canonical.com>
2013-04-28apparmor: add kvzalloc to handle zeroing for kvmallocJohn Johansen1-5/+9
Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Steve Beattie <sbeattie@ubuntu.com>
2012-04-09LSM: do not initialize common_audit_data to 0Eric Paris1-1/+1
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>
2012-04-09LSM: remove the COMMON_AUDIT_DATA_INIT type expansionEric Paris1-1/+1
Just open code it so grep on the source code works better. Signed-off-by: Eric Paris <eparis@redhat.com>
2012-04-03LSM: shrink sizeof LSM specific portion of common_audit_dataEric Paris1-1/+3
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>
2011-09-09apparmor: sparse fix: add apparmor.h to lib.cJames Morris1-0/+1
Fix the following sparse warnings: security/apparmor/lib.c:37:6: warning: symbol 'aa_split_fqname' was not declared. Should it be static? security/apparmor/lib.c:63:6: warning: symbol 'aa_info_message' was not declared. Should it be static? security/apparmor/lib.c:83:6: warning: symbol 'kvmalloc' was not declared. Should it be static? security/apparmor/lib.c:123:6: warning: symbol 'kvfree' was not declared. Should it be static? Signed-off-by: James Morris <jmorris@namei.org>
2011-06-21net: remove mm.h inclusion from netdevice.hAlexey Dobriyan1-0/+1
Remove linux/mm.h inclusion from netdevice.h -- it's unused (I've checked manually). To prevent mm.h inclusion via other channels also extract "enum dma_data_direction" definition into separate header. This tiny piece is what gluing netdevice.h with mm.h via "netdevice.h => dmaengine.h => dma-mapping.h => scatterlist.h => mm.h". Removal of mm.h from scatterlist.h was tried and was found not feasible on most archs, so the link was cutoff earlier. Hope people are OK with tiny include file. Note, that mm_types.h is still dragged in, but it is a separate story. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2010-09-08AppArmor: Fix splitting an fqname into separate namespace and profile namesJohn Johansen1-1/+1
As per Dan Carpenter <error27@gmail.com> If we have a ns name without a following profile then in the original code it did "*ns_name = &name[1];". "name" is NULL so "*ns_name" is 0x1. That isn't useful and could cause an oops when this function is called from aa_remove_profiles(). Beyond this the assignment of the namespace name was wrong in the case where the profile name was provided as it was being set to &name[1] after name = skip_spaces(split + 1); Move the ns_name assignment before updating name for the split and also add skip_spaces, making the interface more robust. Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-08-02AppArmor: misc. base functions and definesJohn Johansen1-0/+133
Miscellaneous functions and defines needed by AppArmor, including the base path resolution routines. Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <jmorris@namei.org>