All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] checkpolicy/policy_scan.y
@ 2003-10-22 17:56 Joerg Hoh
  2003-10-22 18:33 ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: Joerg Hoh @ 2003-10-22 17:56 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

Hi
                                                                                                                                                             
The following patch fixes some warnings in policy_parse.y (add -W to the
CFLAGS to display this warnings):
                                                                                                                                                             
policy_parse.y: In function `set_types':
policy_parse.y:1666: warning: comparison between signed and unsigned
policy_parse.y:1674: warning: comparison between signed and unsigned
policy_parse.y:1694: warning: comparison between signed and unsigned
...
policy_parse.y: In function `define_constraint':
policy_parse.y:2543: warning: comparison between signed and unsigned
policy_parse.y: In function `define_cexpr':
policy_parse.y:2644: warning: cast from pointer to integer of different size
policy_parse.y:2645: warning: cast from pointer to integer of different size
policy_parse.y:2648: warning: cast from pointer to integer of different size
policy_parse.y:2649: warning: cast from pointer to integer of different size
policy_parse.y: In function `set_user_roles':
policy_parse.y:2711: warning: comparison between signed and unsigned
policy_parse.y:2719: warning: comparison between signed and unsigned
policy_parse.y:2738: warning: comparison between signed and unsigned
                                                                                                                                                             
Joerg


[-- Attachment #2: policy_scan --]
[-- Type: text/plain, Size: 6270 bytes --]

12a13
> #include <stdint.h>
51c52
< static constraint_expr_t *define_cexpr(__u32 expr_type, void *arg1, void* arg2);
---
> static uintptr_t define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2);
57,58c58,59
< static int define_fs_context(int major, int minor);
< static int define_port_context(int low, int high);
---
> static int define_fs_context(uintptr_t major, uintptr_t minor);
> static int define_port_context(uintptr_t low, uintptr_t high);
60c61
< static int define_node_context(int addr, int mask);
---
> static int define_node_context(uintptr_t addr, uintptr_t mask);
64c65
< 	int val;
---
> 	uintptr_t val;
68,69c69,70
< %type <ptr> role_def roles cexpr cexpr_prim op roleop
< %type <val> ipv4_addr_def number
---
> %type <ptr> role_def roles
> %type <val> ipv4_addr_def number cexpr cexpr_prim op roleop
325c326
< 			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, $2);
---
> 			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, $2);
328c329
< 			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, $2);
---
> 			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
331c332
< 			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_TYPE, $2);
---
> 			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_TYPE, $2);
334c335
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_USER, $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_USER, $2);
337c338
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_USER | CEXPR_TARGET), $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_USER | CEXPR_TARGET), $2);
340c341
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, $2);
343c344
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), $2);
346c347
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, $2);
349c350
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), $2);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), $2);
352c353
< 			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, (void*)CEXPR_EQ);
---
> 			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, CEXPR_EQ);
355c356
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, (void*)CEXPR_EQ);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, CEXPR_EQ);
358c359
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), (void*)CEXPR_EQ);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), CEXPR_EQ);
361c362
< 			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, (void*)$2);
---
> 			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
364c365
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, (void*)CEXPR_EQ);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, CEXPR_EQ);
367c368
< 			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), (void*)CEXPR_EQ);
---
> 			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), CEXPR_EQ);
371c372
< 			{ $$ = (void*)CEXPR_EQ; }
---
> 			{ $$ = CEXPR_EQ; }
373c374
< 			{ $$ = (void*)CEXPR_NEQ; }
---
> 			{ $$ = CEXPR_NEQ; }
378c379
< 			{ $$ = (void*)CEXPR_DOM; }
---
> 			{ $$ = CEXPR_DOM; }
380c381
< 			{ $$ = (void*)CEXPR_DOMBY; }
---
> 			{ $$ = CEXPR_DOMBY; }
382c383
< 			{ $$ = (void*)CEXPR_INCOMP; }
---
> 			{ $$ = CEXPR_INCOMP; }
1662c1663
< 	int i;
---
> 	unsigned int i;
1718c1719,1720
< 	int ret, i, j, k;
---
> 	int ret;
> 	unsigned int i, j, k;
1867c1869,1870
< 	int i, rc;
---
> 	unsigned int i;
> 	int rc;
1894c1897
< static int te_avtab_helper(int which, int stype, int ttype, 
---
> static int te_avtab_helper(int which, unsigned int stype, unsigned int ttype, 
1900c1903,1904
< 	int ret, k;
---
> 	int ret;
> 	unsigned int k;
1961c1965,1966
< 	int i, j, hiclass, self = 0;
---
> 	unsigned int i, j, hiclass;
> 	int self = 0;
2024c2029
< 				avp[i] = ~0;
---
> 				avp[i] = (access_vector_t) -1;
2222c2227,2228
< 	int i, ret;
---
> 	unsigned int i;
> 	int ret;
2277c2283
< 	int i;
---
> 	unsigned int i;
2320c2326
< 	int i, j;
---
> 	unsigned int i, j;
2401c2407
< 	int i, j;
---
> 	unsigned int i, j;
2465c2471,2472
< 	int i, depth;
---
> 	unsigned int i;
> 	int depth;
2575,2576c2582,2583
< static constraint_expr_t *
<  define_cexpr(__u32 expr_type, void* arg1, void* arg2)
---
> static uintptr_t
>  define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2)
2589c2596
< 		return (constraint_expr_t *)1; /* any non-NULL value */
---
> 		return 1; /* any non-NULL value */
2595c2602
< 		return NULL;
---
> 		return 0;
2611c2618
< 			return NULL;
---
> 			return 0;
2614c2621
< 		return (struct constraint_expr *) arg1;
---
> 		return arg1;
2626c2633
< 			return NULL;
---
> 			return 0;
2639c2646
< 			return NULL;
---
> 			return 0;
2642c2649
< 		return (struct constraint_expr *) arg1;
---
> 		return arg1;
2644,2646c2651,2653
< 		expr->attr = (__u32)arg1;
< 		expr->op = (__u32)arg2;
< 		return expr;
---
> 		expr->attr = arg1;
> 		expr->op = arg2;
> 		return (uintptr_t) expr;
2648,2649c2655,2656
< 		expr->attr = (__u32)arg1;
< 		expr->op = (__u32)arg2;
---
> 		expr->attr = arg1;
> 		expr->op = arg2;
2658c2665
< 					return NULL;
---
> 					return 0;
2668c2675
< 					return NULL;
---
> 					return 0;
2674c2681
< 					return NULL;
---
> 					return 0;
2680c2687
< 				return NULL;
---
> 				return 0;
2686c2693
< 				return NULL;
---
> 				return 0;
2690c2697
< 		return expr;
---
> 		return (uintptr_t) expr;
2694c2701
< 		return NULL;
---
> 		return 0;
2699c2706
< 	return NULL;
---
> 	return 0;
2707c2714
< 	int i;
---
> 	unsigned int i;
3140c3147
< static int define_fs_context(int major, int minor)
---
> static int define_fs_context(uintptr_t major, uintptr_t minor)
3157c3164
< 	newc->u.name = (char *) malloc(6);
---
> 	newc->u.name = malloc((size_t) 6);
3163c3170,3171
< 	sprintf(newc->u.name, "%02x:%02x", major, minor);
---
> 	sprintf(newc->u.name, "%02lx:%02lx", (unsigned long) major,
> 	                 (unsigned long)minor);
3196c3204
< static int define_port_context(int low, int high)
---
> static int define_port_context(uintptr_t low, uintptr_t high)
3295c3303
< static int define_node_context(int addr, int mask)
---
> static int define_node_context(uintptr_t addr, uintptr_t mask)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 17:56 [PATCH] checkpolicy/policy_scan.y Joerg Hoh
@ 2003-10-22 18:33 ` Stephen Smalley
  2003-10-22 19:02   ` Joerg Hoh
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2003-10-22 18:33 UTC (permalink / raw)
  To: Joerg Hoh; +Cc: selinux, James Morris

On Wed, 2003-10-22 at 13:56, Joerg Hoh wrote:
> The following patch fixes some warnings in policy_parse.y (add -W to the
> CFLAGS to display this warnings):

- You forgot to use diff -u to generate your patch.

- As is noted in the gcc man page, some of the warnings generated by -W
options not implied by -Wall are constructions that users generally do
not consider questionable.  They might indicate a problem, but we
shouldn't just blindly stamp them out.

- It makes no sense at all to change the type of val from int to
uintptr_t, as I think I mentioned in an earlier email.  The purpose of
the %union declaration is to define the set of types that may be
returned as semantic values.  Hence, the current %union declaration
indicates that semantic values may be integers (val) or pointers (ptr),
and the %type declarations specify the type for specific nonterminals. 
If val is uintptr_t, then there is no reason to have a %union at all, as
a single type can be used for all nonterminals (but this results in even
weaker typing than the %union, which at least distinguishes the two
cases).  Now, it may make sense to change the type of ptr to uintptr_t,
because that is sometimes used to pass unsigned integers or pointers for
define_cexpr.  And it may make sense to change the type of val from int
to unsigned int as I think it is only used for unsigned integers.  But
it makes absolutely no sense to change val to uintptr_t and to then move
cexpr over to type val.  I will look into changing ptr to uintptr_t and
val to unsigned int and the corresponding changes to the rest of the
code.

- Casting -1 to an unsigned type is an ugly construct, IMHO.  I much
prefer ~0, as that clearly indicates the intent.  If we need to
explicitly cast it to (access_vector_t) or add a UL suffix, then that is
fine, but I don't see it as an improvement to replace it with -1.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 18:33 ` Stephen Smalley
@ 2003-10-22 19:02   ` Joerg Hoh
  2003-10-22 20:44     ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: Joerg Hoh @ 2003-10-22 19:02 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, James Morris

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Wed, Oct 22, 2003 at 02:33:23PM -0400, Stephen Smalley wrote:
> On Wed, 2003-10-22 at 13:56, Joerg Hoh wrote:
> > The following patch fixes some warnings in policy_parse.y (add -W to the
> > CFLAGS to display this warnings):
> 
> - You forgot to use diff -u to generate your patch.

The unified patch is attached, sorry about that.

> - As is noted in the gcc man page, some of the warnings generated by -W
> options not implied by -Wall are constructions that users generally do
> not consider questionable.  They might indicate a problem, but we
> shouldn't just blindly stamp them out.

policy_parse.y:1666: warning: comparison between signed and unsigned

IMHO this is a warning we should fix (because it is easy to fix). Sure, a
warning is no error, but no warning is better than a warning that is 
suppressed by the compiler.


> - Casting -1 to an unsigned type is an ugly construct, IMHO.  I much
> prefer ~0, as that clearly indicates the intent.  If we need to
> explicitly cast it to (access_vector_t) or add a UL suffix, then that is
> fine, but I don't see it as an improvement to replace it with -1.

~0 is not clearly to me :-| 

Ok, I'll try to extract some coding conventions from your last mails.

Joerg

[-- Attachment #2: policy_parse --]
[-- Type: text/plain, Size: 11590 bytes --]

--- ../../upstream/current/checkpolicy/policy_parse.y	2003-10-19 19:29:30.000000000 +0200
+++ policy_parse.y	2003-10-20 21:01:39.000000000 +0200
@@ -10,6 +10,7 @@
 #include "services.h"
 #include "queue.h"
 #include "checkpolicy.h"
+#include <stdint.h>
 
 #define TRUE 1
 #define FALSE 0
@@ -48,25 +49,25 @@
 static int define_role_trans(void);
 static int define_role_allow(void);
 static int define_constraint(constraint_expr_t *expr);
-static constraint_expr_t *define_cexpr(__u32 expr_type, void *arg1, void* arg2);
+static uintptr_t define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2);
 static int define_user(void);
 static int parse_security_context(context_struct_t *c);
 static int define_initial_sid_context(void);
 static int define_fs_use(int behavior);
 static int define_genfs_context(int has_type);
-static int define_fs_context(int major, int minor);
-static int define_port_context(int low, int high);
+static int define_fs_context(uintptr_t major, uintptr_t minor);
+static int define_port_context(uintptr_t low, uintptr_t high);
 static int define_netif_context(void);
-static int define_node_context(int addr, int mask);
+static int define_node_context(uintptr_t addr, uintptr_t mask);
 %}
 
 %union {
-	int val;
+	uintptr_t val;
 	void *ptr;
 }
 
-%type <ptr> role_def roles cexpr cexpr_prim op roleop
-%type <val> ipv4_addr_def number
+%type <ptr> role_def roles
+%type <val> ipv4_addr_def number cexpr cexpr_prim op roleop
 
 %token PATH
 %token CLONE
@@ -322,64 +323,64 @@
 			{ $$ = $1; }
 			;
 cexpr_prim		: U1 op U2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, $2);
 			  if ($$ == 0) return -1; }
 			| R1 roleop R2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| T1 op T2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_TYPE, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_TYPE, $2);
 			  if ($$ == 0) return -1; }
 			| U1 op { if (insert_separator(1)) return -1; } user_names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_USER, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_USER, $2);
 			  if ($$ == 0) return -1; }
 			| U2 op { if (insert_separator(1)) return -1; } user_names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_USER | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_USER | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| R1 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| R2 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| T1 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, $2);
 			  if ($$ == 0) return -1; }
 			| T2 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| SAMEUSER
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| SOURCE ROLE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| TARGET ROLE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| ROLE roleop
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, (void*)$2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| SOURCE TYPE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| TARGET TYPE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			;
 op			: EQUALS
-			{ $$ = (void*)CEXPR_EQ; }
+			{ $$ = CEXPR_EQ; }
 			| NOTEQUAL
-			{ $$ = (void*)CEXPR_NEQ; }
+			{ $$ = CEXPR_NEQ; }
 			;
 roleop			: op 
 			{ $$ = $1; }
 			| DOM
-			{ $$ = (void*)CEXPR_DOM; }
+			{ $$ = CEXPR_DOM; }
 			| DOMBY
-			{ $$ = (void*)CEXPR_DOMBY; }
+			{ $$ = CEXPR_DOMBY; }
 			| INCOMP
-			{ $$ = (void*)CEXPR_INCOMP; }
+			{ $$ = CEXPR_INCOMP; }
 			;
 users			: user_def
 			| users user_def
@@ -1659,7 +1660,7 @@
 		     char *id)
 {
 	type_datum_t *t;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all types */
@@ -1715,7 +1716,8 @@
 	class_datum_t *cladatum;
 	ebitmap_t stypes, ttypes, tclasses;
 	__u32 newtype = 0;
-	int ret, i, j, k;
+	int ret;
+	unsigned int i, j, k;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -1864,7 +1866,8 @@
 	static char avbuf[1024];
 	class_datum_t *cladatum;
 	char *perm = NULL, *p;
-	int i, rc;
+	unsigned int i;
+	int rc;
 
 	cladatum = policydbp->class_val_to_struct[tclass-1];
 	p = avbuf;
@@ -1891,13 +1894,14 @@
 }
 
 
-static int te_avtab_helper(int which, int stype, int ttype, 
+static int te_avtab_helper(int which, unsigned int stype, unsigned int ttype, 
 			   ebitmap_t *tclasses, access_vector_t *avp)
 
 {
 	avtab_key_t avkey;
 	avtab_datum_t avdatum, *avdatump;
-	int ret, k;
+	int ret;
+	unsigned int k;
 
 	if (which == -AVTAB_ALLOWED) {
 		yyerror("neverallow should not reach this function.");
@@ -1958,7 +1962,8 @@
 	perm_datum_t *perdatum;
 	ebitmap_t stypes, ttypes, tclasses;
 	access_vector_t *avp;
-	int i, j, hiclass, self = 0;
+	unsigned int i, j, hiclass;
+	int self = 0;
 	te_assert_t *newassert;
 
 	if (pass == 1) {
@@ -2021,7 +2026,7 @@
 
 			if (strcmp(id, "*") == 0) {
 				/* set all permissions in the class */
-				avp[i] = ~0;
+				avp[i] = (access_vector_t) -1;
 				continue;
 			}
 
@@ -2219,7 +2224,8 @@
 {
 	role_datum_t *role;
 	char *role_id;
-	int i, ret;
+	unsigned int i;
+	int ret;
 
 	if (pass == 1) {
 		role_id = queue_remove(id_queue);
@@ -2274,7 +2280,7 @@
 		     char *id)
 {
 	role_datum_t *r;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all roles */
@@ -2317,7 +2323,7 @@
 	role_datum_t *role;
 	ebitmap_t roles, types;
 	struct role_trans *tr = 0;
-	int i, j;
+	unsigned int i, j;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2398,7 +2404,7 @@
 	char *id;
 	ebitmap_t roles, new_roles;
 	struct role_allow *ra = 0;
-	int i, j;
+	unsigned int i, j;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2462,7 +2468,8 @@
 	perm_datum_t *perdatum;
 	ebitmap_t classmap;
 	constraint_expr_t *e;
-	int i, depth;
+	unsigned int i;
+	int depth;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2572,8 +2579,8 @@
 }
 
 
-static constraint_expr_t *
- define_cexpr(__u32 expr_type, void* arg1, void* arg2)
+static uintptr_t
+ define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2)
 {
 	struct constraint_expr *expr, *e1 = NULL, *e2;
 	user_datum_t *user;
@@ -2586,13 +2593,13 @@
 			while ((id = queue_remove(id_queue))) 
 				free(id);
 		}
-		return (constraint_expr_t *)1; /* any non-NULL value */
+		return 1; /* any non-NULL value */
 	}
 
 	expr = malloc(sizeof(struct constraint_expr));
 	if (!expr) {
 		yyerror("out of memory");
-		return NULL;
+		return 0;
 	}
 	memset(expr, 0, sizeof(constraint_expr_t));
 	expr->expr_type = expr_type;
@@ -2608,10 +2615,10 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = expr;
-		return (struct constraint_expr *) arg1;
+		return arg1;
 	case CEXPR_AND:
 	case CEXPR_OR:
 		e1 = NULL;
@@ -2623,7 +2630,7 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = (struct constraint_expr *) arg2;
 
@@ -2636,17 +2643,17 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = expr;
-		return (struct constraint_expr *) arg1;
+		return arg1;
 	case CEXPR_ATTR:
-		expr->attr = (__u32)arg1;
-		expr->op = (__u32)arg2;
-		return expr;
+		expr->attr = arg1;
+		expr->op = arg2;
+		return (uintptr_t) expr;
 	case CEXPR_NAMES:
-		expr->attr = (__u32)arg1;
-		expr->op = (__u32)arg2;
+		expr->attr = arg1;
+		expr->op = arg2;
 		while ((id = (char *) queue_remove(id_queue))) {
 			if (expr->attr & CEXPR_USER) {
 				user = (user_datum_t *) hashtab_search(policydbp->p_users.table,
@@ -2655,7 +2662,7 @@
 					sprintf(errormsg, "unknown user %s", id);
 					yyerror(errormsg);
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				val = user->value;
 			} else if (expr->attr & CEXPR_ROLE) {
@@ -2665,38 +2672,38 @@
 					sprintf(errormsg, "unknown role %s", id);
 					yyerror(errormsg);
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
 				if (set_types(&expr->names, id)) {
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				continue;
 			} else {
 				yyerror("invalid constraint expression");
 				free(expr);
-				return NULL;
+				return 0;
 			}
 			if (ebitmap_set_bit(&expr->names, val - 1, TRUE)) {
 				yyerror("out of memory");
 				ebitmap_destroy(&expr->names);
 				free(expr);
-				return NULL;
+				return 0;
 			}
 			free(id);
 		}
-		return expr;
+		return (uintptr_t) expr;
 	default:
 		yyerror("invalid constraint expression");
 		free(expr);
-		return NULL;
+		return 0;
 	}
 
 	yyerror("invalid constraint expression");
 	free(expr);
-	return NULL;
+	return 0;
 }
 
 
@@ -2704,7 +2711,7 @@
 			  char *id)
 {
 	role_datum_t *r;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all roles */
@@ -3137,7 +3144,7 @@
 	return 0;
 }
 
-static int define_fs_context(int major, int minor)
+static int define_fs_context(uintptr_t major, uintptr_t minor)
 {
 	ocontext_t *newc, *c, *head;
 
@@ -3154,13 +3161,14 @@
 	}
 	memset(newc, 0, sizeof(ocontext_t));
 
-	newc->u.name = (char *) malloc(6);
+	newc->u.name = malloc((size_t) 6);
 	if (!newc->u.name) {
 		yyerror("out of memory");
 		free(newc);
 		return -1;
 	}
-	sprintf(newc->u.name, "%02x:%02x", major, minor);
+	sprintf(newc->u.name, "%02lx:%02lx", (unsigned long) major,
+	                 (unsigned long)minor);
 
 	if (parse_security_context(&newc->context[0])) {
 		free(newc->u.name);
@@ -3193,7 +3201,7 @@
 	return 0;
 }
 
-static int define_port_context(int low, int high)
+static int define_port_context(uintptr_t low, uintptr_t high)
 {
 	ocontext_t *newc;
 	char *id;
@@ -3292,7 +3300,7 @@
 	return 0;
 }
 
-static int define_node_context(int addr, int mask)
+static int define_node_context(uintptr_t addr, uintptr_t mask)
 {
 	ocontext_t *newc, *c, *l, *head;
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 19:02   ` Joerg Hoh
@ 2003-10-22 20:44     ` Stephen Smalley
  2003-10-22 21:14       ` Joerg Hoh
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2003-10-22 20:44 UTC (permalink / raw)
  To: Joerg Hoh; +Cc: selinux, James Morris

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

Below is a modified form of your patch that seeks to preserve the
distinction between val and ptr and introduces a valptr for the
uintptr_t case.  Let me know if you see warnings with this patch that
you do not see with your patch.  Note that with either patch, warnings
will still occur on 64bit unless you have also patched your constraint.h
to use uintptr_t for the fields of the constraint_expr structure, but
that is not acceptable as it changes the binary policy format (or it
would, if the write.c code were also adjusted correspondingly, and it
serves no purpose otherwise). 

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

[-- Attachment #2: parse.patch --]
[-- Type: text/x-patch, Size: 11386 bytes --]

Index: checkpolicy/policy_parse.y
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.5
diff -u -r1.5 policy_parse.y
--- checkpolicy/policy_parse.y	26 Jun 2003 17:23:12 -0000	1.5
+++ checkpolicy/policy_parse.y	22 Oct 2003 20:02:25 -0000
@@ -10,6 +10,7 @@
 #include "services.h"
 #include "queue.h"
 #include "checkpolicy.h"
+#include <stdint.h>
 
 #define TRUE 1
 #define FALSE 0
@@ -48,25 +49,27 @@
 static int define_role_trans(void);
 static int define_role_allow(void);
 static int define_constraint(constraint_expr_t *expr);
-static constraint_expr_t *define_cexpr(__u32 expr_type, void *arg1, void* arg2);
+static uintptr_t define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2);
 static int define_user(void);
 static int parse_security_context(context_struct_t *c);
 static int define_initial_sid_context(void);
 static int define_fs_use(int behavior);
 static int define_genfs_context(int has_type);
-static int define_fs_context(int major, int minor);
-static int define_port_context(int low, int high);
+static int define_fs_context(unsigned int major, unsigned int minor);
+static int define_port_context(unsigned int low, unsigned int high);
 static int define_netif_context(void);
-static int define_node_context(int addr, int mask);
+static int define_node_context(unsigned int addr, unsigned int mask);
 %}
 
 %union {
-	int val;
+	unsigned int val;
+	uintptr_t valptr;
 	void *ptr;
 }
 
-%type <ptr> role_def roles cexpr cexpr_prim op roleop
-%type <val> ipv4_addr_def number
+%type <ptr> role_def roles
+%type <valptr> cexpr cexpr_prim 
+%type <val> ipv4_addr_def number op roleop
 
 %token PATH
 %token CLONE
@@ -322,64 +325,64 @@
 			{ $$ = $1; }
 			;
 cexpr_prim		: U1 op U2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, $2);
 			  if ($$ == 0) return -1; }
 			| R1 roleop R2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| T1 op T2
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_TYPE, $2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_TYPE, $2);
 			  if ($$ == 0) return -1; }
 			| U1 op { if (insert_separator(1)) return -1; } user_names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_USER, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_USER, $2);
 			  if ($$ == 0) return -1; }
 			| U2 op { if (insert_separator(1)) return -1; } user_names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_USER | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_USER | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| R1 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| R2 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| T1 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, $2);
 			  if ($$ == 0) return -1; }
 			| T2 op { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), $2);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), $2);
 			  if ($$ == 0) return -1; }
 			| SAMEUSER
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_USER, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| SOURCE ROLE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_ROLE, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| TARGET ROLE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_ROLE | CEXPR_TARGET), (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| ROLE roleop
-			{ $$ = define_cexpr(CEXPR_ATTR, (void*)CEXPR_ROLE, (void*)$2);
+			{ $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2);
 			  if ($$ == 0) return -1; }
 			| SOURCE TYPE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)CEXPR_TYPE, (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			| TARGET TYPE { if (insert_separator(1)) return -1; } names_push
-			{ $$ = define_cexpr(CEXPR_NAMES, (void*)(CEXPR_TYPE | CEXPR_TARGET), (void*)CEXPR_EQ);
+			{ $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), CEXPR_EQ);
 			  if ($$ == 0) return -1; }
 			;
 op			: EQUALS
-			{ $$ = (void*)CEXPR_EQ; }
+			{ $$ = CEXPR_EQ; }
 			| NOTEQUAL
-			{ $$ = (void*)CEXPR_NEQ; }
+			{ $$ = CEXPR_NEQ; }
 			;
 roleop			: op 
 			{ $$ = $1; }
 			| DOM
-			{ $$ = (void*)CEXPR_DOM; }
+			{ $$ = CEXPR_DOM; }
 			| DOMBY
-			{ $$ = (void*)CEXPR_DOMBY; }
+			{ $$ = CEXPR_DOMBY; }
 			| INCOMP
-			{ $$ = (void*)CEXPR_INCOMP; }
+			{ $$ = CEXPR_INCOMP; }
 			;
 users			: user_def
 			| users user_def
@@ -1659,7 +1662,7 @@
 		     char *id)
 {
 	type_datum_t *t;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all types */
@@ -1715,7 +1718,8 @@
 	class_datum_t *cladatum;
 	ebitmap_t stypes, ttypes, tclasses;
 	__u32 newtype = 0;
-	int ret, i, j, k;
+	int ret;
+	unsigned int i, j, k;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -1864,7 +1868,8 @@
 	static char avbuf[1024];
 	class_datum_t *cladatum;
 	char *perm = NULL, *p;
-	int i, rc;
+	unsigned int i;
+	int rc;
 
 	cladatum = policydbp->class_val_to_struct[tclass-1];
 	p = avbuf;
@@ -1891,13 +1896,14 @@
 }
 
 
-static int te_avtab_helper(int which, int stype, int ttype, 
+static int te_avtab_helper(int which, unsigned int stype, unsigned int ttype, 
 			   ebitmap_t *tclasses, access_vector_t *avp)
 
 {
 	avtab_key_t avkey;
 	avtab_datum_t avdatum, *avdatump;
-	int ret, k;
+	int ret;
+	unsigned int k;
 
 	if (which == -AVTAB_ALLOWED) {
 		yyerror("neverallow should not reach this function.");
@@ -1958,7 +1964,8 @@
 	perm_datum_t *perdatum;
 	ebitmap_t stypes, ttypes, tclasses;
 	access_vector_t *avp;
-	int i, j, hiclass, self = 0;
+	unsigned int i, j, hiclass;
+	int self = 0;
 	te_assert_t *newassert;
 
 	if (pass == 1) {
@@ -2021,7 +2028,7 @@
 
 			if (strcmp(id, "*") == 0) {
 				/* set all permissions in the class */
-				avp[i] = ~0;
+				avp[i] = ~0UL;
 				continue;
 			}
 
@@ -2219,7 +2226,8 @@
 {
 	role_datum_t *role;
 	char *role_id;
-	int i, ret;
+	unsigned int i;
+	int ret;
 
 	if (pass == 1) {
 		role_id = queue_remove(id_queue);
@@ -2274,7 +2282,7 @@
 		     char *id)
 {
 	role_datum_t *r;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all roles */
@@ -2317,7 +2325,7 @@
 	role_datum_t *role;
 	ebitmap_t roles, types;
 	struct role_trans *tr = 0;
-	int i, j;
+	unsigned int i, j;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2398,7 +2406,7 @@
 	char *id;
 	ebitmap_t roles, new_roles;
 	struct role_allow *ra = 0;
-	int i, j;
+	unsigned int i, j;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2462,7 +2470,8 @@
 	perm_datum_t *perdatum;
 	ebitmap_t classmap;
 	constraint_expr_t *e;
-	int i, depth;
+	unsigned int i;
+	int depth;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2572,8 +2581,8 @@
 }
 
 
-static constraint_expr_t *
- define_cexpr(__u32 expr_type, void* arg1, void* arg2)
+static uintptr_t
+ define_cexpr(__u32 expr_type, uintptr_t arg1, uintptr_t arg2)
 {
 	struct constraint_expr *expr, *e1 = NULL, *e2;
 	user_datum_t *user;
@@ -2586,13 +2595,13 @@
 			while ((id = queue_remove(id_queue))) 
 				free(id);
 		}
-		return (constraint_expr_t *)1; /* any non-NULL value */
+		return 1; /* any non-NULL value */
 	}
 
 	expr = malloc(sizeof(struct constraint_expr));
 	if (!expr) {
 		yyerror("out of memory");
-		return NULL;
+		return 0;
 	}
 	memset(expr, 0, sizeof(constraint_expr_t));
 	expr->expr_type = expr_type;
@@ -2608,10 +2617,10 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = expr;
-		return (struct constraint_expr *) arg1;
+		return arg1;
 	case CEXPR_AND:
 	case CEXPR_OR:
 		e1 = NULL;
@@ -2623,7 +2632,7 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = (struct constraint_expr *) arg2;
 
@@ -2636,17 +2645,17 @@
 		if (!e1 || e1->next) {
 			yyerror("illegal constraint expression");
 			free(expr);
-			return NULL;
+			return 0;
 		}
 		e1->next = expr;
-		return (struct constraint_expr *) arg1;
+		return arg1;
 	case CEXPR_ATTR:
-		expr->attr = (__u32)arg1;
-		expr->op = (__u32)arg2;
-		return expr;
+		expr->attr = arg1;
+		expr->op = arg2;
+		return (uintptr_t)expr;
 	case CEXPR_NAMES:
-		expr->attr = (__u32)arg1;
-		expr->op = (__u32)arg2;
+		expr->attr = arg1;
+		expr->op = arg2;
 		while ((id = (char *) queue_remove(id_queue))) {
 			if (expr->attr & CEXPR_USER) {
 				user = (user_datum_t *) hashtab_search(policydbp->p_users.table,
@@ -2655,7 +2664,7 @@
 					sprintf(errormsg, "unknown user %s", id);
 					yyerror(errormsg);
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				val = user->value;
 			} else if (expr->attr & CEXPR_ROLE) {
@@ -2665,38 +2674,38 @@
 					sprintf(errormsg, "unknown role %s", id);
 					yyerror(errormsg);
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
 				if (set_types(&expr->names, id)) {
 					free(expr);
-					return NULL;
+					return 0;
 				}
 				continue;
 			} else {
 				yyerror("invalid constraint expression");
 				free(expr);
-				return NULL;
+				return 0;
 			}
 			if (ebitmap_set_bit(&expr->names, val - 1, TRUE)) {
 				yyerror("out of memory");
 				ebitmap_destroy(&expr->names);
 				free(expr);
-				return NULL;
+				return 0;
 			}
 			free(id);
 		}
-		return expr;
+		return (uintptr_t)expr;
 	default:
 		yyerror("invalid constraint expression");
 		free(expr);
-		return NULL;
+		return 0;
 	}
 
 	yyerror("invalid constraint expression");
 	free(expr);
-	return NULL;
+	return 0;
 }
 
 
@@ -2704,7 +2713,7 @@
 			  char *id)
 {
 	role_datum_t *r;
-	int i;
+	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all roles */
@@ -3137,7 +3146,7 @@
 	return 0;
 }
 
-static int define_fs_context(int major, int minor)
+static int define_fs_context(unsigned int major, unsigned int minor)
 {
 	ocontext_t *newc, *c, *head;
 
@@ -3193,7 +3202,7 @@
 	return 0;
 }
 
-static int define_port_context(int low, int high)
+static int define_port_context(unsigned int low, unsigned int high)
 {
 	ocontext_t *newc;
 	char *id;
@@ -3292,7 +3301,7 @@
 	return 0;
 }
 
-static int define_node_context(int addr, int mask)
+static int define_node_context(unsigned int addr, unsigned int mask)
 {
 	ocontext_t *newc, *c, *l, *head;
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 20:44     ` Stephen Smalley
@ 2003-10-22 21:14       ` Joerg Hoh
  2003-10-22 21:21         ` Stephen Smalley
  2003-10-22 21:27         ` Stephen Smalley
  0 siblings, 2 replies; 9+ messages in thread
From: Joerg Hoh @ 2003-10-22 21:14 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, James Morris

On Wed, Oct 22, 2003 at 04:44:57PM -0400, Stephen Smalley wrote:
> Below is a modified form of your patch that seeks to preserve the
> distinction between val and ptr and introduces a valptr for the
> uintptr_t case.  Let me know if you see warnings with this patch that
> you do not see with your patch. 

The original warnings:
yacc -d policy_parse.y
cc -g  -Wall -O2 -pipe -include global.h -I. -Iinclude -o y.tab.o -c y.tab.c
y.tab.c: In function `yyparse':
y.tab.c:1511: warning: implicit declaration of function `yylex'
policy_parse.y: In function `define_cexpr':
policy_parse.y:2644: warning: cast from pointer to integer of different size
policy_parse.y:2645: warning: cast from pointer to integer of different size
policy_parse.y:2648: warning: cast from pointer to integer of different size
policy_parse.y:2649: warning: cast from pointer to integer of different size

with your patch:
yacc -d policy_parse.y
cc -g  -Wall -O2 -pipe -include global.h -I. -Iinclude -o y.tab.o -c y.tab.c
y.tab.c: In function `yyparse':
y.tab.c:1513: warning: implicit declaration of function `yylex'
policy_parse.y: In function `define_te_avtab':
policy_parse.y:2031: warning: large integer implicitly truncated to unsigned type

with my patch:
yacc -d policy_parse.y
cc -g  -Wall -W  -O2 -pipe -include global.h -I. -Iinclude -o y.tab.o -c y.tab.c
y.tab.c: In function `yyparse':
y.tab.c:1512: warning: implicit declaration of function `yylex'

> Note that with either patch, warnings
> will still occur on 64bit unless you have also patched your constraint.h
> to use uintptr_t for the fields of the constraint_expr structure, but
> that is not acceptable as it changes the binary policy format (or it
> would, if the write.c code were also adjusted correspondingly, and it
> serves no purpose otherwise). 

I will try it tomorrow.

Joerg

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 21:14       ` Joerg Hoh
@ 2003-10-22 21:21         ` Stephen Smalley
  2003-10-22 21:27         ` Stephen Smalley
  1 sibling, 0 replies; 9+ messages in thread
From: Stephen Smalley @ 2003-10-22 21:21 UTC (permalink / raw)
  To: Joerg Hoh; +Cc: selinux, James Morris

On Wed, 2003-10-22 at 17:14, Joerg Hoh wrote:
> policy_parse.y: In function `define_te_avtab':
> policy_parse.y:2031: warning: large integer implicitly truncated to unsigned type

My mistake, ~0UL should be ~0U.  Or (access_vector_t) ~0.  

> > Note that with either patch, warnings
> > will still occur on 64bit unless you have also patched your constraint.h
> > to use uintptr_t for the fields of the constraint_expr structure, but
> > that is not acceptable as it changes the binary policy format (or it
> > would, if the write.c code were also adjusted correspondingly, and it
> > serves no purpose otherwise). 
> 
> I will try it tomorrow.

Not sure what you mean.  The point is that you can't change the types on
those fields.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 21:14       ` Joerg Hoh
  2003-10-22 21:21         ` Stephen Smalley
@ 2003-10-22 21:27         ` Stephen Smalley
  2003-10-23 21:04           ` Bastian Blank
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2003-10-22 21:27 UTC (permalink / raw)
  To: Joerg Hoh; +Cc: selinux, James Morris

On Wed, 2003-10-22 at 17:14, Joerg Hoh wrote:
> with your patch:
> yacc -d policy_parse.y
> cc -g  -Wall -O2 -pipe -include global.h -I. -Iinclude -o y.tab.o -c y.tab.c
> y.tab.c: In function `yyparse':
> y.tab.c:1513: warning: implicit declaration of function `yylex'
> policy_parse.y: In function `define_te_avtab':
> policy_parse.y:2031: warning: large integer implicitly truncated to unsigned type

I've merged this patch with the ~0UL -> ~0U fix.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-22 21:27         ` Stephen Smalley
@ 2003-10-23 21:04           ` Bastian Blank
  2003-10-24 13:00             ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: Bastian Blank @ 2003-10-23 21:04 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

On Wed, Oct 22, 2003 at 05:27:13PM -0400, Stephen Smalley wrote:
> I've merged this patch with the ~0UL -> ~0U fix.

I just find out that this patch have unpredictable side effects. It uses
integers of different size at the same time (op and roleop uses int,
cexpr and cexpr_prim uses uintptr_t).

this should work on 32bit machines (big and little), but breaks if
uintptr_t is larger than int, because accessing the int member only
writes the corresponding 32bit. the attached sources demonstrates this
problem.

bastian

-- 
Ahead warp factor one, Mr. Sulu.

[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 387 bytes --]

#include <stdint.h>
#include <stdio.h>

union
{
  uint16_t val1;
  uint32_t val2;
} test;

int main (void)
{
  test.val2 = 1 | 1 << 16;
  printf ("write 0x%x into 32\n", 1 | 1 << 16);
  printf ("32: 0x%x\n", test.val2);
  printf ("16: 0x%hx\n", test.val1);
  test.val1 = 1;
  printf ("write 0x%x into 16\n", 1);
  printf ("32: 0x%x\n", test.val2);
  printf ("16: 0x%hx\n", test.val1);
}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpolicy/policy_scan.y
  2003-10-23 21:04           ` Bastian Blank
@ 2003-10-24 13:00             ` Stephen Smalley
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Smalley @ 2003-10-24 13:00 UTC (permalink / raw)
  To: Bastian Blank; +Cc: selinux

On Thu, 2003-10-23 at 17:04, Bastian Blank wrote:
> On Wed, Oct 22, 2003 at 05:27:13PM -0400, Stephen Smalley wrote:
> > I've merged this patch with the ~0UL -> ~0U fix.
> 
> I just find out that this patch have unpredictable side effects. It uses
> integers of different size at the same time (op and roleop uses int,
> cexpr and cexpr_prim uses uintptr_t).
> 
> this should work on 32bit machines (big and little), but breaks if
> uintptr_t is larger than int, because accessing the int member only
> writes the corresponding 32bit. the attached sources demonstrates this
> problem.

Ok, so op and roleop need to be moved to <valptr> as well, with
corresponding changes to the rest of the code.  Remind me again as to
how this is any better than just using void* as in the original code?
And note that truncation will still ultimately occur when the values are
assigned to the fields of the constraint expression if they ever exceed
a 32bit value, as I've mentioned previously.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2003-10-24 13:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-22 17:56 [PATCH] checkpolicy/policy_scan.y Joerg Hoh
2003-10-22 18:33 ` Stephen Smalley
2003-10-22 19:02   ` Joerg Hoh
2003-10-22 20:44     ` Stephen Smalley
2003-10-22 21:14       ` Joerg Hoh
2003-10-22 21:21         ` Stephen Smalley
2003-10-22 21:27         ` Stephen Smalley
2003-10-23 21:04           ` Bastian Blank
2003-10-24 13:00             ` Stephen Smalley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.