apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init

[ Upstream commit b31d3f7385fbb49681d44e7104cfa033cba4b1e8 ]

strcpy() is deprecated and sprintf() does not perform bounds checking
either. Although an overflow is unlikely, it's better to proactively
avoid it by using the safer strscpy() and scnprintf(), respectively.

Additionally, unify memory allocation for 'hname' to simplify and
improve aa_policy_init().

Closes: https://github.com/KSPP/linux/issues/88
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Stable-dep-of: 3daad923a868 ("apparmor: policy_int make sure list heads are initialized before fail path")
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 82dbb97ad406584761d655dd9e72cf08b91e185f..acf7f5189beca79e7ee575fbd88f5bc30e75e0ed 100644 (file)
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -478,19 +478,17 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
                    const char *name, gfp_t gfp)
 {
        char *hname;
+       size_t hname_sz;
 
+       hname_sz = (prefix ? strlen(prefix) + 2 : 0) + strlen(name) + 1;
        /* freed by policy_free */
-       if (prefix) {
-               hname = aa_str_alloc(strlen(prefix) + strlen(name) + 3, gfp);
-               if (hname)
-                       sprintf(hname, "%s//%s", prefix, name);
-       } else {
-               hname = aa_str_alloc(strlen(name) + 1, gfp);
-               if (hname)
-                       strcpy(hname, name);
-       }
+       hname = aa_str_alloc(hname_sz, gfp);
        if (!hname)
                return false;
+       if (prefix)
+               scnprintf(hname, hname_sz, "%s//%s", prefix, name);
+       else
+               strscpy(hname, name, hname_sz);
        policy->hname = hname;
        /* base.name is a substring of fqname */
        policy->name = basename(policy->hname);