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: [email protected]
Signed-off-by: Zubin Mithra <[email protected]>
---
security/apparmor/lib.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 974affe50531..76491e7f4177 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -90,10 +90,12 @@ const char *aa_splitn_fqname(const char *fqname, size_t n, const char **ns_name,
const char *end = fqname + n;
const char *name = skipn_spaces(fqname, n);
- if (!name)
- return NULL;
*ns_name = NULL;
*ns_len = 0;
+
+ if (!name)
+ return NULL;
+
if (name[0] == ':') {
char *split = strnchr(&name[1], end - &name[1], ':');
*ns_name = skipn_spaces(&name[1], end - &name[1]);
--
2.19.0.605.g01d371f741-goog
On Thu, Sep 27, 2018 at 2:49 PM, Zubin Mithra <[email protected]> wrote:
> 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: [email protected]
> Signed-off-by: Zubin Mithra <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> security/apparmor/lib.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
> index 974affe50531..76491e7f4177 100644
> --- a/security/apparmor/lib.c
> +++ b/security/apparmor/lib.c
> @@ -90,10 +90,12 @@ const char *aa_splitn_fqname(const char *fqname, size_t n, const char **ns_name,
> const char *end = fqname + n;
> const char *name = skipn_spaces(fqname, n);
>
> - if (!name)
> - return NULL;
> *ns_name = NULL;
> *ns_len = 0;
> +
> + if (!name)
> + return NULL;
> +
> if (name[0] == ':') {
> char *split = strnchr(&name[1], end - &name[1], ':');
> *ns_name = skipn_spaces(&name[1], end - &name[1]);
> --
> 2.19.0.605.g01d371f741-goog
>
--
Kees Cook
Pixel Security