* libsetrans performance patch
@ 2006-01-03 18:54 Russell Coker
2006-01-03 19:58 ` Daniel J Walsh
0 siblings, 1 reply; 3+ messages in thread
From: Russell Coker @ 2006-01-03 18:54 UTC (permalink / raw)
To: SE-Linux
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
I have optimised the libsetrans library a bit. Changed locking of a FILE *,
removed some needless strdup/strlen operations, and fixed a bug whereby
strtrim could potentially copy random data from the heap (ended up removing
strtrim).
--
http://www.coker.com.au/selinux/ My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/ Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/ Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/ My home page
[-- Attachment #2: libsetrans.diff --]
[-- Type: text/x-diff, Size: 5670 bytes --]
diff -rup libsetrans-0.1.11.orig/src/setrans.c libsetrans-0.1.11/src/setrans.c
--- libsetrans-0.1.11.orig/src/setrans.c 2005-12-08 00:50:31.000000000 +1100
+++ libsetrans-0.1.11/src/setrans.c 2006-01-02 20:28:36.000000000 +1100
@@ -3,7 +3,9 @@
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
+#include <stdio_ext.h>
#include <ctype.h>
+#include <alloca.h>
#include <selinux/selinux.h>
#include <selinux/context.h>
#include "dso.h"
@@ -22,23 +24,6 @@ typedef struct labels {
static labels_t *labellist=NULL;
-/* Remove excess white space */
-static char * strtrim(char *dest, char *source, int size) {
- int i=0;
- char *ptr=source;
- i=0;
- while(isspace(*ptr) && i < size) {
- ptr++;
- i++;
- }
- strncpy(dest,ptr,size);
- for(i=strlen(dest)-1; i> 0; i--) {
- if (!isspace(dest[i])) break;
- }
- dest[i+1]='\0';
- return dest;
-}
-
void finish_context_translations(void) {
labels_t *ptr=NULL;
labels_t *current=NULL;
@@ -56,54 +41,54 @@ void finish_context_translations(void) {
/* Process line from translation file.
Remove white space and set name do data before the "=" and sename to data
- after it */
-static int process_label(const char *buffer, labels_t **labels) {
- char name[BUFSIZ];
- char name1[BUFSIZ];
- char *newbuf=strdup(buffer);
- int namesize=sizeof(name);
+ after it
+ May modify the data pointed to by the buffer paremeter */
+static int process_label(char *buffer, labels_t **labels) {
+ char *name, *name_end;
labels_t *next=NULL;
- char *ptr;
- int rc=0;
+ char *ptr, *tok;
- if (!newbuf) return -1;
-
- char *tok=strtok_r(newbuf,"=",&ptr);
- if (!tok) goto err;
- strncpy(name1,tok, BUFSIZ-1);
- strtrim(name,name1,namesize-1);
- if ( name[0]=='#' ) goto err;
- tok=strtok_r(NULL,"\0",&ptr);
- if (!tok) goto err;
+ while(isspace(*buffer))
+ buffer++;
+ if(buffer[0] == '#') return 0;
+ name = strtok_r(buffer, "=", &ptr);
+ if (!name || !*name) return 0;
+ tok = strtok_r(NULL, "\0", &ptr);
+ if (!tok) return 0;
+ name_end = tok - 2;
+ while(isspace(*name_end))
+ {
+ name_end = '\0';
+ name_end--;
+ }
while (isspace(*tok)) tok++;
+ if(!*tok) return 0;
next=(labels_t *) calloc(1, sizeof(labels_t));
if (!next) {
- rc=-1;
- goto err;
+ return -1;
}
next->label.name=strdup(name);
if (!next->label.name) {
free(next);
- rc=-1;
- goto err;
+ return -1;
}
- strncpy(name1,tok, BUFSIZ-1);
- strtrim(name,name1,namesize-1);
- next->label.sename=strdup(name);
+ name_end = tok + strlen(tok) - 1;
+ while(isspace(*name_end))
+ {
+ *name_end = '\0';
+ name_end--;
+ }
+ next->label.sename=strdup(tok);
if (!next->label.sename) {
free(next->label.name);
free(next);
- rc=-1;
- goto err;
+ return -1;
}
*labels=next;
- rc=1;
-err:
- free(newbuf);
- return rc;
+ return 1;
}
/* Look for selabel via internal name */
@@ -121,21 +106,15 @@ static const char *translate_internal(co
}
static char *translate_categories(const char *sensitivity, const char *cat) {
char *label=NULL;
- char *inbuf=NULL;
- int slen=strlen(sensitivity);
+ char *inbuf = alloca(strlen(sensitivity) + strlen(cat) + 2);
char *ptr;
int error=0;
- char *categories=strdup(cat);
- if (!categories) return NULL;
+ char *categories=strdupa(cat);
+ if (!categories || !inbuf) return NULL;
char *tok=strtok_r(categories,",",&ptr);
while (tok) {
const char *trans;
- inbuf=malloc(slen+strlen(tok)+2);
- if (!inbuf) {
- error=1;
- break;
- }
sprintf(inbuf, "%s:%s", sensitivity, tok);
trans=translate_internal(inbuf);
if (strcmp(trans, inbuf)==0) {
@@ -156,13 +135,10 @@ static char *translate_categories(const
}
free(tmp);
}
- free(inbuf); inbuf=NULL;
tok=strtok_r(NULL,",",&ptr);
}
- free(categories);
if (error) {
if (label) free(label);
- if (inbuf) free(inbuf);
return NULL;
}
return label;
@@ -175,7 +151,7 @@ static const char *translate(const char
if (orig) {
if (strcmp(orig,labels)==0) {
char *translated=NULL;
- char *tmpbuf=strdup(orig);
+ char *tmpbuf=strdupa(orig);
char *ptr=NULL;
char *tok=NULL;
char *sensitivity=NULL;
@@ -196,8 +172,6 @@ static const char *translate(const char
}
done:
- if (tmpbuf)
- free(tmpbuf);
if (categories) {
free(categories);
return translated;
@@ -209,7 +183,7 @@ static const char *translate(const char
}
/* Look for selabel via external name */
-static char *untranslate_internal(const char *sename) {
+static const char *untranslate_internal(const char *sename) {
labels_t *ptr=NULL;
if (labellist)
for(ptr=labellist->next;ptr; ptr=ptr->next)
@@ -223,7 +197,7 @@ static char *untranslate(const char *sen
if (orig) {
if (strcmp(orig,sename)==0) {
char *untranslated=NULL;
- char *tmpbuf=strdup(orig);
+ char *tmpbuf=strdupa(orig);
char *ptr=NULL;
char *tok=NULL;
char *sensitivity=NULL;
@@ -245,7 +219,8 @@ static char *untranslate(const char *sen
break;
}
} else {
- sensitivity=calloc(1,idx+1);
+ sensitivity = alloca(idx + 1);
+ sensitivity[idx] = '\0';
strncpy(sensitivity, utrans, idx);
}
category=&utrans[idx+1];
@@ -266,10 +241,6 @@ static char *untranslate(const char *sen
tok=strtok_r(NULL,",",&ptr);
}
done:
- if (tmpbuf)
- free(tmpbuf);
- if (sensitivity)
- free(sensitivity);
if (error) {
if (untranslated)
free(untranslated);
@@ -302,6 +273,7 @@ int init_context_translations(void) {
cfg = fopen(selinux_translations_path(),"r");
if (!cfg) return 1;
+ __fsetlocking(cfg, FSETLOCKING_BYCALLER);
ptr=labellist=calloc(1,sizeof(labels_t));
if (!ptr) {
fclose(cfg);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: libsetrans performance patch
2006-01-03 18:54 libsetrans performance patch Russell Coker
@ 2006-01-03 19:58 ` Daniel J Walsh
2006-01-04 8:37 ` Russell Coker
0 siblings, 1 reply; 3+ messages in thread
From: Daniel J Walsh @ 2006-01-03 19:58 UTC (permalink / raw)
To: russell; +Cc: SE-Linux
Russell Coker wrote:
> I have optimised the libsetrans library a bit. Changed locking of a FILE *,
> removed some needless strdup/strlen operations, and fixed a bug whereby
> strtrim could potentially copy random data from the heap (ended up removing
> strtrim).
>
I have added most of your previous patch to libsetrans-0.1.14-1 Please
check to make sure I got everything. There was one problem in your
patch that caused untranslate to fail a:b:c->a:b:c:s0
>
> ------------------------------------------------------------------------
>
> diff -rup libsetrans-0.1.11.orig/src/setrans.c libsetrans-0.1.11/src/setrans.c
> --- libsetrans-0.1.11.orig/src/setrans.c 2005-12-08 00:50:31.000000000 +1100
> +++ libsetrans-0.1.11/src/setrans.c 2006-01-02 20:28:36.000000000 +1100
> @@ -3,7 +3,9 @@
> #include <stdlib.h>
> #include <string.h>
> #include <stdio.h>
> +#include <stdio_ext.h>
> #include <ctype.h>
> +#include <alloca.h>
> #include <selinux/selinux.h>
> #include <selinux/context.h>
> #include "dso.h"
> @@ -22,23 +24,6 @@ typedef struct labels {
>
> static labels_t *labellist=NULL;
>
> -/* Remove excess white space */
> -static char * strtrim(char *dest, char *source, int size) {
> - int i=0;
> - char *ptr=source;
> - i=0;
> - while(isspace(*ptr) && i < size) {
> - ptr++;
> - i++;
> - }
> - strncpy(dest,ptr,size);
> - for(i=strlen(dest)-1; i> 0; i--) {
> - if (!isspace(dest[i])) break;
> - }
> - dest[i+1]='\0';
> - return dest;
> -}
> -
> void finish_context_translations(void) {
> labels_t *ptr=NULL;
> labels_t *current=NULL;
> @@ -56,54 +41,54 @@ void finish_context_translations(void) {
>
> /* Process line from translation file.
> Remove white space and set name do data before the "=" and sename to data
> - after it */
> -static int process_label(const char *buffer, labels_t **labels) {
> - char name[BUFSIZ];
> - char name1[BUFSIZ];
> - char *newbuf=strdup(buffer);
> - int namesize=sizeof(name);
> + after it
> + May modify the data pointed to by the buffer paremeter */
> +static int process_label(char *buffer, labels_t **labels) {
> + char *name, *name_end;
> labels_t *next=NULL;
> - char *ptr;
> - int rc=0;
> + char *ptr, *tok;
>
> - if (!newbuf) return -1;
> -
> - char *tok=strtok_r(newbuf,"=",&ptr);
> - if (!tok) goto err;
> - strncpy(name1,tok, BUFSIZ-1);
> - strtrim(name,name1,namesize-1);
> - if ( name[0]=='#' ) goto err;
> - tok=strtok_r(NULL,"\0",&ptr);
> - if (!tok) goto err;
> + while(isspace(*buffer))
> + buffer++;
> + if(buffer[0] == '#') return 0;
> + name = strtok_r(buffer, "=", &ptr);
> + if (!name || !*name) return 0;
> + tok = strtok_r(NULL, "\0", &ptr);
> + if (!tok) return 0;
> + name_end = tok - 2;
> + while(isspace(*name_end))
> + {
> + name_end = '\0';
> + name_end--;
> + }
> while (isspace(*tok)) tok++;
>
THIS LINE SHOULD BE REMOVED so "" will translate to s0.
> + if(!*tok) return 0;
>
>
> next=(labels_t *) calloc(1, sizeof(labels_t));
> if (!next) {
> - rc=-1;
> - goto err;
> + return -1;
> }
>
> next->label.name=strdup(name);
> if (!next->label.name) {
> free(next);
> - rc=-1;
> - goto err;
> + return -1;
> }
>
> - strncpy(name1,tok, BUFSIZ-1);
> - strtrim(name,name1,namesize-1);
> - next->label.sename=strdup(name);
> + name_end = tok + strlen(tok) - 1;
> + while(isspace(*name_end))
> + {
> + *name_end = '\0';
> + name_end--;
> + }
> + next->label.sename=strdup(tok);
> if (!next->label.sename) {
> free(next->label.name);
> free(next);
> - rc=-1;
> - goto err;
> + return -1;
> }
> *labels=next;
> - rc=1;
> -err:
> - free(newbuf);
> - return rc;
> + return 1;
> }
>
> /* Look for selabel via internal name */
> @@ -121,21 +106,15 @@ static const char *translate_internal(co
> }
> static char *translate_categories(const char *sensitivity, const char *cat) {
> char *label=NULL;
> - char *inbuf=NULL;
> - int slen=strlen(sensitivity);
> + char *inbuf = alloca(strlen(sensitivity) + strlen(cat) + 2);
> char *ptr;
> int error=0;
> - char *categories=strdup(cat);
> - if (!categories) return NULL;
> + char *categories=strdupa(cat);
> + if (!categories || !inbuf) return NULL;
> char *tok=strtok_r(categories,",",&ptr);
>
> while (tok) {
> const char *trans;
> - inbuf=malloc(slen+strlen(tok)+2);
> - if (!inbuf) {
> - error=1;
> - break;
> - }
> sprintf(inbuf, "%s:%s", sensitivity, tok);
> trans=translate_internal(inbuf);
> if (strcmp(trans, inbuf)==0) {
> @@ -156,13 +135,10 @@ static char *translate_categories(const
> }
> free(tmp);
> }
> - free(inbuf); inbuf=NULL;
> tok=strtok_r(NULL,",",&ptr);
> }
> - free(categories);
> if (error) {
> if (label) free(label);
> - if (inbuf) free(inbuf);
> return NULL;
> }
> return label;
> @@ -175,7 +151,7 @@ static const char *translate(const char
> if (orig) {
> if (strcmp(orig,labels)==0) {
> char *translated=NULL;
> - char *tmpbuf=strdup(orig);
> + char *tmpbuf=strdupa(orig);
> char *ptr=NULL;
> char *tok=NULL;
> char *sensitivity=NULL;
> @@ -196,8 +172,6 @@ static const char *translate(const char
>
> }
> done:
> - if (tmpbuf)
> - free(tmpbuf);
> if (categories) {
> free(categories);
> return translated;
> @@ -209,7 +183,7 @@ static const char *translate(const char
> }
>
> /* Look for selabel via external name */
> -static char *untranslate_internal(const char *sename) {
> +static const char *untranslate_internal(const char *sename) {
> labels_t *ptr=NULL;
> if (labellist)
> for(ptr=labellist->next;ptr; ptr=ptr->next)
> @@ -223,7 +197,7 @@ static char *untranslate(const char *sen
> if (orig) {
> if (strcmp(orig,sename)==0) {
> char *untranslated=NULL;
> - char *tmpbuf=strdup(orig);
> + char *tmpbuf=strdupa(orig);
> char *ptr=NULL;
> char *tok=NULL;
> char *sensitivity=NULL;
> @@ -245,7 +219,8 @@ static char *untranslate(const char *sen
> break;
> }
> } else {
> - sensitivity=calloc(1,idx+1);
> + sensitivity = alloca(idx + 1);
> + sensitivity[idx] = '\0';
> strncpy(sensitivity, utrans, idx);
> }
> category=&utrans[idx+1];
> @@ -266,10 +241,6 @@ static char *untranslate(const char *sen
> tok=strtok_r(NULL,",",&ptr);
> }
> done:
> - if (tmpbuf)
> - free(tmpbuf);
> - if (sensitivity)
> - free(sensitivity);
> if (error) {
> if (untranslated)
> free(untranslated);
> @@ -302,6 +273,7 @@ int init_context_translations(void) {
> cfg = fopen(selinux_translations_path(),"r");
> if (!cfg) return 1;
>
> + __fsetlocking(cfg, FSETLOCKING_BYCALLER);
> ptr=labellist=calloc(1,sizeof(labels_t));
> if (!ptr) {
> fclose(cfg);
>
--
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* Re: libsetrans performance patch
2006-01-03 19:58 ` Daniel J Walsh
@ 2006-01-04 8:37 ` Russell Coker
0 siblings, 0 replies; 3+ messages in thread
From: Russell Coker @ 2006-01-04 8:37 UTC (permalink / raw)
To: Daniel J Walsh; +Cc: SE-Linux
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]
On Wednesday 04 January 2006 06:58, Daniel J Walsh <dwalsh@redhat.com> wrote:
> Russell Coker wrote:
> > I have optimised the libsetrans library a bit. Changed locking of a FILE
> > *, removed some needless strdup/strlen operations, and fixed a bug
> > whereby strtrim could potentially copy random data from the heap (ended
> > up removing strtrim).
>
> I have added most of your previous patch to libsetrans-0.1.14-1 Please
> check to make sure I got everything. There was one problem in your
> patch that caused untranslate to fail a:b:c->a:b:c:s0
It seems to have got everything.
setrans.c: In function 'translate_context':
setrans.c:381: warning: control may reach end of non-void function 'translate'
being inlined
I've attached a patch which fixes the above bug, removed a needless check for
the return value of strdupa(), and includes the fix for the problem you
mention above (pity I didn't test rpm's use of libsetrans before sending the
previous patch).
--
http://www.coker.com.au/selinux/ My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/ Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/ Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/ My home page
[-- Attachment #2: trans.diff --]
[-- Type: text/x-diff, Size: 852 bytes --]
diff -rup libsetrans-0.1.13.orig/src/setrans.c libsetrans-0.1.13/src/setrans.c
--- libsetrans-0.1.13.orig/src/setrans.c 2006-01-03 01:12:20.000000000 +1100
+++ libsetrans-0.1.13/src/setrans.c 2006-01-04 19:33:30.000000000 +1100
@@ -62,7 +62,6 @@ static int process_label(char *buffer, l
name_end--;
}
while (isspace(*tok)) tok++;
- if(!*tok) return 0;
next=(labels_t *) calloc(1, sizeof(labels_t));
if (!next) {
@@ -142,7 +141,6 @@ static char *translate_categories(const
char *ptr;
int error=0;
char *categories=strdupa(cat);
- if (!categories) return NULL;
char *tok=strtok_r(categories,",",&ptr);
while (tok) {
@@ -251,6 +249,8 @@ static const char *translate(const char
free((char *) l1);
free((char *) l2);
if (error==0) return ptr;
+ free(ptr);
+ return NULL;
}
else
return translate_label(labels);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-01-04 8:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-03 18:54 libsetrans performance patch Russell Coker
2006-01-03 19:58 ` Daniel J Walsh
2006-01-04 8:37 ` Russell Coker
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.