All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libselinux-3
@ 2003-10-23 21:54 Bastian Blank
  2003-10-24 19:08 ` Bastian Blank
  0 siblings, 1 reply; 3+ messages in thread
From: Bastian Blank @ 2003-10-23 21:54 UTC (permalink / raw)
  To: selinux


[-- Attachment #1.1: Type: text/plain, Size: 264 bytes --]

hi folks

this patch fixes following problems:
- size_t vs. int, ssize_t
- unsigned int vs. int
- a little bit error checking
files: src/*

bastian

-- 
Only a fool fights in a burning house.
		-- Kank the Klingon, "Day of the Dove", stardate unknown

[-- Attachment #1.2: libselinux-3 --]
[-- Type: text/plain, Size: 8157 bytes --]

Index: src/get_context_list.c
===================================================================
--- src/get_context_list.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/get_context_list.c	(.../trunk/libselinux)	(revision 53)
@@ -36,13 +36,13 @@
 	       an entry is found, and a 1 is returned otherwise.
 */
 static int find_line (FILE *infile, security_context_t con, char *line, 
-                      int length)
+                      size_t length)
 {
     char *current_line;
     char *ptr, *ptr2 = NULL;
     int found = 0;
     char *cc_str = 0;
-    int cc_len = 0;
+    size_t cc_len = 0;
 
     /* Skip the user field. */
     cc_str = index(con, ':');
@@ -53,8 +53,8 @@
     if (!cc_len)
 	    return -1;
 
-    current_line = (char *) malloc (length);
+    current_line = malloc (length);
     if (!current_line)
 	    return (-1);
 
     while (!feof (infile)) {
@@ -119,9 +119,9 @@
                              int pri_length)
 {
     char *ptr, *ptr2;
-    int length;
+    size_t length;
     security_context_t current_context;
-    int current_context_len;
+    size_t current_context_len;
     int count = 0;
 
     ptr = instr;
@@ -205,7 +205,7 @@
 {
     FILE *config_file;    /* The configuration file                    */
     char *fname = 0;      /* The name of the user's configuration file */
-    int fname_len;        /* The length of fname                       */
+    size_t fname_len;     /* The length of fname                       */
     struct passwd *pwd;   /* The user's passwd structure               */
     int retval;           /* The return value                          */
 
@@ -218,7 +218,7 @@
             return -1;
         }
         fname_len = strlen (pwd->pw_dir) + 20;
-        fname = (char *) malloc (fname_len);
+        fname = malloc (fname_len);
         if (!fname)
         {
             return -1;
@@ -366,13 +366,13 @@
 	if (!ptr)
 		return -1;
 	plen = strlen(ptr);
-	if (buf[plen-1] == '\n') 
+	if (plen > 1 && buf[plen-1] == '\n') 
 		buf[plen-1] = 0;
 
 	nlen = strlen(user)+1+plen+1;
 	*newcon = malloc(nlen);
 	rc = snprintf(*newcon, nlen, "%s:%s", user, ptr);
-	if (rc < 0 || rc >= nlen) {
+	if (rc < 0 || (size_t) rc >= nlen) {
 		free(*newcon);
 		*newcon = 0;
 		return -1;
Index: src/fgetfilecon.c
===================================================================
--- src/fgetfilecon.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/fgetfilecon.c	(.../trunk/libselinux)	(revision 53)
@@ -10,7 +10,7 @@
 int fgetfilecon(int fd, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = (size_t) ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/lgetfilecon.c
===================================================================
--- src/lgetfilecon.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/lgetfilecon.c	(.../trunk/libselinux)	(revision 53)
@@ -10,7 +10,7 @@
 int lgetfilecon(const char *path, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = lgetxattr(path, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = lgetxattr(path, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = (size_t) ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/Makefile
===================================================================
--- src/Makefile	(.../upstream/current/libselinux)	(revision 53)
+++ src/Makefile	(.../trunk/libselinux)	(revision 53)
@@ -11,7 +11,7 @@
 LIBSO=$(TARGET).$(LIBVERSION)
 OBJS= $(patsubst %.c,%.o,$(wildcard *.c))
 LOBJS= $(patsubst %.c,%.lo,$(wildcard *.c))
-CFLAGS = -Wall
+CFLAGS = -Wall -W -Wconversion -Werror
 override CFLAGS += -I../include 
 
 all: $(LIBA) $(LIBSO)
Index: src/fsetfilecon.c
===================================================================
Index: src/helpers.c
===================================================================
--- src/helpers.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/helpers.c	(.../trunk/libselinux)	(revision 53)
@@ -23,7 +23,7 @@
 
 security_class_t string_to_security_class(const char *s)
 {
-	int val;
+	unsigned int val;
 
 	if (isdigit(s[0])) {
 		val = atoi(s);
@@ -45,7 +45,7 @@
 {
         char          **common_pts = 0;
         access_vector_t common_base = 0;
-        int             i, i2, perm;
+        unsigned int             i, i2, perm;
  
  
         if (av == 0) {
Index: src/lsetfilecon.c
===================================================================
Index: src/compute_user.c
===================================================================
--- src/compute_user.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/compute_user.c	(.../trunk/libselinux)	(revision 53)
@@ -16,8 +16,8 @@
 	char **ary;
 	char *buf, *ptr;
 	size_t size;
-	int fd, ret, i;
-	unsigned int nel;
+	int fd, ret;
+	unsigned int i, nel;
 
 	fd = open(SELINUXMNT "user", O_RDWR);
 	if (fd < 0)
Index: src/getfilecon.c
===================================================================
--- src/getfilecon.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/getfilecon.c	(.../trunk/libselinux)	(revision 53)
@@ -10,7 +10,7 @@
 int getfilecon(const char *path, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = getxattr(path, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = getxattr(path, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = (size_t) ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/context.c
===================================================================
--- src/context.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/context.c	(.../trunk/libselinux)	(revision 53)
@@ -41,10 +41,10 @@
         n->component[3] = 0;
         for ( i = 0, tok = str; *tok; i++ ) {
                 for ( p = tok; *p && *p != ':'; p++ ) { /* empty */ }
-                n->component[i] = (char*) malloc(p-tok+1);
+                n->component[i] = malloc((size_t) (p-tok+1));
 		if (n->component[i] == 0)
 		  goto err;
-                strncpy(n->component[i],tok,p-tok);
+                strncpy(n->component[i],tok,(size_t) (p-tok));
                 n->component[i][p-tok] = '\0';
                 tok = *p ? p+1 : p;
         }
@@ -93,14 +93,15 @@
 context_str(context_t context)
 {
         context_private_t *n = context->ptr;
-        int i, total;
+        int i;
+        size_t total = 0;
         conditional_free(&n->current_str);
-        for ( i = total = 0; i < 4; i++ ) {
+        for ( i = 0; i < 4; i++ ) {
                 if ( n->component[i] ) {
                         total += strlen(n->component[i])+1;
                 }
         }
-        n->current_str = (char*) malloc(total);
+        n->current_str = malloc(total);
         if ( n->current_str != 0 ) {
                 strcpy(n->current_str,n->component[0]);
                 strcat(n->current_str,":");
Index: src/get_default_type.c
===================================================================
--- src/get_default_type.c	(.../upstream/current/libselinux)	(revision 53)
+++ src/get_default_type.c	(.../trunk/libselinux)	(revision 53)
@@ -29,7 +29,7 @@
 {
     char buf[250];
     char *ptr = "", *end, *t;
-    int len;
+    size_t len;
     int found = 0;
 
     len = strlen(role);

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH] libselinux-3
  2003-10-23 21:54 [PATCH] libselinux-3 Bastian Blank
@ 2003-10-24 19:08 ` Bastian Blank
  2003-10-24 21:09   ` Stephen Smalley
  0 siblings, 1 reply; 3+ messages in thread
From: Bastian Blank @ 2003-10-24 19:08 UTC (permalink / raw)
  To: selinux; +Cc: Stephen Smalley

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

updated patch.

bastian

-- 
What kind of love is that?  Not to be loved; never to have shown love.
		-- Commissioner Nancy Hedford, "Metamorphosis",
		   stardate 3219.8

[-- Attachment #2: libselinux-3 --]
[-- Type: text/plain, Size: 7997 bytes --]

--- src/get_context_list.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/get_context_list.c	(.../trunk/libselinux)	(revision 62)
@@ -36,13 +36,13 @@
 	       an entry is found, and a 1 is returned otherwise.
 */
 static int find_line (FILE *infile, security_context_t con, char *line, 
-                      int length)
+                      size_t length)
 {
     char *current_line;
     char *ptr, *ptr2 = NULL;
     int found = 0;
     char *cc_str = 0;
-    int cc_len = 0;
+    size_t cc_len = 0;
 
     /* Skip the user field. */
     cc_str = index(con, ':');
@@ -53,12 +53,12 @@
     if (!cc_len)
 	    return -1;
 
-    current_line = (char *) malloc (length);
+    current_line = malloc (length);
     if (!current_line)
 	    return (-1);
 
     while (!feof (infile)) {
 	if (!fgets(current_line, length, infile)) {
 		free(current_line);
 		return -1;
 	}
@@ -119,9 +119,9 @@
                              int pri_length)
 {
     char *ptr, *ptr2;
-    int length;
+    size_t length;
     security_context_t current_context;
-    int current_context_len;
+    size_t current_context_len;
     int count = 0;
 
     ptr = instr;
@@ -205,7 +205,7 @@
 {
     FILE *config_file;    /* The configuration file                    */
     char *fname = 0;      /* The name of the user's configuration file */
-    int fname_len;        /* The length of fname                       */
+    size_t fname_len;     /* The length of fname                       */
     struct passwd *pwd;   /* The user's passwd structure               */
     int retval;           /* The return value                          */
 
@@ -218,7 +218,7 @@
             return -1;
         }
         fname_len = strlen (pwd->pw_dir) + 20;
-        fname = (char *) malloc (fname_len);
+        fname = malloc (fname_len);
         if (!fname)
         {
             return -1;
@@ -366,13 +366,13 @@
 	if (!ptr)
 		return -1;
 	plen = strlen(ptr);
-	if (buf[plen-1] == '\n') 
+	if (plen > 1 && buf[plen-1] == '\n') 
 		buf[plen-1] = 0;
 
 	nlen = strlen(user)+1+plen+1;
 	*newcon = malloc(nlen);
 	rc = snprintf(*newcon, nlen, "%s:%s", user, ptr);
-	if (rc < 0 || rc >= nlen) {
+	if (rc < 0 || (size_t) rc >= nlen) {
 		free(*newcon);
 		*newcon = 0;
 		return -1;
Index: src/fgetfilecon.c
===================================================================
--- src/fgetfilecon.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/fgetfilecon.c	(.../trunk/libselinux)	(revision 62)
@@ -10,7 +10,7 @@
 int fgetfilecon(int fd, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/lgetfilecon.c
===================================================================
--- src/lgetfilecon.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/lgetfilecon.c	(.../trunk/libselinux)	(revision 62)
@@ -10,7 +10,7 @@
 int lgetfilecon(const char *path, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = lgetxattr(path, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = lgetxattr(path, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/Makefile
===================================================================
--- src/Makefile	(.../upstream/current/libselinux)	(revision 62)
+++ src/Makefile	(.../trunk/libselinux)	(revision 62)
@@ -11,8 +11,9 @@
 LIBSO=$(TARGET).$(LIBVERSION)
 OBJS= $(patsubst %.c,%.o,$(wildcard *.c))
 LOBJS= $(patsubst %.c,%.lo,$(wildcard *.c))
-CFLAGS = -Wall
-override CFLAGS += -I../include 
+CFLAGS = -Wall -W -Werror -O2 -pipe
+override CFLAGS += -I../include
+LDFLAGS =
 
 all: $(LIBA) $(LIBSO)
 
@@ -25,10 +26,10 @@
 	ln -sf $@ $(TARGET) 
 
 %.o:  %.c
-	$(CC) -o $@ -c $(CFLAGS) $<
+	$(CC) $(CFLAGS) -c -o $@ $<
 
 %.lo:  %.c
-	$(CC) -o $@ -c -fPIC $(CFLAGS) $<
+	$(CC) $(CFLAGS) -fPIC -c -o $@ $<
 
 install: all
 	test -d $(LIBDIR) || install -m 755 -d $(LIBDIR)
Index: src/fsetfilecon.c
===================================================================
Index: src/compute_av.c
===================================================================
--- src/compute_av.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/compute_av.c	(.../trunk/libselinux)	(revision 62)
@@ -16,7 +16,7 @@
 			struct av_decision *avd)
 {
 	char *buf;
-	unsigned int len;
+	size_t len;
 	int fd, ret;
 
 	fd = open(SELINUXMNT "access", O_RDWR);
Index: src/helpers.c
===================================================================
--- src/helpers.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/helpers.c	(.../trunk/libselinux)	(revision 62)
@@ -23,7 +23,7 @@
 
 security_class_t string_to_security_class(const char *s)
 {
-	int val;
+	unsigned int val;
 
 	if (isdigit(s[0])) {
 		val = atoi(s);
@@ -45,7 +45,7 @@
 {
         char          **common_pts = 0;
         access_vector_t common_base = 0;
-        int             i, i2, perm;
+        unsigned int             i, i2, perm;
  
  
         if (av == 0) {
Index: src/lsetfilecon.c
===================================================================
Index: src/compute_user.c
===================================================================
--- src/compute_user.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/compute_user.c	(.../trunk/libselinux)	(revision 62)
@@ -16,8 +16,8 @@
 	char **ary;
 	char *buf, *ptr;
 	size_t size;
-	int fd, ret, i;
-	unsigned int nel;
+	int fd, ret;
+	unsigned int i, nel;
 
 	fd = open(SELINUXMNT "user", O_RDWR);
 	if (fd < 0)
Index: src/getfilecon.c
===================================================================
--- src/getfilecon.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/getfilecon.c	(.../trunk/libselinux)	(revision 62)
@@ -10,7 +10,7 @@
 int getfilecon(const char *path, security_context_t *context)
 {
 	char *buf;
-	ssize_t size;
+	size_t size;
 	ssize_t ret;
 
 	size = INITCONTEXTLEN+1;
@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = getxattr(path, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = getxattr(path, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;
Index: src/get_default_type.c
===================================================================
--- src/get_default_type.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/get_default_type.c	(.../trunk/libselinux)	(revision 62)
@@ -29,7 +29,7 @@
 {
     char buf[250];
     char *ptr = "", *end, *t;
-    int len;
+    size_t len;
     int found = 0;
 
     len = strlen(role);
Index: src/context.c
===================================================================
--- src/context.c	(.../upstream/current/libselinux)	(revision 62)
+++ src/context.c	(.../trunk/libselinux)	(revision 62)
@@ -93,14 +93,15 @@
 context_str(context_t context)
 {
         context_private_t *n = context->ptr;
-        int i, total;
+        int i;
+        size_t total = 0;
         conditional_free(&n->current_str);
-        for ( i = total = 0; i < 4; i++ ) {
+        for ( i = 0; i < 4; i++ ) {
                 if ( n->component[i] ) {
                         total += strlen(n->component[i])+1;
                 }
         }
-        n->current_str = (char*) malloc(total);
+        n->current_str = malloc(total);
         if ( n->current_str != 0 ) {
                 strcpy(n->current_str,n->component[0]);
                 strcat(n->current_str,":");

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

* Re: [PATCH] libselinux-3
  2003-10-24 19:08 ` Bastian Blank
@ 2003-10-24 21:09   ` Stephen Smalley
  0 siblings, 0 replies; 3+ messages in thread
From: Stephen Smalley @ 2003-10-24 21:09 UTC (permalink / raw)
  To: Bastian Blank; +Cc: selinux

On Fri, 2003-10-24 at 15:08, Bastian Blank wrote:
> updated patch.

@@ -366,13 +366,13 @@
 	if (!ptr)
 		return -1;
 	plen = strlen(ptr);
-	if (buf[plen-1] == '\n') 
+	if (plen > 1 && buf[plen-1] == '\n') 
 		buf[plen-1] = 0;

I believe that the original code is correct, and that the patch
introduces a minor bug.  If the file is empty, ptr will be NULL and the
!ptr test will catch it. Hence, plen should always be at least 1. 
Further, your test for plen > 0 means that you won't clear the newline
in the case where the file contains only a newline and nothing else,
i.e. plen == 1 and buf[0] == '\n'.  

@@ -23,11 +23,11 @@
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
-		if (size < 0)
+		ret = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
+		if (ret < 0)
 			goto out;
 
-		size++;
+		size = ret + 1;
 		newbuf = realloc(buf, size);
 		if (!newbuf)
 			goto out;

As with the earlier patch for the *getfilecon functions, this
one introduces a bug.  Look at the result of this patch when
the realloc fails; since ret is reset by your change, the
out path will not treat it as an error, and the caller will
be returned garbage without any indication that the call
actually failed.

-- 
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] 3+ messages in thread

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-10-23 21:54 [PATCH] libselinux-3 Bastian Blank
2003-10-24 19:08 ` Bastian Blank
2003-10-24 21:09   ` 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.