On Thu, 2012-06-07 at 15:35 +0200, intrigeri@debian.org wrote:
> Package: linux-2.6
> Severity: normal
> Version: 3.2.19-1
> Tags: patch
> X-Debbugs-CC: john.johansen@canonical.com, kees@debian.org, micah@riseup.net
>
> Hi,
>
> the AppArmor compatibility patch applied to fix #661151
> totally breaks AppArmor support; this is a regression.
> Details: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=661151#83
>
> Applying the "AppArmor: compatibility patch for v5 network controll"
> on top of the already applied one fixes the problem for me.
> I did test with the patch that can be found there:
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-precise.git;a=commit;h=d253e5fb4a6b552e7cd2a3c80934ab4f92faec97
>
> Please consider applying this network compatibility patch,
> not only to fix this regression,
> but also to get us working network mediation.
So these independent patches really aren't... or else userland gets
confused if we apply just the one.
Looking at the network controller patch:
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
[...]
> @@ -621,6 +622,104 @@ static int apparmor_task_setrlimit(struct task_struct *task,
> return error;
> }
>
> +static int apparmor_socket_create(int family, int type, int protocol, int kern)
> +{
> + struct aa_profile *profile;
> + int error = 0;
> +
> + if (kern)
> + return 0;
If we don't want to restrict sockets used by the kernel, don't we need
to store the kern flag for later use by aa_revalidate_sk()?
> + profile = __aa_current_profile();
> + if (!unconfined(profile))
> + error = aa_net_perm(OP_CREATE, profile, family, type, protocol,
> + NULL);
> + return error;
> +}
[...]
> --- /dev/null
> +++ b/security/apparmor/net.c
[...]
> +static int audit_net(struct aa_profile *profile, int op, u16 family, int type,
> + int protocol, struct sock *sk, int error)
> +{
[...]
> + } else {
> + u16 quiet_mask = profile->net.quiet[sa.u.net.family];
> + u16 kill_mask = 0;
> + u16 denied = (1 << sa.aad.net.type) & ~quiet_mask;
> +
> + if (denied & kill_mask)
> + audit_type = AUDIT_APPARMOR_KILL;
> +
> + if ((denied & quiet_mask) &&
Since denied has already been masked with ~quiet_mask, this condition
can never be true.
> + AUDIT_MODE(profile) != AUDIT_NOQUIET &&
> + AUDIT_MODE(profile) != AUDIT_ALL)
> + return COMPLAIN_MODE(profile) ? 0 : sa.aad.error;
> + }
> +
> + return aa_audit(audit_type, profile, GFP_KERNEL, &sa, audit_cb);
> +}
[...]
> +/**
> + * aa_revalidate_sk - Revalidate access to a sock
> + * @op: operation being checked
> + * @sk: sock being revalidated (NOT NULL)
> + *
> + * Returns: %0 else error if permission denied
> + */
> +int aa_revalidate_sk(int op, struct sock *sk)
> +{
> + struct aa_profile *profile;
> + int error = 0;
> +
> + /* aa_revalidate_sk should not be called from interrupt context
> + * don't mediate these calls as they are not task related
> + */
> + if (in_interrupt())
> + return 0;
I wonder why this is being checked at all.
> + profile = __aa_current_profile();
> + if (!unconfined(profile))
> + error = aa_net_perm(op, profile, sk->sk_family, sk->sk_type,
> + sk->sk_protocol, sk);
> +
> + return error;
> +}
[...]
Ben.
--
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.
Attachment:
signature.asc
Description: This is a digitally signed message part