2018-08-23 16:25:54

by Arnd Bergmann

Subject: [PATCH] apparmor: remove unused label

After the corresponding 'goto' was removed, we get a warning
for the 'fail' label:

security/apparmor/policy_unpack.c: In function 'unpack_dfa':
security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label]

Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack")
Signed-off-by: Arnd Bergmann <[email protected]>
---
security/apparmor/policy_unpack.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 3647b5834ace..96d8cf68ce65 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)

return dfa;

-fail:
aa_put_dfa(dfa);
return ERR_PTR(-EPROTO);
}
--
2.18.0

2018-08-23 16:26:50

by Kees Cook

Subject: Re: [PATCH] apparmor: remove unused label

On Thu, Aug 23, 2018 at 7:09 AM, Arnd Bergmann <[email protected]> wrote:
> After the corresponding 'goto' was removed, we get a warning
> for the 'fail' label:
>
> security/apparmor/policy_unpack.c: In function 'unpack_dfa':
> security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label]
>
> Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack")
> Signed-off-by: Arnd Bergmann <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

-Kees

> ---
> security/apparmor/policy_unpack.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 3647b5834ace..96d8cf68ce65 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)
>
> return dfa;
>
> -fail:
> aa_put_dfa(dfa);
> return ERR_PTR(-EPROTO);
> }
> --
> 2.18.0
>



--
Kees Cook
Pixel Security

2018-08-23 16:27:07

by Tetsuo Handa

Subject: Re: [PATCH] apparmor: remove unused label

On 2018/08/23 23:21, Kees Cook wrote:
> On Thu, Aug 23, 2018 at 7:09 AM, Arnd Bergmann <[email protected]> wrote:
>> After the corresponding 'goto' was removed, we get a warning
>> for the 'fail' label:
>>
>> security/apparmor/policy_unpack.c: In function 'unpack_dfa':
>> security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label]
>>
>> Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>
> Reviewed-by: Kees Cook <[email protected]>
>
> -Kees
>
>> ---
>> security/apparmor/policy_unpack.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
>> index 3647b5834ace..96d8cf68ce65 100644
>> --- a/security/apparmor/policy_unpack.c
>> +++ b/security/apparmor/policy_unpack.c
>> @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)
>>
>> return dfa;
>>
>> -fail:
>> aa_put_dfa(dfa);
>> return ERR_PTR(-EPROTO);

If these lines are unreachable, please remove together...
And that is what Gustavo A. R. Silva reported before this patch?

>> }
>> --
>> 2.18.0
>>
>
>
>

2018-08-23 16:47:03

by Thiago Jung Bauermann

Subject: Re: [PATCH] apparmor: remove unused label

Arnd Bergmann <[email protected]> writes:

> After the corresponding 'goto' was removed, we get a warning
> for the 'fail' label:
>
> security/apparmor/policy_unpack.c: In function 'unpack_dfa':
> security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label]
>
> Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> security/apparmor/policy_unpack.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 3647b5834ace..96d8cf68ce65 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)
>
> return dfa;
>
> -fail:
> aa_put_dfa(dfa);
> return ERR_PTR(-EPROTO);
> }

Shouldn't the two lines after the label be removed as well? IIUC they're
unreachable now.

--
Thiago Jung Bauermann
IBM Linux Technology Center

2018-08-23 18:23:48

by John Johansen

Subject: Re: [PATCH] apparmor: remove unused label

On 08/23/2018 07:09 AM, Arnd Bergmann wrote:

thank you for the patch, but a fix for this issue was pushed to apparmor-next yesterday


> After the corresponding 'goto' was removed, we get a warning
> for the 'fail' label:
>
> security/apparmor/policy_unpack.c: In function 'unpack_dfa':
> security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label]
>
> Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> security/apparmor/policy_unpack.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 3647b5834ace..96d8cf68ce65 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)
>
> return dfa;
>
> -fail:
> aa_put_dfa(dfa);
> return ERR_PTR(-EPROTO);
> }
>

2018-08-23 21:31:55

by Arnd Bergmann

Subject: Re: [PATCH] apparmor: remove unused label

On Thu, Aug 23, 2018 at 8:21 PM John Johansen
<[email protected]> wrote:
>
> On 08/23/2018 07:09 AM, Arnd Bergmann wrote:
>
> thank you for the patch, but a fix for this issue was pushed to apparmor-next yesterday
>

Ok, good. As several people pointed out, my patch was also wrong, so that
saves me doing another one ;-)

Arnd