[PATCH] apparmor: Add empty statement between label and declaration in profile_transition(()

[PATCH] apparmor: Add empty statement between label and declaration in profile_transition(()

[Nathan Chancellor]

Clang 18 and newer warns (or errors with CONFIG_WERROR=y):

  security/apparmor/domain.c:695:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
    695 |                 struct aa_profile *new_profile = NULL;
        |                 ^

With Clang 17 and older, this is just an unconditional hard error:

  security/apparmor/domain.c:695:3: error: expected expression
    695 |                 struct aa_profile *new_profile = NULL;
        |                 ^
  security/apparmor/domain.c:697:3: error: use of undeclared identifier 'new_profile'
    697 |                 new_profile = aa_new_learning_profile(profile, false, name,
        |                 ^
  security/apparmor/domain.c:699:8: error: use of undeclared identifier 'new_profile'
    699 |                 if (!new_profile) {
        |                      ^
  security/apparmor/domain.c:704:11: error: use of undeclared identifier 'new_profile'
    704 |                         new = &new_profile->label;
        |                                ^

Add a semicolon directly after the label to create an empty statement,
which keeps the original intent of the code while clearing up the
warning/error on all clang versions.

Fixes: ee650b3820f3 ("apparmor: properly handle cx/px lookup failure for complain")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202411101808.AI8YG6cs-lkp@intel.com/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 security/apparmor/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 602d7a1bb44823a9b81e34d270b03c5f3aff3a34..eb0f222aa29442686b0a6751001c879f5b366c59 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -691,7 +691,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred,
 			error = -EACCES;
 		}
 	} else if (COMPLAIN_MODE(profile)) {
-create_learning_profile:
+create_learning_profile:;
 		/* no exec permission - learning mode */
 		struct aa_profile *new_profile = NULL;
 

---
base-commit: 8c4f7960ae8a7a03a43f814e4af471b8e6ea3391
change-id: 20241111-apparmor-fix-label-declaration-warning-fcd24ce2d447

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>

Re: [PATCH] apparmor: Add empty statement between label and declaration in profile_transition(()

[John Johansen]

On 11/11/24 06:49, Nathan Chancellor wrote:
> Clang 18 and newer warns (or errors with CONFIG_WERROR=y):
> 
>    security/apparmor/domain.c:695:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
>      695 |                 struct aa_profile *new_profile = NULL;
>          |                 ^
> 
> With Clang 17 and older, this is just an unconditional hard error:
> 
>    security/apparmor/domain.c:695:3: error: expected expression
>      695 |                 struct aa_profile *new_profile = NULL;
>          |                 ^
>    security/apparmor/domain.c:697:3: error: use of undeclared identifier 'new_profile'
>      697 |                 new_profile = aa_new_learning_profile(profile, false, name,
>          |                 ^
>    security/apparmor/domain.c:699:8: error: use of undeclared identifier 'new_profile'
>      699 |                 if (!new_profile) {
>          |                      ^
>    security/apparmor/domain.c:704:11: error: use of undeclared identifier 'new_profile'
>      704 |                         new = &new_profile->label;
>          |                                ^
> 
> Add a semicolon directly after the label to create an empty statement,
> which keeps the original intent of the code while clearing up the
> warning/error on all clang versions.
> 
> Fixes: ee650b3820f3 ("apparmor: properly handle cx/px lookup failure for complain")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202411101808.AI8YG6cs-lkp@intel.com/
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

thanks for the patch, but I went with an alternate version, that I did last Sunday
(sorry I guess I forgot to push the tree). Since I hadn't pushed the tree I did
consider replacing my patch with it but in the end decided to not go with
the C99 variable declaration, moving the var to the top of the outer block (what
my Sunday patch did).

The reason being that while I don't think the style guideline forbid them, I end
up getting patches for them anyways, as some compiler flag sets will warn about
them.




> ---
>   security/apparmor/domain.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 602d7a1bb44823a9b81e34d270b03c5f3aff3a34..eb0f222aa29442686b0a6751001c879f5b366c59 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -691,7 +691,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred,
>   			error = -EACCES;
>   		}
>   	} else if (COMPLAIN_MODE(profile)) {
> -create_learning_profile:
> +create_learning_profile:;
>   		/* no exec permission - learning mode */
>   		struct aa_profile *new_profile = NULL;
>   
> 
> ---
> base-commit: 8c4f7960ae8a7a03a43f814e4af471b8e6ea3391
> change-id: 20241111-apparmor-fix-label-declaration-warning-fcd24ce2d447
> 
> Best regards,

Re: [PATCH] apparmor: Add empty statement between label and declaration in profile_transition(()

[Nathan Chancellor]

On Fri, Nov 15, 2024 at 06:17:11PM -0800, John Johansen wrote:
> thanks for the patch, but I went with an alternate version, that I did last Sunday
> (sorry I guess I forgot to push the tree). Since I hadn't pushed the tree I did
> consider replacing my patch with it but in the end decided to not go with
> the C99 variable declaration, moving the var to the top of the outer block (what
> my Sunday patch did).

Thanks for the information, I considered doing the same thing but went
with the minimal fix just for ease of acceptance. Appreciate you getting
this resolved now.

> The reason being that while I don't think the style guideline forbid them, I end
> up getting patches for them anyways, as some compiler flag sets will warn about
> them.

For what it's worth, commit b5ec6fd286df ("kbuild: Drop
-Wdeclaration-after-statement") dropped the option that warns about
that, so you should not have to worry about that anymore.

Cheers,
Nathan