- 01 Nov, 2022 1 commit
-
-
Xiu Jianfeng authored
After changes in commit a1bd627b ("apparmor: share profile name on replacement"), the hname member of struct aa_policy is not valid slab object, but a subset of that, it can not be freed by kfree_sensitive(), use aa_policy_destroy() to fix it. Fixes: a1bd627b ("apparmor: share profile name on replacement") Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
- 25 Oct, 2022 11 commits
-
-
Xiu Jianfeng authored
Before aa_alloc_profile(), it has allocated string for @*ns_name if @tmpns is not NULL, so directly return -ENOMEM if aa_alloc_profile() failed will cause a memleak issue, and even if aa_alloc_profile() succeed, in the @fail_profile tag of aa_unpack(), it need to free @ns_name as well, this patch fixes them. Fixes: 736ec752 ("AppArmor: policy routines for loading and unpacking policy") Fixes: 04dc715e ("apparmor: audit policy ns specified in policy load") Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Gaosheng Cui authored
When the aa_profile is released, we will call free_ruleset to release aa_ruleset, but we don't free the memory of aa_ruleset, so there will be memleak, fix it. unreferenced object 0xffff8881475df800 (size 1024): comm "apparmor_parser", pid 883, jiffies 4294899650 (age 9114.088s) hex dump (first 32 bytes): 00 f8 5d 47 81 88 ff ff 00 f8 5d 47 81 88 ff ff ..]G......]G.... 00 00 00 00 00 00 00 00 00 dc 65 47 81 88 ff ff ..........eG.... backtrace: [<00000000370e658e>] __kmem_cache_alloc_node+0x182/0x700 [<00000000f2f5a6d2>] kmalloc_trace+0x2c/0x130 [<00000000c5c905b3>] aa_alloc_profile+0x1bc/0x5c0 [<00000000bc4fa72b>] unpack_profile+0x319/0x30c0 [<00000000eab791e9>] aa_unpack+0x307/0x1450 [<000000002c3a6ee1>] aa_replace_profiles+0x1b8/0x3790 [<00000000d0c3fd54>] policy_update+0x35a/0x890 [<00000000d04fed90>] profile_replace+0x1d1/0x260 [<00000000cba0c0a7>] vfs_write+0x283/0xd10 [<000000006bae64a5>] ksys_write+0x134/0x260 [<00000000b2fd8f31>] __x64_sys_write+0x78/0xb0 [<00000000f3c8a015>] do_syscall_64+0x5c/0x90 [<00000000a242b1db>] entry_SYSCALL_64_after_hwframe+0x63/0xcd Fixes: 217af7e2 ("apparmor: refactor profile rules and attachments") Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Yang Li authored
'resouce' -> 'resource' Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2396Reported-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>
-
Xiu Jianfeng authored
According to the implementations of cred_label() and set_cred_label(), we should use pointer to struct aa_label for lbs_cred instead of struct aa_task_ctx, this patch fixes it. Fixes: bbd3662a ("Infrastructure management of the cred security blob") Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Jiapeng Chong authored
security/apparmor/ipc.c:53: warning: expecting prototype for audit_cb(). Prototype was for audit_signal_cb() instead. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2337Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Jiapeng Chong authored
security/apparmor/lsm.c:753: warning: expecting prototype for apparmor_bprm_committed_cred(). Prototype was for apparmor_bprm_committed_creds() instead. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2338Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Jiapeng Chong authored
security/apparmor/audit.c:93: warning: expecting prototype for audit_base(). Prototype was for audit_pre() instead. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2339Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Unfortunately it is possible for some userspace's to load children profiles before the parent profile. This can even happen when the child and the parent are in different load sets. Fix this by creating a null place holder profile that grants no permissions and can be replaced by the parent once it is loaded. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Bother unconfined and learning profiles use the null profile as their base. Refactor so they are share a common base routine. This doesn't save much atm but will be important when the feature set of the parent is inherited. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Gaosheng Cui authored
Update the comments for aa_getprocattr() and audit_resource(), the args of them have beed changed since commit 76a1d263 ("apparmor: switch getprocattr to using label_print fns()"). Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Gaosheng Cui authored
Remove the following useless static inline functions: 1. label_is_visible() is a static function in security/apparmor/label.c, and it's not used, aa_ns_visible() can do the same things as it, so it's redundant. 2. is_deleted() is a static function in security/apparmor/file.c, and it's not used since commit aebd873e ("apparmor: refactor path name lookup and permission checks around labels"), so it's redundant. They are redundant, so remove them. Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
- 11 Oct, 2022 1 commit
-
-
John Johansen authored
unpack_profile() sets a default error on entry but this gets overridden by error assignment by functions called in its body. If an error check that was relying on the default value is triggered after one of these error assignments then zero will be passed to ERR_PTR. Fix this by setting up a default -EPROTO assignment in the error path and while we are at it make sure the correct error is returned in non-default cases. Fixes: 217af7e2 ("apparmor: refactor profile rules and attachments") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
- 10 Oct, 2022 1 commit
-
-
John Johansen authored
The error path has one case where *table is uninitialized, initialize it. Fixes: a0792e2c ("apparmor: make transition table unpack generic so it can be reused") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
- 04 Oct, 2022 1 commit
-
-
Muhammad Usama Anjum authored
The unpack_perms_table() can return error which is negative value. Store the return value to a signed variable. policy->size is unsigned variable. It shouldn't be used to store the return status. Fixes: 2d6b2dea7f3c ("apparmor: add the ability for policy to specify a permission table") Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
- 03 Oct, 2022 25 commits
-
-
John Johansen authored
The apparmor kunit tests are failing on the out of bounds array check with the following failure # policy_unpack_test_unpack_array_out_of_bounds: EXPECTATION FAILED at security/apparmor/policy_unpack_test.c:178 Expected unpack_array(puf->e, name, &array_size) == 1, but unpack_array(puf->e, name, &array_size) == -1 # policy_unpack_test_unpack_array_out_of_bounds: EXPECTATION FAILED at security/apparmor/policy_unpack_test.c:180 Expected array_size == 0, but array_size == 64192 not ok 5 - policy_unpack_test_unpack_array_out_of_bounds This is because unpack_array changed to allow distinguishing between the array not being present and an error. In the error case the array size is not set and should not be tested. Reported-by: kernel test robot <yujie.liu@intel.com> Fixes: 995a5b64620e ("apparmor: make unpack_array return a trianary value") Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
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>
-
John Johansen authored
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>
-
John Johansen authored
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>
-
John Johansen authored
The index into the trans_table has a max size of 2^24 bits which the code was testing but this is unnecessary as unpack_array can only unpack a table of 2^16 bits in size so the table unpacked will never be larger than what can be indexed, and any test here is redundant. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
When compute_fperms was moved to policy_compat and made static it was renamed from aa_compute_fperms to just compute_fperms to help indicate it is only available statically. Unfortunately the doc comment did not also get updated to reflect the change. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Xiu Jianfeng authored
Make __aa_path_perm() static as it's only used inside apparmor/file.c. Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
Gaosheng Cui authored
In aa_get_task_label(), aa_get_newest_cred_label(__task_cred(task)) can do the same things as aa_get_newest_label(__aa_task_raw_label(task)), so we can replace it and remove __aa_task_raw_label() to simplify the code. Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
The unconfined label flag is not being computed correctly. It should only be set if all the profiles in the vector are set, which is different than what is required for the debug and stale flag that are set if any on the profile flags are set. Fixes: c1ed5da1 ("apparmor: allow label to carry debug flags") Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
The class name map did not have the reserved names added. Fix this Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
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>
-
John Johansen authored
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>
-
John Johansen authored
Add an additional verification that loaded permission sets don't overlap in ways that are not intended. This will help ensure that permission accumulation can't result in an invalid permission set. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Perm accumulation is going to be used much more frequently so let the compiler figure out if it can be optimized when used. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
accumulate permission indexes on a first encountered basis. This favors original rulesets so that new ones can not override without profile replacement. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
While the dfa xindex's are verified, the indexes in the permission table are not currently verified. Fix this. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Currently permissions are encoded in the dfa accept entries that are then mapped to an internal permission structure. This limits the permissions that userspace can specify, so allow userspace to directly specify the permission table. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
currently unpack_array() does not return an error nor whether the array is not present. The ability to detect an error or the array not being present is needed so rework the unpack_array() to return the needed information. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
There are currently three policydb rule groupings (xmatch, file, policydb) that each do their own slightly different thing. Group them into a single routine and unify. This extends/unifies dfa features by - all dfas are allowed having an optional start field - all dfas are allowed having a string/transition table Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Currently the transition table is tied to the file dfa. Make it so we can unpack a transition table against any dfa. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Allow the profile to contain a user mode prompt flag. This works similar to complain mode but will try to send messages to a userspace daemon. If the daemon is not present or timesout regular informent will occur. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Audit messages currently don't contain the mediation class which can make them less clear than they should be in some circumstances. With newer mediation classes coming this potential confusion will become worse. Fix this by adding the mediatin class to the messages. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
add indexes for label and tag entries. Rename the domain table to the str_table as its a shared string table with label and tags. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
The details of mapping old policy into newer policy formats clutters up the unpack code and makes it possible to accidentally use old mappings in code, so isolate the mapping code into its own file. This will become more important when the dfa remapping code lands, as it will greatly expand the compat code base. Signed-off-by: John Johansen <john.johansen@canonical.com>
-
John Johansen authored
Allow the xindex to have 2^24 entries. Signed-off-by: John Johansen <john.johansen@canonical.com>
-