Bumping this now that we're past the holidays :)
On Fri, Dec 21, 2018 at 11:25 AM Eric Chiang <ericchi...@google.com> wrote:
>
> Hey Seth,
>
> The proposed userland parser changes explicitly won't pass a
> zero-sized array. However, modifying the parser to pass zero-sized
> arrays causes kcalloc(0) to return ZERO_SIZE_PTR and the policy still
> works as if the array hadn't been passed at all.
>
> Note that other uses of unpack_array don't check for size == 0. If I'm
> missing an issue here (which is totally possible), we need to add
> checks to those calls too:
>
> $ ag -r -A5 '= unpack_array' security/apparmor/
> security/apparmor/policy_unpack.c
> 455: size = unpack_array(e, NULL);
> 456- /* currently 4 exec bits and entries 0-3 are reserved iupcx */
> 457- if (size > 16 - 4)
> 458- goto fail;
> 459- profile->file.trans.table = kcalloc(size, sizeof(char *),
> 460- GFP_KERNEL);
> --
> 523: size = unpack_array(e, NULL);
> 524- profile->xattr_count = size;
> 525- profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
> 526- if (!profile->xattrs)
> 527- goto fail;
> 528- for (i = 0; i < size; i++) {
> --
> 551: size = unpack_array(e, NULL);
> 552-
> 553- profile->secmark = kcalloc(size, sizeof(struct aa_secmark),
> 554- GFP_KERNEL);
> 555- if (!profile->secmark)
> 556- goto fail;
> --
> 600: size = unpack_array(e, NULL);
> 601- if (size > RLIM_NLIMITS)
> 602- goto fail;
> 603- for (i = 0; i < size; i++) {
> 604- u64 tmp2 = 0;
> 605- int a = aa_map_resource(i);
>
>
>
> On Thu, Dec 20, 2018 at 8:30 PM Seth Arnold <seth.arn...@canonical.com> wrote:
> >
> > On Thu, Dec 20, 2018 at 01:28:38PM -0800, Eric Chiang wrote:
> > > --- a/security/apparmor/policy_unpack.c
> > > +++ b/security/apparmor/policy_unpack.c
> > > @@ -535,6 +535,24 @@ static bool unpack_xattrs(struct aa_ext *e, struct
> > > aa_profile *profile)
> > > goto fail;
> > > }
> > >
> > > + if (unpack_nameX(e, AA_STRUCT, "xattr_keys")) {
> > > + int i, size;
> > > +
> > > + size = unpack_array(e, NULL);
> > > + profile->xattr_keys_count = size;
> > > + profile->xattr_keys = kcalloc(size, sizeof(char *),
> > > GFP_KERNEL);
> >
> > Do we need to worry about a zero-size array here?
> >
> > Thanks
> > --
> > AppArmor mailing list
> > AppArmor@lists.ubuntu.com
> > Modify settings or unsubscribe at:
> > https://lists.ubuntu.com/mailman/listinfo/apparmor
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
