| Age | Commit message (Collapse) | Author |
|---|
|
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull AppArmor updates from John Johansen:
"This adds a single feature, switch the hash used to check policy from
sha1 to sha256
There are fixes for two memory leaks, and refcount bug and a potential
crash when a profile name is empty. Along with a couple minor code
cleanups.
Summary:
Features
- switch policy hash from sha1 to sha256
Bug Fixes
- Fix refcount leak in task_kill
- Fix leak of pdb objects and trans_table
- avoid crash when parse profie name is empty
Cleanups
- add static to stack_msg and nulldfa
- more kernel-doc cleanups"
* tag 'apparmor-pr-2024-01-18' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
apparmor: Fix memory leak in unpack_profile()
apparmor: avoid crash when parsed profile name is empty
apparmor: fix possible memory leak in unpack_trans_table
apparmor: free the allocated pdb objects
apparmor: Fix ref count leak in task_kill
apparmor: cleanup network hook comments
apparmor: add missing params to aa_may_ptrace kernel-doc comments
apparmor: declare nulldfa as static
apparmor: declare stack_msg as static
apparmor: switch SECURITY_APPARMOR_HASH from sha1 to sha256
|
|
Pull misc filesystem updates from Al Viro:
"Misc cleanups (the part that hadn't been picked by individual fs
trees)"
* tag 'pull-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
apparmorfs: don't duplicate kfree_link()
orangefs: saner arguments passing in readdir guts
ocfs2_find_match(): there's no such thing as NULL or negative ->d_parent
reiserfs_add_entry(): get rid of pointless namelen checks
__ocfs2_add_entry(), ocfs2_prepare_dir_for_insert(): namelen checks
ext4_add_entry(): ->d_name.len is never 0
befs: d_obtain_alias(ERR_PTR(...)) will do the right thing
affs: d_obtain_alias(ERR_PTR(...)) will do the right thing
/proc/sys: use d_splice_alias() calling conventions to simplify failure exits
hostfs: use d_splice_alias() calling conventions to simplify failure exits
udf_fiiter_add_entry(): check for zero ->d_name.len is bogus...
udf: d_obtain_alias(ERR_PTR(...)) will do the right thing...
udf: d_splice_alias() will do the right thing on ERR_PTR() inode
nfsd: kill stale comment about simple_fill_super() requirements
bfs_add_entry(): get rid of pointless ->d_name.len checks
nilfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing...
zonefs: d_splice_alias() will do the right thing on ERR_PTR() inode
|
|
Prevent move_mount from applying the attach_disconnected flag
to move_mount(). This prevents detached mounts from appearing
as / when applying mount mediation, which is not only incorrect
but could result in bad policy being generated.
Basic mount rules like
allow mount,
allow mount options=(move) -> /target/,
will allow detached mounts, allowing older policy to continue
to function. New policy gains the ability to specify `detached` as
a source option
allow mount detached -> /target/,
In addition make sure support of move_mount is advertised as
a feature to userspace so that applications that generate policy
can respond to the addition.
Note: this fixes mediation of move_mount when a detached mount is used,
it does not fix the broader regression of apparmor mediation of
mounts under the new mount api.
Link: https://lore.kernel.org/all/68c166b8-5b4d-4612-8042-1dee3334385b@leemhuis.info/T/#mb35fdde37f999f08f0b02d58dc1bf4e6b65b8da2
Fixes: 157a3537d6bc ("apparmor: Fix regression in mount mediation")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
rawdata_link_cb() is identical to it
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
|
|
sha1 is insecure and has colisions, thus it is not useful for even
lightweight policy hash checks. Switch to sha256, which on modern
hardware is fast enough.
Separately as per NIST Policy on Hash Functions, sha1 usage must be
withdrawn by 2030. This config option currently is one of many that
holds up sha1 usage.
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull apparmor updates from John Johansen:
"This adds initial support for mediating io_uring and userns creation.
Adds a new restriction that tightens the use of change_profile, and a
couple of optimizations to reduce performance bottle necks that have
been found when retrieving the current task's secid and allocating
work buffers.
The majority of the patch set continues cleaning up and simplifying
the code (fixing comments, removing now dead functions, and macros
etc). Finally there are 4 bug fixes, with the regression fix having
had a couple months of testing.
Features:
- optimize retrieving current task secid
- add base io_uring mediation
- add base userns mediation
- improve buffer allocation
- allow restricting unprivilege change_profile
Cleanups:
- Fix kernel doc comments
- remove unused declarations
- remove unused functions
- remove unneeded #ifdef
- remove unused macros
- mark fns static
- cleanup fn with unused return values
- cleanup audit data
- pass cred through to audit data
- refcount the pdb instead of using duplicates
- make SK_CTX macro an inline fn
- some comment cleanups
Bug fixes:
- fix regression in mount mediation
- fix invalid refenece
- use passed in gfp flags
- advertise avaiability of extended perms and disconnected.path"
* tag 'apparmor-pr-2023-11-03' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: (39 commits)
apparmor: Fix some kernel-doc comments
apparmor: Fix one kernel-doc comment
apparmor: Fix some kernel-doc comments
apparmor: mark new functions static
apparmor: Fix regression in mount mediation
apparmor: cache buffers on percpu list if there is lock contention
apparmor: add io_uring mediation
apparmor: add user namespace creation mediation
apparmor: allow restricting unprivileged change_profile
apparmor: advertise disconnected.path is available
apparmor: refcount the pdb
apparmor: provide separate audit messages for file and policy checks
apparmor: pass cred through to audit info.
apparmor: rename audit_data->label to audit_data->subj_label
apparmor: combine common_audit_data and apparmor_audit_data
apparmor: rename SK_CTX() to aa_sock and make it an inline fn
apparmor: Optimize retrieving current task secid
apparmor: remove unused functions in policy_ns.c/.h
apparmor: remove unneeded #ifdef in decompress_zstd()
apparmor: fix invalid reference on profile->disconnected
...
|
|
For now, the io_uring mediation is limited to sqpoll and
override_creds.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Unprivileged user namespace creation is often used as a first step
in privilege escalation attacks. Instead of disabling it at the
sysrq level, which blocks its legitimate use as for setting up a sandbox,
allow control on a per domain basis.
This allows an admin to quickly lock down a system while also still
allowing legitimate use.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
unprivileged unconfined can use change_profile to alter the confinement
set by the mac admin.
Allow restricting unprivileged unconfined by still allowing change_profile
but stacking the change against unconfined. This allows unconfined to
still apply system policy but allows the task to enter the new confinement.
If unprivileged unconfined is required a sysctl is provided to switch
to the previous behavior.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
While disconnected.path has been available for a while it was never
properly advertised as a feature. Fix this so that userspace doesn't
need special casing to handle it.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
With the move to permission tables the dfa is no longer a stand
alone entity when used, needing a minimum of a permission table.
However it still could be shared among different pdbs each using
a different permission table.
Instead of duping the permission table when sharing a pdb, add a
refcount to the pdb so it can be easily shared.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The cred is needed to properly audit some messages, and will be needed
in the future for uid conditional mediation. So pass it through to
where the apparmor_audit_data struct gets defined.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Convert to using the new inode timestamp accessor functions.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://lore.kernel.org/r/20231004185347.80880-82-jlayton@kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
|
|
The whole function is guarded by CONFIG_SECURITY_APPARMOR_EXPORT_BINARY,
so the #ifdef here is redundant, remove it.
Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
In later patches, we're going to change how the inode's ctime field is
used. Switch to using accessor functions instead of raw accesses of
inode->i_ctime.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Jan Kara <jack@suse.cz>
Message-Id: <20230705190309.579783-87-jlayton@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
|
|
In general, when updating the mtime on an inode, one must also update
the ctime. Add the missing ctime updates.
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Message-Id: <20230705190309.579783-5-jlayton@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
|
|
Userspace won't load policy using extended perms unless it knows the
kernel can handle them. Advertise that extended perms are supported in
the feature set.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Jon Tourville <jontourville@me.com>
|
|
Convert to struct mnt_idmap.
Last cycle we merged the necessary infrastructure in
256c8aed2b42 ("fs: introduce dedicated idmap type for mounts").
This is just the conversion to struct mnt_idmap.
Currently we still pass around the plain namespace that was attached to a
mount. This is in general pretty convenient but it makes it easy to
conflate namespaces that are relevant on the filesystem with namespaces
that are relevent on the mount level. Especially for non-vfs developers
without detailed knowledge in this area this can be a potential source for
bugs.
Once the conversion to struct mnt_idmap is done all helpers down to the
really low-level helpers will take a struct mnt_idmap argument instead of
two namespace arguments. This way it becomes impossible to conflate the two
eliminating the possibility of any bugs. All of the vfs and all filesystems
only operate on struct mnt_idmap.
Acked-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
|
|
The rawdata readback has a few of problems. First if compression is
enabled when the data is read then the compressed data is read out
instead decompressing the data. Second if compression of the data
fails, the code does not handle holding onto the raw_data in
uncompressed form. Third if the compression is enabled/disabled after
the rawdata was loaded, the check against the global control of
whether to use compression does not reflect what was already done to
the data.
Fix these by always storing the compressed size, along with the
original data size even if compression fails or is not used. And use
this to detect whether the rawdata is actually compressed.
Fixes: 52ccc20c652b ("apparmor: use zstd compression for profile data")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Jon Tourville <jon.tourville@canonical.com>
|
|
Unfortunately the switch to using zstd compression did not properly
ifdef all the code that uses zstd_ symbols. So that if exporting of
binary policy is disabled in the config the compile will fail with the
following errors
security/apparmor/lsm.c:1545: undefined reference to `zstd_min_clevel'
aarch64-linux-ld: security/apparmor/lsm.c:1545: undefined reference to `zstd_max_clevel'
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 52ccc20c652b ("apparmor: use zstd compression for profile data")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Jon Tourville <jon.tourville@canonical.com>
|
|
The decompress ctx was not properly initialized when reading raw
profile data back to userspace.
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 52ccc20c652b ("apparmor: use zstd compression for profile data")
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Convert profile->rules to a list as the next step towards supporting
multiple rulesets in a profile. For this step only support a single
list entry item. The logic for iterating the list will come as a
separate step.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
In preparation for moving from a single set of rules and a single
attachment to multiple rulesets and attachments separate from the
profile refactor attachment information and ruleset info into their
own structures.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Convert from an unsigned int to a state_t for state position. This is
a step in prepping for the state position carrying some additional
flags, and a limited form of backtracking to support variables.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Remap polidydb dfa accept table from embedded perms to an index, and
then move the perm lookup to use the accept entry as an index into the
perm table. This is done so that the perm table can be separated from
the dfa, allowing dfa accept to index to share expanded permission
sets.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
continue permission unification by converting xmatch to use the
policydb struct that is used by the other profile dfas.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
file_rules and policydb are almost the same and will need the same
features in the future so combine them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Rather than computing policydb permissions for each access
permissions can be computed once on profile load and stored for lookup.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Rather than computing file permissions for each file access, file
permissions can be computed once on profile load and stored for lookup.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Create two new files in apparmor's sysfs:
/sys/kernel/security/apparmor/raw_data_compression_level_min
/sys/kernel/security/apparmor/raw_data_compression_level_max
These correspond to the minimum and maximum zstd compression levels
that can be assigned to the apparmor module parameter
raw_data_compression_level.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Change the algorithm used by apparmor to compress profile data from
zlib to zstd, using the new zstd API introduced in 5.16.
Zstd provides a larger range of compression levels than zlib and
significantly better performance at the default level (for a relatively
small increase in compressed size).
The apparmor module parameter raw_data_compression_level is now clamped
to the minimum and maximum compression levels reported by the zstd
library. A compression level of 0 retains the previous behavior of
disabling policy compression instead of using zstd's behavior, which is
to use the default compression level.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
In multi_transaction_new(), the variable t is not freed or passed out
on the failure of copy_from_user(t->data, buf, size), which could lead
to a memleak.
Fix this bug by adding a put_multi_transaction(t) in the error path.
Fixes: 1dea3b41e84c5 ("apparmor: speed up transactional queries")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
AppArmor split out task oriented controls to their own logical file
a while ago. Ptrace mediation is better grouped with task than
ipc, so move it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The policydb permission set has left the xbits unused. Make them available
for mediation.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
When copy_from_user failed, the memory is freed by kvfree. however the
management struct and data blob are allocated independently, so only
kvfree(data) cause a memleak issue here. Use aa_put_loaddata(data) to
fix this issue.
Fixes: a6a52579e52b5 ("apparmor: split load data into management struct and data blob")
Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
IF CONFIG_SECURITY_APPARMOR_EXPORT_BINARY is disabled, there remains
some unneed references to zlib, and can result in undefined symbol
references if ZLIB_INFLATE or ZLIB_DEFLATE are not defined.
Reported-by: kernel test robot <lkp@intel.com>
Fixes: abfb9c0725f2 ("apparmor: make export of raw binary profile to userspace optional")
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Don't use /** for non-kernel-doc comments and change function name
aa_mangle_name to mangle_name in kernel-doc comment to Remove some
warnings found by running scripts/kernel-doc, which is caused by
using 'make W=1'.
security/apparmor/apparmorfs.c:1503: warning: Cannot understand *
on line 1503 - I thought it was a doc line
security/apparmor/apparmorfs.c:1530: warning: Cannot understand *
on line 1530 - I thought it was a doc line
security/apparmor/apparmorfs.c:1892: warning: Cannot understand *
on line 1892 - I thought it was a doc line
security/apparmor/apparmorfs.c:108: warning: expecting prototype for
aa_mangle_name(). Prototype was for mangle_name() instead
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Currently if sha1 hashing of policy is disabled a sha1 hash symlink
to the non-existent file is created. There is now reason to create
the symlink in this case so don't do it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Embedded systems have limited space and don't need the introspection
or checkpoint restore capability provided by exporting the raw
profile binary data so make it so make it a config option.
This will reduce run time memory use and also speed up policy loads.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull apparmor updates from John Johansen:
"Features
- use per file locks for transactional queries
- update policy management capability checks to work with LSM stacking
Bug Fixes:
- check/put label on apparmor_sk_clone_security()
- fix error check on update of label hname
- fix introspection of of task mode for unconfined tasks
Cleanups:
- avoid -Wempty-body warning
- remove duplicated 'Returns:' comments
- fix doc warning
- remove unneeded one-line hook wrappers
- use struct_size() helper in kzalloc()
- fix zero-length compiler warning in AA_BUG()
- file.h: delete duplicated word
- delete repeated words in comments
- remove repeated declaration"
* tag 'apparmor-pr-2021-11-10' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
apparmor: remove duplicated 'Returns:' comments
apparmor: remove unneeded one-line hook wrappers
apparmor: Use struct_size() helper in kzalloc()
apparmor: fix zero-length compiler warning in AA_BUG()
apparmor: use per file locks for transactional queries
apparmor: fix doc warning
apparmor: Remove the repeated declaration
apparmor: avoid -Wempty-body warning
apparmor: Fix internal policy capable check for policy management
apparmor: fix error check
security: apparmor: delete repeated words in comments
security: apparmor: file.h: delete duplicated word
apparmor: switch to apparmor to internal capable check for policy management
apparmor: update policy capable checks to use a label
apparmor: fix introspection of of task mode for unconfined tasks
apparmor: check/put label on apparmor_sk_clone_security()
|
|
As made mention of in commit 1dea3b41e84c5 ("apparmor: speed up
transactional queries"), a single lock is currently used to synchronize
transactional queries. We can, use the lock allocated for each file by
VFS instead.
Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Fix gcc W=1 warning:
security/apparmor/apparmorfs.c:2125: warning: Function parameter or member 'p' not described in '__next_profile'
Signed-off-by: ChenXiaoSong <chenxiaosong2@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Previously the policy capable checks assumed they were using the
current task. Make them take the task label so the query can be
made against an arbitrary task.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Extend some inode methods with an additional user namespace argument. A
filesystem that is aware of idmapped mounts will receive the user
namespace the mount has been marked with. This can be used for
additional permission checking and also to enable filesystems to
translate between uids and gids if they need to. We have implemented all
relevant helpers in earlier patches.
As requested we simply extend the exisiting inode method instead of
introducing new ones. This is a little more code churn but it's mostly
mechanical and doesnt't leave us with additional inode methods.
Link: https://lore.kernel.org/r/20210121131959.646623-25-christian.brauner@ubuntu.com
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Howells <dhowells@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
|
|
Strangely I hadn't had noticed the existence of the list_entry_is_head()
in apparmor code when added the same one in the list.h. Luckily it's
fully identical and didn't break builds. In any case we don't need a
duplicate anymore, thus remove it from apparmor code.
Link: https://lkml.kernel.org/r/20201208100639.88182-1-andriy.shevchenko@linux.intel.com
Fixes: e130816164e244 ("include/linux/list.h: add a macro to test if entry is pointing to the head")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E . Hallyn " <serge@hallyn.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull apparmor updates from John Johansen:
"Features:
- Replace zero-length array with flexible-array
- add a valid state flags check
- add consistency check between state and dfa diff encode flags
- add apparmor subdir to proc attr interface
- fail unpack if profile mode is unknown
- add outofband transition and use it in xattr match
- ensure that dfa state tables have entries
Cleanups:
- Use true and false for bool variable
- Remove semicolon
- Clean code by removing redundant instructions
- Replace two seq_printf() calls by seq_puts() in aa_label_seq_xprint()
- remove duplicate check of xattrs on profile attachment
- remove useless aafs_create_symlink
Bug fixes:
- Fix memory leak of profile proxy
- fix introspection of of task mode for unconfined tasks
- fix nnp subset test for unconfined
- check/put label on apparmor_sk_clone_security()"
* tag 'apparmor-pr-2020-06-07' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
apparmor: Fix memory leak of profile proxy
apparmor: fix introspection of of task mode for unconfined tasks
apparmor: check/put label on apparmor_sk_clone_security()
apparmor: Use true and false for bool variable
security/apparmor/label.c: Clean code by removing redundant instructions
apparmor: Replace zero-length array with flexible-array
apparmor: ensure that dfa state tables have entries
apparmor: remove duplicate check of xattrs on profile attachment.
apparmor: add outofband transition and use it in xattr match
apparmor: fail unpack if profile mode is unknown
apparmor: fix nnp subset test for unconfined
apparmor: remove useless aafs_create_symlink
apparmor: add proc subdir to attrs
apparmor: add consistency check between state and dfa diff encode flags
apparmor: add a valid state flags check
AppArmor: Remove semicolon
apparmor: Replace two seq_printf() calls by seq_puts() in aa_label_seq_xprint()
|
|
policy_update() invokes begin_current_label_crit_section(), which
returns a reference of the updated aa_label object to "label" with
increased refcount.
When policy_update() returns, "label" becomes invalid, so the refcount
should be decreased to keep refcount balanced.
The reference counting issue happens in one exception handling path of
policy_update(). When aa_may_manage_policy() returns not NULL, the
refcnt increased by begin_current_label_crit_section() is not decreased,
causing a refcnt leak.
Fix this issue by jumping to "end_section" label when
aa_may_manage_policy() returns not NULL.
Fixes: 5ac8c355ae00 ("apparmor: allow introspecting the loaded policy pre internal transform")
Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull openat2 support from Al Viro:
"This is the openat2() series from Aleksa Sarai.
I'm afraid that the rest of namei stuff will have to wait - it got
zero review the last time I'd posted #work.namei, and there had been a
leak in the posted series I'd caught only last weekend. I was going to
repost it on Monday, but the window opened and the odds of getting any
review during that... Oh, well.
Anyway, openat2 part should be ready; that _did_ get sane amount of
review and public testing, so here it comes"
From Aleksa's description of the series:
"For a very long time, extending openat(2) with new features has been
incredibly frustrating. This stems from the fact that openat(2) is
possibly the most famous counter-example to the mantra "don't silently
accept garbage from userspace" -- it doesn't check whether unknown
flags are present[1].
This means that (generally) the addition of new flags to openat(2) has
been fraught with backwards-compatibility issues (O_TMPFILE has to be
defined as __O_TMPFILE|O_DIRECTORY|[O_RDWR or O_WRONLY] to ensure old
kernels gave errors, since it's insecure to silently ignore the
flag[2]). All new security-related flags therefore have a tough road
to being added to openat(2).
Furthermore, the need for some sort of control over VFS's path
resolution (to avoid malicious paths resulting in inadvertent
breakouts) has been a very long-standing desire of many userspace
applications.
This patchset is a revival of Al Viro's old AT_NO_JUMPS[3] patchset
(which was a variant of David Drysdale's O_BENEATH patchset[4] which
was a spin-off of the Capsicum project[5]) with a few additions and
changes made based on the previous discussion within [6] as well as
others I felt were useful.
In line with the conclusions of the original discussion of
AT_NO_JUMPS, the flag has been split up into separate flags. However,
instead of being an openat(2) flag it is provided through a new
syscall openat2(2) which provides several other improvements to the
openat(2) interface (see the patch description for more details). The
following new LOOKUP_* flags are added:
LOOKUP_NO_XDEV:
Blocks all mountpoint crossings (upwards, downwards, or through
absolute links). Absolute pathnames alone in openat(2) do not
trigger this. Magic-link traversal which implies a vfsmount jump is
also blocked (though magic-link jumps on the same vfsmount are
permitted).
LOOKUP_NO_MAGICLINKS:
Blocks resolution through /proc/$pid/fd-style links. This is done
by blocking the usage of nd_jump_link() during resolution in a
filesystem. The term "magic-links" is used to match with the only
reference to these links in Documentation/, but I'm happy to change
the name.
It should be noted that this is different to the scope of
~LOOKUP_FOLLOW in that it applies to all path components. However,
you can do openat2(NO_FOLLOW|NO_MAGICLINKS) on a magic-link and it
will *not* fail (assuming that no parent component was a
magic-link), and you will have an fd for the magic-link.
In order to correctly detect magic-links, the introduction of a new
LOOKUP_MAGICLINK_JUMPED state flag was required.
LOOKUP_BENEATH:
Disallows escapes to outside the starting dirfd's
tree, using techniques such as ".." or absolute links. Absolute
paths in openat(2) are also disallowed.
Conceptually this flag is to ensure you "stay below" a certain
point in the filesystem tree -- but this requires some additional
to protect against various races that would allow escape using
"..".
Currently LOOKUP_BENEATH implies LOOKUP_NO_MAGICLINKS, because it
can trivially beam you around the filesystem (breaking the
protection). In future, there might be similar safety checks done
as in LOOKUP_IN_ROOT, but that requires more discussion.
In addition, two new flags are added that expand on the above ideas:
LOOKUP_NO_SYMLINKS:
Does what it says on the tin. No symlink resolution is allowed at
all, including magic-links. Just as with LOOKUP_NO_MAGICLINKS this
can still be used with NOFOLLOW to open an fd for the symlink as
long as no parent path had a symlink component.
LOOKUP_IN_ROOT:
This is an extension of LOOKUP_BENEATH that, rather than blocking
attempts to move past the root, forces all such movements to be
scoped to the starting point. This provides chroot(2)-like
protection but without the cost of a chroot(2) for each filesystem
operation, as well as being safe against race attacks that
chroot(2) is not.
If a race is detected (as with LOOKUP_BENEATH) then an error is
generated, and similar to LOOKUP_BENEATH it is not permitted to
cross magic-links with LOOKUP_IN_ROOT.
The primary need for this is from container runtimes, which
currently need to do symlink scoping in userspace[7] when opening
paths in a potentially malicious container.
There is a long list of CVEs that could have bene mitigated by
having RESOLVE_THIS_ROOT (such as CVE-2017-1002101,
CVE-2017-1002102, CVE-2018-15664, and CVE-2019-5736, just to name a
few).
In order to make all of the above more usable, I'm working on
libpathrs[8] which is a C-friendly library for safe path resolution.
It features a userspace-emulated backend if the kernel doesn't support
openat2(2). Hopefully we can get userspace to switch to using it, and
thus get openat2(2) support for free once it's ready.
Future work would include implementing things like
RESOLVE_NO_AUTOMOUNT and possibly a RESOLVE_NO_REMOTE (to allow
programs to be sure they don't hit DoSes though stale NFS handles)"
* 'work.openat2' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
Documentation: path-lookup: include new LOOKUP flags
selftests: add openat2(2) selftests
open: introduce openat2(2) syscall
namei: LOOKUP_{IN_ROOT,BENEATH}: permit limited ".." resolution
namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution
namei: LOOKUP_NO_XDEV: block mountpoint crossing
namei: LOOKUP_NO_MAGICLINKS: block magic-link resolution
namei: LOOKUP_NO_SYMLINKS: block symlink resolution
namei: allow set_root() to produce errors
namei: allow nd_jump_link() to produce errors
nsfs: clean-up ns_get_path() signature to return int
namei: only return -ECHILD from follow_dotdot_rcu()
|
|
There are cases where the a special out of band transition that can
not be triggered by input is useful in separating match conditions
in the dfa encoding.
The null_transition is currently used as an out of band transition
for match conditions that can not contain a \0 in their input
but apparmor needs an out of band transition for cases where
the match condition is allowed to contain any input character.
Achieve this by allowing for an explicit transition out of input
range that can only be triggered by code.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
