diff mbox series

[v3,4/7] selinux: make left shifts well defined

Message ID 20230807171143.208481-3-cgzones@googlemail.com
State New
Headers
Series [v3,1/7] selinux: avoid implicit conversions in avtab code |

Commit Message

Christian Göttsche Aug. 7, 2023, 5:11 p.m. UTC 
  The loops upper bound represent the number of permissions used (for the
current class or in general).  The limit for this is 32, thus we might
left shift of one less, 31.  Shifting a base of 1 results in undefined
behavior; use (u32)1 as base.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3: split from parent commit and apply cast to correct shift operand
---
 security/selinux/ss/services.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Paul Moore Aug. 9, 2023, 11:07 p.m. UTC | #1 
On Aug  7, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> The loops upper bound represent the number of permissions used (for the
> current class or in general).  The limit for this is 32, thus we might
> left shift of one less, 31.  Shifting a base of 1 results in undefined
> behavior; use (u32)1 as base.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v3: split from parent commit and apply cast to correct shift operand
> ---
>  security/selinux/ss/services.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com
diff mbox series

Patch

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index dacec2ebdcd7..1eeffc66ea7d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -207,22 +207,22 @@  static void map_decision(struct selinux_map *map,
 
 		for (i = 0, result = 0; i < n; i++) {
 			if (avd->allowed & mapping->perms[i])
-				result |= 1<<i;
+				result |= (u32)1<<i;
 			if (allow_unknown && !mapping->perms[i])
-				result |= 1<<i;
+				result |= (u32)1<<i;
 		}
 		avd->allowed = result;
 
 		for (i = 0, result = 0; i < n; i++)
 			if (avd->auditallow & mapping->perms[i])
-				result |= 1<<i;
+				result |= (u32)1<<i;
 		avd->auditallow = result;
 
 		for (i = 0, result = 0; i < n; i++) {
 			if (avd->auditdeny & mapping->perms[i])
-				result |= 1<<i;
+				result |= (u32)1<<i;
 			if (!allow_unknown && !mapping->perms[i])
-				result |= 1<<i;
+				result |= (u32)1<<i;
 		}
 		/*
 		 * In case the kernel has a bug and requests a permission
@@ -230,7 +230,7 @@  static void map_decision(struct selinux_map *map,
 		 * should audit that denial
 		 */
 		for (; i < (sizeof(u32)*8); i++)
-			result |= 1<<i;
+			result |= (u32)1<<i;
 		avd->auditdeny = result;
 	}
 }