diff mbox series

libselinux: Fix integer comparison issues when compiling for 32-bit

Message ID 20240701182732.85548-1-jwcart2@gmail.com (mailing list archive)
State Accepted
Delegated to: Petr Lautrbach
Headers show
Series libselinux: Fix integer comparison issues when compiling for 32-bit | expand

Commit Message

James Carter July 1, 2024, 6:27 p.m. UTC
Trying to compile libselinux for 32-bit produces the following error:

selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
 1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
      |                               ^~

Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
cast both as uint32_t for the comparison.

Reported-by: Daniel Schepler
Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libselinux/src/selinux_restorecon.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Christian Göttsche July 7, 2024, 6:59 a.m. UTC | #1
On Mon, 1 Jul 2024 at 20:27, James Carter <jwcart2@gmail.com> wrote:
>
> Trying to compile libselinux for 32-bit produces the following error:
>
> selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
>  1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
>       |                               ^~
>
> Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
> cast both as uint32_t for the comparison.

LGTM.
Reviewed-by: Christian Göttsche <cgzones@googlemail.com>

> Reported-by: Daniel Schepler
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>  libselinux/src/selinux_restorecon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index acb729c8..bc6ed935 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -1191,8 +1191,8 @@ static int selinux_restorecon_common(const char *pathname_orig,
>         }
>
>         /* Skip digest on in-memory filesystems and /sys */
> -       if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
> -           state.sfsb.f_type == SYSFS_MAGIC)
> +       if ((uint32_t)state.sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
> +               state.sfsb.f_type == TMPFS_MAGIC || state.sfsb.f_type == SYSFS_MAGIC)
>                 state.setrestorecondigest = false;
>
>         if (state.flags.set_xdev)
> @@ -1490,7 +1490,7 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
>
>         if (!recurse) {
>                 if (statfs(pathname, &sfsb) == 0) {
> -                       if (sfsb.f_type == RAMFS_MAGIC ||
> +                       if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
>                             sfsb.f_type == TMPFS_MAGIC)
>                                 return 0;
>                 }
> @@ -1525,7 +1525,7 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
>                         continue;
>                 case FTS_D:
>                         if (statfs(ftsent->fts_path, &sfsb) == 0) {
> -                               if (sfsb.f_type == RAMFS_MAGIC ||
> +                               if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
>                                     sfsb.f_type == TMPFS_MAGIC)
>                                         continue;
>                         }
> --
> 2.45.2
>
>
Stephen Smalley July 30, 2024, 11:58 a.m. UTC | #2
On Sun, Jul 7, 2024 at 3:00 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Mon, 1 Jul 2024 at 20:27, James Carter <jwcart2@gmail.com> wrote:
> >
> > Trying to compile libselinux for 32-bit produces the following error:
> >
> > selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
> >  1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
> >       |                               ^~
> >
> > Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
> > cast both as uint32_t for the comparison.
>
> LGTM.
> Reviewed-by: Christian Göttsche <cgzones@googlemail.com>

Do you have a reproducer for this? Building with -m32 didn't seem to
trigger the error for me on 64-bit Fedora.
James Carter July 30, 2024, 1:42 p.m. UTC | #3
On Tue, Jul 30, 2024 at 7:58 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Sun, Jul 7, 2024 at 3:00 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > On Mon, 1 Jul 2024 at 20:27, James Carter <jwcart2@gmail.com> wrote:
> > >
> > > Trying to compile libselinux for 32-bit produces the following error:
> > >
> > > selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
> > >  1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
> > >       |                               ^~
> > >
> > > Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
> > > cast both as uint32_t for the comparison.
> >
> > LGTM.
> > Reviewed-by: Christian Göttsche <cgzones@googlemail.com>
>
> Do you have a reproducer for this? Building with -m32 didn't seem to
> trigger the error for me on 64-bit Fedora.

Building the SELinux userspace with:
DESTDIR=~/local make clean distclean; clear; CPPFLAGS+="-g -m32"
DESTDIR=~/local make install install-pywrap
gives me the error.

Jim
Stephen Smalley July 30, 2024, 3:01 p.m. UTC | #4
On Mon, Jul 1, 2024 at 2:27 PM James Carter <jwcart2@gmail.com> wrote:
>
> Trying to compile libselinux for 32-bit produces the following error:
>
> selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
>  1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
>       |                               ^~
>
> Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
> cast both as uint32_t for the comparison.
>
> Reported-by: Daniel Schepler
> Signed-off-by: James Carter <jwcart2@gmail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  libselinux/src/selinux_restorecon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index acb729c8..bc6ed935 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -1191,8 +1191,8 @@ static int selinux_restorecon_common(const char *pathname_orig,
>         }
>
>         /* Skip digest on in-memory filesystems and /sys */
> -       if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
> -           state.sfsb.f_type == SYSFS_MAGIC)
> +       if ((uint32_t)state.sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
> +               state.sfsb.f_type == TMPFS_MAGIC || state.sfsb.f_type == SYSFS_MAGIC)
>                 state.setrestorecondigest = false;
>
>         if (state.flags.set_xdev)
> @@ -1490,7 +1490,7 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
>
>         if (!recurse) {
>                 if (statfs(pathname, &sfsb) == 0) {
> -                       if (sfsb.f_type == RAMFS_MAGIC ||
> +                       if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
>                             sfsb.f_type == TMPFS_MAGIC)
>                                 return 0;
>                 }
> @@ -1525,7 +1525,7 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
>                         continue;
>                 case FTS_D:
>                         if (statfs(ftsent->fts_path, &sfsb) == 0) {
> -                               if (sfsb.f_type == RAMFS_MAGIC ||
> +                               if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
>                                     sfsb.f_type == TMPFS_MAGIC)
>                                         continue;
>                         }
> --
> 2.45.2
>
>
Stephen Smalley July 30, 2024, 5:19 p.m. UTC | #5
On Tue, Jul 30, 2024 at 11:01 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jul 1, 2024 at 2:27 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > Trying to compile libselinux for 32-bit produces the following error:
> >
> > selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
> >  1194 |         if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
> >       |                               ^~
> >
> > Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
> > cast both as uint32_t for the comparison.
> >
> > Reported-by: Daniel Schepler
> > Signed-off-by: James Carter <jwcart2@gmail.com>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Applied.
diff mbox series

Patch

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index acb729c8..bc6ed935 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -1191,8 +1191,8 @@  static int selinux_restorecon_common(const char *pathname_orig,
 	}
 
 	/* Skip digest on in-memory filesystems and /sys */
-	if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
-	    state.sfsb.f_type == SYSFS_MAGIC)
+	if ((uint32_t)state.sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
+		state.sfsb.f_type == TMPFS_MAGIC || state.sfsb.f_type == SYSFS_MAGIC)
 		state.setrestorecondigest = false;
 
 	if (state.flags.set_xdev)
@@ -1490,7 +1490,7 @@  int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
 
 	if (!recurse) {
 		if (statfs(pathname, &sfsb) == 0) {
-			if (sfsb.f_type == RAMFS_MAGIC ||
+			if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
 			    sfsb.f_type == TMPFS_MAGIC)
 				return 0;
 		}
@@ -1525,7 +1525,7 @@  int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags,
 			continue;
 		case FTS_D:
 			if (statfs(ftsent->fts_path, &sfsb) == 0) {
-				if (sfsb.f_type == RAMFS_MAGIC ||
+				if ((uint32_t)sfsb.f_type == (uint32_t)RAMFS_MAGIC ||
 				    sfsb.f_type == TMPFS_MAGIC)
 					continue;
 			}