[apparmor] GSoC review r26 and r27

[Kshitij Gupta]

Hello,

On Fri, Jan 30, 2015 at 4:37 PM, Christian Boltz <apparmor at cboltz.de> wrote:

> Hello,
>
> Am Samstag, 27. Juli 2013 schrieb John Johansen:
> > On 07/27/2013 10:02 AM, Christian Boltz wrote:
>
> (yes, those dates and the subject are correct ;-)
>
> Time travel...

> > @John: I'm still waiting for your answer about
> > >
> > >     # ix implies m, so we don't need to add m if ix is present
> >
> > so ignore this, as we are not doing this
> >
> > > I have some profiles that contain "mrix" (for example sbin.dhclient
> > > and usr.sbin.ntpd), so either the old logprof was buggy or the
> > > comment is wrong ;-)
> >
> > neither, it was actually a change in kernel behavior that affected
> > policy. It used to be that m was not needed for ix because of where
> > the tests where done.
> >
> > A change in that behavior happened 5 or 6 years ago.
> >
> > so at best the comment should have been changed as this rolled through
>
> So 18 months later, here's a patch that removes the outdated comment ;-)
>
>
> [ utils-drop-ix-m-comment.diff ]
>
> === modified file 'utils/aa-mergeprof'
> --- utils/aa-mergeprof  2014-10-16 21:35:06 +0000
> +++ utils/aa-mergeprof  2015-01-30 11:03:42 +0000
> @@ -434,14 +434,6 @@
>                          if not allow_mode & apparmor.aamode.AA_MAY_EXEC:
>                              mode |= apparmor.aa.str_to_mode('ix')
>
> -                    # m is not implied by ix
> -
> -                    ### If we get an mmap request, check if we already
> have it in allow_mode
> -                    ##if mode & AA_EXEC_MMAP:
> -                    ##    # ix implies m, so we don't need to add m if ix
> is present
> -                    ##    if contains(allow_mode, 'ix'):
> -                    ##        mode = mode - AA_EXEC_MMAP
> -
>                      if not mode:
>                          continue
>
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-12-24 15:54:57 +0000
> +++ utils/apparmor/aa.py        2015-01-30 11:04:05 +0000
> @@ -1702,14 +1702,6 @@
>                          if not allow_mode & apparmor.aamode.AA_MAY_EXEC:
>                              mode |= str_to_mode('ix')
>
> -                    # m is not implied by ix
> -
> -                    ### If we get an mmap request, check if we already
> have it in allow_mode
> -                    ##if mode & AA_EXEC_MMAP:
> -                    ##    # ix implies m, so we don't need to add m if ix
> is present
> -                    ##    if contains(allow_mode, 'ix'):
> -                    ##        mode = mode - AA_EXEC_MMAP
> -
>                      if not mode:
>                          continue
>
>
> Reminds me of the code duplication and a real need for cleanup. I'll
probably pick this up soon.

Thanks for the patch (from past).

Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>.

Regards,

Kshitij Gupta


> Regards,
>
> Christian Boltz
> --
> SYNOPSIS
>        glimpse - [almost all letters] pattern
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150130/75b9f71b/attachment.html>