[PATCH] selinux: Mark array 'labeling_behaviors' as __maybe_unused

[PATCH] selinux: Mark array 'labeling_behaviors' as __maybe_unused

[Matthias Kaehlcke]

The array is only referenced in an ARRAY_SIZE() statement. Adding the
attribute fixes the following warning when building with clang:

security/selinux/hooks.c:338:20: error: variable 'labeling_behaviors'
    is not needed and will not be emitted
    [-Werror,-Wunneeded-internal-declaration]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 security/selinux/hooks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526d1f30..450ff9f3161c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -400,7 +400,7 @@ static void superblock_free_security(struct super_block *sb)
 
 /* The file system's label must be initialized prior to use. */
 
-static const char *labeling_behaviors[7] = {
+static const char * __maybe_unused labeling_behaviors[7] = {
 	"uses xattr",
 	"uses transition SIDs",
 	"uses task SIDs",
-- 
2.13.0.303.g4ebf302169-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] selinux: Mark array 'labeling_behaviors' as __maybe_unused

[Paul Moore]

On Thu, May 18, 2017 at 3:07 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> The array is only referenced in an ARRAY_SIZE() statement. Adding the
> attribute fixes the following warning when building with clang:
>
> security/selinux/hooks.c:338:20: error: variable 'labeling_behaviors'
>     is not needed and will not be emitted
>     [-Werror,-Wunneeded-internal-declaration]
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  security/selinux/hooks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

The fact that we only reference labeling_behaviors in one spot, and
even then we only use it as a parameter to ARRAY_SIZE(), makes me
believe we may be able to get rid of labeling_behaviors and use
SECURITY_FS_USE_MAX in its place.

Anyone working on any patches which make use of labeling_behaviors?

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526d1f30..450ff9f3161c 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -400,7 +400,7 @@ static void superblock_free_security(struct super_block *sb)
>
>  /* The file system's label must be initialized prior to use. */
>
> -static const char *labeling_behaviors[7] = {
> +static const char * __maybe_unused labeling_behaviors[7] = {
>         "uses xattr",
>         "uses transition SIDs",
>         "uses task SIDs",
> --
> 2.13.0.303.g4ebf302169-goog

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] selinux: Mark array 'labeling_behaviors' as __maybe_unused

[Stephen Smalley]

On Fri, 2017-05-19 at 11:09 -0400, Paul Moore wrote:
> On Thu, May 18, 2017 at 3:07 PM, Matthias Kaehlcke <mka@chromium.org>
> wrote:
> > The array is only referenced in an ARRAY_SIZE() statement. Adding
> > the
> > attribute fixes the following warning when building with clang:
> > 
> > security/selinux/hooks.c:338:20: error: variable
> > 'labeling_behaviors'
> > ????is not needed and will not be emitted
> > ????[-Werror,-Wunneeded-internal-declaration]
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > ?security/selinux/hooks.c | 2 +-
> > ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> The fact that we only reference labeling_behaviors in one spot, and
> even then we only use it as a parameter to ARRAY_SIZE(), makes me
> believe we may be able to get rid of labeling_behaviors and use
> SECURITY_FS_USE_MAX in its place.
> 
> Anyone working on any patches which make use of labeling_behaviors?

I think you could just remove both the array and the code that
referenced it; it only made sense before commit
2088d60e3b2f53d0c9590a0202eeff85b288b1eb.  We already check that the
policy doesn't contain any behavior > SECURITY_FS_USE_MAX during policy
load, so this cannot occur (modulo memory corruption), and it was only
there to make sure we didn't try to dereference off the end of the
array prior to the aforementioned commit.

> 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526d1f30..450ff9f3161c 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -400,7 +400,7 @@ static void superblock_free_security(struct
> > super_block *sb)
> > 
> > ?/* The file system's label must be initialized prior to use. */
> > 
> > -static const char *labeling_behaviors[7] = {
> > +static const char * __maybe_unused labeling_behaviors[7] = {
> > ????????"uses xattr",
> > ????????"uses transition SIDs",
> > ????????"uses task SIDs",
> > --
> > 2.13.0.303.g4ebf302169-goog
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] selinux: Mark array 'labeling_behaviors' as __maybe_unused

[Matthias Kaehlcke]

El Fri, May 19, 2017 at 11:45:19AM -0400 Stephen Smalley ha dit:

> On Fri, 2017-05-19 at 11:09 -0400, Paul Moore wrote:
> > On Thu, May 18, 2017 at 3:07 PM, Matthias Kaehlcke <mka@chromium.org>
> > wrote:
> > > The array is only referenced in an ARRAY_SIZE() statement. Adding
> > > the
> > > attribute fixes the following warning when building with clang:
> > > 
> > > security/selinux/hooks.c:338:20: error: variable
> > > 'labeling_behaviors'
> > > ????is not needed and will not be emitted
> > > ????[-Werror,-Wunneeded-internal-declaration]
> > > 
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > ---
> > > ?security/selinux/hooks.c | 2 +-
> > > ?1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > The fact that we only reference labeling_behaviors in one spot, and
> > even then we only use it as a parameter to ARRAY_SIZE(), makes me
> > believe we may be able to get rid of labeling_behaviors and use
> > SECURITY_FS_USE_MAX in its place.
> > 
> > Anyone working on any patches which make use of labeling_behaviors?
> 
> I think you could just remove both the array and the code that
> referenced it; it only made sense before commit
> 2088d60e3b2f53d0c9590a0202eeff85b288b1eb.  We already check that the
> policy doesn't contain any behavior > SECURITY_FS_USE_MAX during policy
> load, so this cannot occur (modulo memory corruption), and it was only
> there to make sure we didn't try to dereference off the end of the
> array prior to the aforementioned commit.

Thanks for your comments.

I will send out a patch that removes the array shortly.

Matthias
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html