diff mbox

selinux: Mark array 'labeling_behaviors' as __maybe_unused

Message ID 20170518190759.28560-1-mka@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Matthias Kaehlcke May 18, 2017, 7:07 p.m. UTC
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(-)

Comments

Paul Moore May 19, 2017, 3:09 p.m. UTC | #1
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
Stephen Smalley May 19, 2017, 3:45 p.m. UTC | #2
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Kaehlcke May 19, 2017, 4:52 p.m. UTC | #3
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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",