linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] apparmor: Parse secmark policy
@ 2018-10-10 10:42 Dan Carpenter
  2018-10-10 16:04 ` Matthew Garrett
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2018-10-10 10:42 UTC (permalink / raw)
  To: mjg59; +Cc: linux-security-module

Hello Matthew Garrett,

The patch 9caafbe2b4cf: "apparmor: Parse secmark policy" from May 24,
2018, leads to the following static checker warning:

	security/apparmor/policy_unpack.c:580 unpack_secmark()
	warn: should '(struct aa_profile)->secmark' be freed with kzfree()'

security/apparmor/policy_unpack.c
   558                  profile->secmark_count = size;
   559  
   560                  for (i = 0; i < size; i++) {
   561                          if (!unpack_u8(e, &profile->secmark[i].audit, NULL))
   562                                  goto fail;
   563                          if (!unpack_u8(e, &profile->secmark[i].deny, NULL))
   564                                  goto fail;
   565                          if (!unpack_strdup(e, &profile->secmark[i].label, NULL))
   566                                  goto fail;
   567                  }
   568                  if (!unpack_nameX(e, AA_ARRAYEND, NULL))
   569                          goto fail;
   570                  if (!unpack_nameX(e, AA_STRUCTEND, NULL))
   571                          goto fail;
   572          }
   573  
   574          return 1;
   575  
   576  fail:
   577          if (profile->secmark) {
   578                  for (i = 0; i < size; i++)
   579                          kfree(profile->secmark[i].label);
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^
   580                  kfree(profile->secmark);
                              ^^^^^^^^^^^^^^^^
Smatch thinks that these need to be zeroed for security.  I don't know
the details though.

   581                  profile->secmark_count = 0;
   582          }
   583  
   584          e->pos = pos;
   585          return 0;
   586  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] apparmor: Parse secmark policy
  2018-10-10 10:42 [bug report] apparmor: Parse secmark policy Dan Carpenter
@ 2018-10-10 16:04 ` Matthew Garrett
  0 siblings, 0 replies; 2+ messages in thread
From: Matthew Garrett @ 2018-10-10 16:04 UTC (permalink / raw)
  To: dan.carpenter; +Cc: LSM List

On Wed, Oct 10, 2018 at 3:42 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Smatch thinks that these need to be zeroed for security.  I don't know
> the details though.

I don't think these are secret.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-10-10 16:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-10 10:42 [bug report] apparmor: Parse secmark policy Dan Carpenter
2018-10-10 16:04 ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).