[2/5] UBUNTU: SAUCE: AppArmor: Fix Oops when in apparmor_bprm_set_creds

BugLink: http://bugs.launchpad.net/bugs/437258

SRU Justification: This can cause an oops at 000068.  This will happen to
all processes confined or unconfined when name resolution fails at exec.
This can happen in a couple different cases, applications like psxe, and mugen
munge the process during their decrompress and set up links so that a valid
name does not exist.  The other way that this can happen is executing code
from a path that has been lazily unmounted.  This can occur with nfs and
automounters, or any mount point that gets unmounted with lazy unmount allowed.


If name resolution fails due on exec and a profile is not defined
then AppArmor will cause an oops due to a broken conditional leading to
dereferencing a profile pointer that is null.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 ubuntu/apparmor/domain.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Looks right. Either profiles is NULL or the flags are checked.

John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/437258
> 
> SRU Justification: This can cause an oops at 000068.  This will happen to
> all processes confined or unconfined when name resolution fails at exec.
> This can happen in a couple different cases, applications like psxe, and mugen
> munge the process during their decrompress and set up links so that a valid
> name does not exist.  The other way that this can happen is executing code
> from a path that has been lazily unmounted.  This can occur with nfs and
> automounters, or any mount point that gets unmounted with lazy unmount allowed.
> 
> 
> If name resolution fails due on exec and a profile is not defined
> then AppArmor will cause an oops due to a broken conditional leading to
> dereferencing a profile pointer that is null.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>

Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  ubuntu/apparmor/domain.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/ubuntu/apparmor/domain.c b/ubuntu/apparmor/domain.c
> index aa25be2..128e527 100644
> --- a/ubuntu/apparmor/domain.c
> +++ b/ubuntu/apparmor/domain.c
> @@ -248,7 +248,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>  	sa.base.error = aa_get_name(&bprm->file->f_path, 0, &buffer,
>  				    (char **) &sa.name);
>  	if (sa.base.error) {
> -		if (profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
> +		if (!profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
>  			sa.base.error = 0;
>  		sa.base.info = "Exec failed name resolution";
>  		sa.name = bprm->filename;

On Tue, Nov 10, 2009 at 10:29:10AM -0800, John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/437258
> 
> SRU Justification: This can cause an oops at 000068.  This will happen to
> all processes confined or unconfined when name resolution fails at exec.
> This can happen in a couple different cases, applications like psxe, and mugen
> munge the process during their decrompress and set up links so that a valid
> name does not exist.  The other way that this can happen is executing code
> from a path that has been lazily unmounted.  This can occur with nfs and
> automounters, or any mount point that gets unmounted with lazy unmount allowed.
> 
> 
> If name resolution fails due on exec and a profile is not defined
> then AppArmor will cause an oops due to a broken conditional leading to
> dereferencing a profile pointer that is null.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> ---
>  ubuntu/apparmor/domain.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/ubuntu/apparmor/domain.c b/ubuntu/apparmor/domain.c
> index aa25be2..128e527 100644
> --- a/ubuntu/apparmor/domain.c
> +++ b/ubuntu/apparmor/domain.c
> @@ -248,7 +248,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>  	sa.base.error = aa_get_name(&bprm->file->f_path, 0, &buffer,
>  				    (char **) &sa.name);
>  	if (sa.base.error) {
> -		if (profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
> +		if (!profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
>  			sa.base.error = 0;
>  		sa.base.info = "Exec failed name resolution";
>  		sa.name = bprm->filename;

If profile can be NULL then we would OOPS before.  I am assuming we do
not care about the error here, in that case.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw

Applied

Andy Whitcroft wrote:
> On Tue, Nov 10, 2009 at 10:29:10AM -0800, John Johansen wrote:
>> BugLink: http://bugs.launchpad.net/bugs/437258
>>
>> SRU Justification: This can cause an oops at 000068.  This will happen to
>> all processes confined or unconfined when name resolution fails at exec.
>> This can happen in a couple different cases, applications like psxe, and mugen
>> munge the process during their decrompress and set up links so that a valid
>> name does not exist.  The other way that this can happen is executing code
>> from a path that has been lazily unmounted.  This can occur with nfs and
>> automounters, or any mount point that gets unmounted with lazy unmount allowed.
>>
>>
>> If name resolution fails due on exec and a profile is not defined
>> then AppArmor will cause an oops due to a broken conditional leading to
>> dereferencing a profile pointer that is null.
>>
>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>> ---
>>  ubuntu/apparmor/domain.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/ubuntu/apparmor/domain.c b/ubuntu/apparmor/domain.c
>> index aa25be2..128e527 100644
>> --- a/ubuntu/apparmor/domain.c
>> +++ b/ubuntu/apparmor/domain.c
>> @@ -248,7 +248,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>>  	sa.base.error = aa_get_name(&bprm->file->f_path, 0, &buffer,
>>  				    (char **) &sa.name);
>>  	if (sa.base.error) {
>> -		if (profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
>> +		if (!profile || profile->flags & PFLAG_IX_ON_NAME_ERROR)
>>  			sa.base.error = 0;
>>  		sa.base.info = "Exec failed name resolution";
>>  		sa.name = bprm->filename;
> 
> If profile can be NULL then we would OOPS before.  I am assuming we do
> not care about the error here, in that case.
> 
Right profile could be NULL so we would OOPS before.  We do care about the error it gets logged, and in the enforcing (profile) case the execution will be failed unless the special IX_ON_NAME_ERROR is set, which causes the execution to inherit the current confinement.  In the unconfined case AppArmor doesn't fail the execution because it is supposed to have no effect beyond what DAC is enforcing.

jj