All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serge@hallyn.com>
To: Tyler Hicks <tyhicks@canonical.com>
Cc: John Johansen <john.johansen@canonical.com>,
	James Morris <jmorris@namei.org>, Serge Hallyn <serge@hallyn.com>,
	Seth Arnold <seth.arnold@canonical.com>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] apparmor: Fully initialize aa_perms struct when answering userspace query
Date: Tue, 10 Jul 2018 10:03:58 -0500	[thread overview]
Message-ID: <20180710150358.GD1661@mail.hallyn.com> (raw)
In-Reply-To: <1530854701-7348-3-git-send-email-tyhicks@canonical.com>

Quoting Tyler Hicks (tyhicks@canonical.com):
> Fully initialize the aa_perms struct in profile_query_cb() to avoid the
> potential of using an uninitialized struct member's value in a response
> to a query from userspace.
> 
> Detected by CoverityScan CID#1415126 ("Uninitialized scalar variable")
> 
> Fixes: 4f3b3f2d79a4 ("apparmor: add profile permission query ability")
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

> ---
>  security/apparmor/apparmorfs.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 949dd8a48164..e09fe4d7307c 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -603,7 +603,7 @@ static const struct file_operations aa_fs_ns_revision_fops = {
>  static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  			     const char *match_str, size_t match_len)
>  {
> -	struct aa_perms tmp;
> +	struct aa_perms tmp = { };
>  	struct aa_dfa *dfa;
>  	unsigned int state = 0;
>  
> @@ -613,7 +613,6 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  		dfa = profile->file.dfa;
>  		state = aa_dfa_match_len(dfa, profile->file.start,
>  					 match_str + 1, match_len - 1);
> -		tmp = nullperms;
>  		if (state) {
>  			struct path_cond cond = { };
>  
> @@ -627,8 +626,6 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  					 match_str, match_len);
>  		if (state)
>  			aa_compute_perms(dfa, state, &tmp);
> -		else
> -			tmp = nullperms;
>  	}
>  	aa_apply_modes_to_perms(profile, &tmp);
>  	aa_perms_accum_raw(perms, &tmp);
> -- 
> 2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: serge@hallyn.com (Serge E. Hallyn)
To: linux-security-module@vger.kernel.org
Subject: [PATCH 2/2] apparmor: Fully initialize aa_perms struct when answering userspace query
Date: Tue, 10 Jul 2018 10:03:58 -0500	[thread overview]
Message-ID: <20180710150358.GD1661@mail.hallyn.com> (raw)
In-Reply-To: <1530854701-7348-3-git-send-email-tyhicks@canonical.com>

Quoting Tyler Hicks (tyhicks at canonical.com):
> Fully initialize the aa_perms struct in profile_query_cb() to avoid the
> potential of using an uninitialized struct member's value in a response
> to a query from userspace.
> 
> Detected by CoverityScan CID#1415126 ("Uninitialized scalar variable")
> 
> Fixes: 4f3b3f2d79a4 ("apparmor: add profile permission query ability")
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

> ---
>  security/apparmor/apparmorfs.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 949dd8a48164..e09fe4d7307c 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -603,7 +603,7 @@ static const struct file_operations aa_fs_ns_revision_fops = {
>  static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  			     const char *match_str, size_t match_len)
>  {
> -	struct aa_perms tmp;
> +	struct aa_perms tmp = { };
>  	struct aa_dfa *dfa;
>  	unsigned int state = 0;
>  
> @@ -613,7 +613,6 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  		dfa = profile->file.dfa;
>  		state = aa_dfa_match_len(dfa, profile->file.start,
>  					 match_str + 1, match_len - 1);
> -		tmp = nullperms;
>  		if (state) {
>  			struct path_cond cond = { };
>  
> @@ -627,8 +626,6 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
>  					 match_str, match_len);
>  		if (state)
>  			aa_compute_perms(dfa, state, &tmp);
> -		else
> -			tmp = nullperms;
>  	}
>  	aa_apply_modes_to_perms(profile, &tmp);
>  	aa_perms_accum_raw(perms, &tmp);
> -- 
> 2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-07-10 15:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-06  5:24 [PATCH 0/2] Fix AppArmor issues found through static analysis Tyler Hicks
2018-07-06  5:24 ` Tyler Hicks
2018-07-06  5:25 ` [PATCH 1/2] apparmor: Check buffer bounds when mapping permissions mask Tyler Hicks
2018-07-06  5:25   ` Tyler Hicks
2018-07-10 15:00   ` Serge E. Hallyn
2018-07-10 15:00     ` Serge E. Hallyn
2018-07-19 23:28   ` John Johansen
2018-07-19 23:28     ` John Johansen
2018-07-06  5:25 ` [PATCH 2/2] apparmor: Fully initialize aa_perms struct when answering userspace query Tyler Hicks
2018-07-06  5:25   ` Tyler Hicks
2018-07-10 15:03   ` Serge E. Hallyn [this message]
2018-07-10 15:03     ` Serge E. Hallyn
2018-07-19 23:28   ` John Johansen
2018-07-19 23:28     ` John Johansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180710150358.GD1661@mail.hallyn.com \
    --to=serge@hallyn.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=seth.arnold@canonical.com \
    --cc=tyhicks@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.