Quoting Tyler Hicks (tyhi...@canonical.com):
> 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 <tyhi...@canonical.com>
Acked-by: Serge Hallyn <se...@hallyn.com>
> ---
> security/apparmor/file.c | 3 ++-
> security/apparmor/include/perms.h | 3 ++-
> security/apparmor/lib.c | 17 +++++++++++++----
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 224b2fef93ca..4285943f7260 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -47,7 +47,8 @@ static void audit_file_mask(struct audit_buffer *ab, u32
> mask)
> {
> char str[10];
>
> - aa_perm_mask_to_str(str, aa_file_perm_chrs, map_mask_to_chr_mask(mask));
> + aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs,
> + map_mask_to_chr_mask(mask));
> audit_log_string(ab, str);
> }
>
> diff --git a/security/apparmor/include/perms.h
> b/security/apparmor/include/perms.h
> index 38aa6247d00f..b94ec114d1a4 100644
> --- a/security/apparmor/include/perms.h
> +++ b/security/apparmor/include/perms.h
> @@ -137,7 +137,8 @@ extern struct aa_perms allperms;
> xcheck(fn_for_each((L1), (P), (FN1)), fn_for_each((L2), (P), (FN2)))
>
>
> -void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask);
> +void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs,
> + u32 mask);
> void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names,
> u32 mask);
> void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
> diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
> index a7b3f681b80e..7ab368c3789b 100644
> --- a/security/apparmor/lib.c
> +++ b/security/apparmor/lib.c
> @@ -198,15 +198,24 @@ const char *aa_file_perm_names[] = {
> /**
> * aa_perm_mask_to_str - convert a perm mask to its short string
> * @str: character buffer to store string in (at least 10 characters)
> + * @str_size: size of the @str buffer
> + * @chrs: NUL-terminated character buffer of permission characters
> * @mask: permission mask to convert
> */
> -void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask)
> +void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, u32
> mask)
> {
> unsigned int i, perm = 1;
> + size_t num_chrs = strlen(chrs);
> +
> + for (i = 0; i < num_chrs; perm <<= 1, i++) {
> + if (mask & perm) {
> + /* Ensure that one byte is left for NUL-termination */
> + if (WARN_ON_ONCE(str_size <= 1))
> + continue;
>
> - for (i = 0; i < 32; perm <<= 1, i++) {
> - if (mask & perm)
> *str++ = chrs[i];
> + str_size--;
> + }
> }
> *str = '\0';
> }
> @@ -236,7 +245,7 @@ void aa_audit_perm_mask(struct audit_buffer *ab, u32
> mask, const char *chrs,
>
> audit_log_format(ab, "\"");
> if ((mask & chrsmask) && chrs) {
> - aa_perm_mask_to_str(str, chrs, mask & chrsmask);
> + aa_perm_mask_to_str(str, sizeof(str), chrs, mask & chrsmask);
> mask &= ~chrsmask;
> audit_log_format(ab, "%s", str);
> if (mask & namesmask)
> --
> 2.7.4
