All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] policycoreutils: get setfiles to skip mounts without seclabel
@ 2009-07-14 14:15 Thomas Liu
  2009-07-14 15:30 ` Stephen Smalley
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Liu @ 2009-07-14 14:15 UTC (permalink / raw)
  To: selinux; +Cc: sds, jmorris, eparis, dwalsh

Get setfiles to check paths for seclabel and skip them
if it is not supported.

Parse /proc/mounts and add paths that do not have seclabel
to the exclude list.  If another path shows up that does
have seclabel, remove it from the exclude list, since setfiles
will try and when it fails it will skip it.

Also made one of the error messages in add_exclude more
descriptive.

Signed-off-by: Thomas Liu <tliu@redhat.com>
Signed-off-by: Dan Walsh <dwalsh@redhat.com>
---

 policycoreutils/setfiles/setfiles.c |   80 ++++++++++++++++++++++++++++++++++-
 1 files changed, 77 insertions(+), 3 deletions(-)


diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index 1356fb9..9f27a1d 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -11,6 +11,7 @@
 #include <ctype.h>
 #include <regex.h>
 #include <sys/vfs.h>
+#include <sys/utsname.h>
 #define __USE_XOPEN_EXTENDED 1	/* nftw */
 #define SKIP -2
 #define ERR -1
@@ -39,7 +40,7 @@ static int force = 0;
 static int progress = 0;
 static unsigned long long count = 0;
 
-#define MAX_EXCLUDES 100
+#define MAX_EXCLUDES 1000
 static int excludeCtr = 0;
 struct edir {
 	char *directory;
@@ -243,8 +244,8 @@ static int add_exclude(const char *directory)
 		return 1;
 	}
 	if (lstat(directory, &sb)) {
-		fprintf(stderr, "Directory \"%s\" not found, ignoring.\n",
-			directory);
+		fprintf(stderr, "Can't stat directory \"%s\", %s.\n",
+			directory, strerror(errno));
 		return 0;
 	}
 	if ((sb.st_mode & S_IFDIR) == 0) {
@@ -275,6 +276,20 @@ static int add_exclude(const char *directory)
 	return 0;
 }
 
+static void remove_exclude(const char *directory)
+{
+	int i = 0;
+	for (i = 0; i < excludeCtr; i++) {
+		if (strcmp(directory, excludeArray[i].directory) == 0) {
+			if (i != excludeCtr-1)
+				excludeArray[i] = excludeArray[excludeCtr-1];
+			excludeCtr--;
+			return;
+		}
+	}
+	return;
+}
+
 static int exclude(const char *file)
 {
 	int i = 0;
@@ -699,6 +714,62 @@ static void maybe_audit_mass_relabel(void)
 #endif
 }
 
+/*
+   Search /proc mounts for all file systems that do not support extended
+   attributes and add them to the exclude directory table.  File systems
+   support seclabel are labeled seclabel
+*/
+static void exclude_non_seclabel_mounts()
+{
+	struct utsname uts;
+	FILE *fp = fopen("/proc/mounts", "r");
+	size_t len;
+	ssize_t num;
+	int index = 0, found = 0;
+	char *mount_info[4];
+	char *buf = NULL, *item;
+	/* Check to see if the kernel supports seclabel */
+	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
+		return;
+	if (!fp)
+		return;
+
+	while ((num = getline(&buf, &len, fp)) != -1) {
+		found = 0;
+		index = 0;
+		item = strtok(buf, " ");
+		while (item != NULL) {
+			mount_info[index] = item;
+			if (index == 3)
+				break;
+			index++;
+			item = strtok(NULL, " ");
+		}
+		if (index < 3) {
+			fprintf(stderr,
+				"/proc/mounts record \"%s\" has incorrect format.\n",
+				buf);
+			continue;
+		}
+
+		/* remove pre-existing entry */
+		remove_exclude(mount_info[1]);
+
+		item = strtok(mount_info[3], ",");
+		while (item != NULL) {
+			if (strcmp(item, "seclabel") == 0) {
+				found = 1;
+				break;
+			}
+			item = strtok(NULL, ",");
+		}
+
+		/* exclude mount points with out seclabel flag */
+		if (!found)
+			add_exclude(mount_info[1]);
+	}
+}
+
 int main(int argc, char **argv)
 {
 	struct stat sb;
@@ -766,6 +837,9 @@ int main(int argc, char **argv)
 			exit(0);
 	}
 
+	/* This must happen before getops */
+	exclude_non_seclabel_mounts();
+
 	/* Process any options. */
 	while ((opt = getopt(argc, argv, "c:de:f:ilnpqrsvo:FRW0")) > 0) {
 		switch (opt) {



--
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 related	[flat|nested] 7+ messages in thread

* Re: [PATCH] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-14 14:15 [PATCH] policycoreutils: get setfiles to skip mounts without seclabel Thomas Liu
@ 2009-07-14 15:30 ` Stephen Smalley
  2009-07-14 16:04   ` Thomas Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Smalley @ 2009-07-14 15:30 UTC (permalink / raw)
  To: Thomas Liu; +Cc: selinux, jmorris, eparis, dwalsh

On Tue, 2009-07-14 at 10:15 -0400, Thomas Liu wrote:
> Get setfiles to check paths for seclabel and skip them
> if it is not supported.
> 
> Parse /proc/mounts and add paths that do not have seclabel
> to the exclude list.  If another path shows up that does
> have seclabel, remove it from the exclude list, since setfiles
> will try and when it fails it will skip it.
> 
> Also made one of the error messages in add_exclude more
> descriptive.
> 
> Signed-off-by: Thomas Liu <tliu@redhat.com>
> Signed-off-by: Dan Walsh <dwalsh@redhat.com>
> ---
> 
>  policycoreutils/setfiles/setfiles.c |   80 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 77 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
> index 1356fb9..9f27a1d 100644
> --- a/policycoreutils/setfiles/setfiles.c
> +++ b/policycoreutils/setfiles/setfiles.c
> @@ -11,6 +11,7 @@
>  #include <ctype.h>
>  #include <regex.h>
>  #include <sys/vfs.h>
> +#include <sys/utsname.h>
>  #define __USE_XOPEN_EXTENDED 1	/* nftw */
>  #define SKIP -2
>  #define ERR -1
> @@ -39,7 +40,7 @@ static int force = 0;
>  static int progress = 0;
>  static unsigned long long count = 0;
>  
> -#define MAX_EXCLUDES 100
> +#define MAX_EXCLUDES 1000
>  static int excludeCtr = 0;
>  struct edir {
>  	char *directory;
> @@ -243,8 +244,8 @@ static int add_exclude(const char *directory)
>  		return 1;
>  	}
>  	if (lstat(directory, &sb)) {
> -		fprintf(stderr, "Directory \"%s\" not found, ignoring.\n",
> -			directory);
> +		fprintf(stderr, "Can't stat directory \"%s\", %s.\n",
> +			directory, strerror(errno));

Might want to keep the "ignoring" part as otherwise this appears to be a
fatal error.

>  		return 0;
>  	}
>  	if ((sb.st_mode & S_IFDIR) == 0) {
> @@ -275,6 +276,20 @@ static int add_exclude(const char *directory)
>  	return 0;
>  }
>  
> +static void remove_exclude(const char *directory)
> +{
> +	int i = 0;
> +	for (i = 0; i < excludeCtr; i++) {
> +		if (strcmp(directory, excludeArray[i].directory) == 0) {
> +			if (i != excludeCtr-1)
> +				excludeArray[i] = excludeArray[excludeCtr-1];
> +			excludeCtr--;
> +			return;
> +		}

The function name and interface doesn't clearly indicate that this
function only works if removing the last entry (and if so, why are we
scanning the entire array)?  But see below as well.

> +	}
> +	return;
> +}
> +
>  static int exclude(const char *file)
>  {
>  	int i = 0;
> @@ -699,6 +714,62 @@ static void maybe_audit_mass_relabel(void)
>  #endif
>  }
>  
> +/*
> +   Search /proc mounts for all file systems that do not support extended
> +   attributes and add them to the exclude directory table.  File systems
> +   support seclabel are labeled seclabel
> +*/
> +static void exclude_non_seclabel_mounts()
> +{
> +	struct utsname uts;
> +	FILE *fp = fopen("/proc/mounts", "r");
> +	size_t len;
> +	ssize_t num;
> +	int index = 0, found = 0;
> +	char *mount_info[4];
> +	char *buf = NULL, *item;
> +	/* Check to see if the kernel supports seclabel */
> +	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
> +		return;

Move the fopen down here, or fclose(fp) before returning above.

> +	if (!fp)
> +		return;
> +
> +	while ((num = getline(&buf, &len, fp)) != -1) {
> +		found = 0;
> +		index = 0;
> +		item = strtok(buf, " ");
> +		while (item != NULL) {
> +			mount_info[index] = item;
> +			if (index == 3)
> +				break;
> +			index++;
> +			item = strtok(NULL, " ");
> +		}
> +		if (index < 3) {
> +			fprintf(stderr,
> +				"/proc/mounts record \"%s\" has incorrect format.\n",
> +				buf);
> +			continue;
> +		}
> +
> +		/* remove pre-existing entry */
> +		remove_exclude(mount_info[1]);

Do we really need this?  Are there multiple entries on the same mount
point directory with different option lists?  Is this to handle special
cases like rootfs (which should likely just get ignored)?

> +
> +		item = strtok(mount_info[3], ",");
> +		while (item != NULL) {
> +			if (strcmp(item, "seclabel") == 0) {
> +				found = 1;
> +				break;
> +			}
> +			item = strtok(NULL, ",");
> +		}
> +
> +		/* exclude mount points with out seclabel flag */
> +		if (!found)
> +			add_exclude(mount_info[1]);
> +	}
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct stat sb;
> @@ -766,6 +837,9 @@ int main(int argc, char **argv)
>  			exit(0);
>  	}
>  
> +	/* This must happen before getops */
> +	exclude_non_seclabel_mounts();

What happens if a directory gets excluded by both this logic and via
explicit option?  We don't want multiple entries in the array for that.

> +
>  	/* Process any options. */
>  	while ((opt = getopt(argc, argv, "c:de:f:ilnpqrsvo:FRW0")) > 0) {
>  		switch (opt) {
> 
> 
> 
> --
> 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.
-- 
Stephen Smalley
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] 7+ messages in thread

* Re: [PATCH] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-14 15:30 ` Stephen Smalley
@ 2009-07-14 16:04   ` Thomas Liu
  2009-07-14 17:50     ` Daniel J Walsh
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Liu @ 2009-07-14 16:04 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, jmorris, eparis, dwalsh

On Tue, 2009-07-14 at 11:30 -0400, Stephen Smalley wrote:
> On Tue, 2009-07-14 at 10:15 -0400, Thomas Liu wrote:
> > Get setfiles to check paths for seclabel and skip them
> > if it is not supported.
> > 
> > Parse /proc/mounts and add paths that do not have seclabel
> > to the exclude list.  If another path shows up that does
> > have seclabel, remove it from the exclude list, since setfiles
> > will try and when it fails it will skip it.
> > 
> > Also made one of the error messages in add_exclude more
> > descriptive.
> > 
> > Signed-off-by: Thomas Liu <tliu@redhat.com>
> > Signed-off-by: Dan Walsh <dwalsh@redhat.com>
> > ---
> > 
> >  policycoreutils/setfiles/setfiles.c |   80 ++++++++++++++++++++++++++++++++++-
> >  1 files changed, 77 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
> > index 1356fb9..9f27a1d 100644
> > --- a/policycoreutils/setfiles/setfiles.c
> > +++ b/policycoreutils/setfiles/setfiles.c
> > @@ -11,6 +11,7 @@
> >  #include <ctype.h>
> >  #include <regex.h>
> >  #include <sys/vfs.h>
> > +#include <sys/utsname.h>
> >  #define __USE_XOPEN_EXTENDED 1	/* nftw */
> >  #define SKIP -2
> >  #define ERR -1
> > @@ -39,7 +40,7 @@ static int force = 0;
> >  static int progress = 0;
> >  static unsigned long long count = 0;
> >  
> > -#define MAX_EXCLUDES 100
> > +#define MAX_EXCLUDES 1000
> >  static int excludeCtr = 0;
> >  struct edir {
> >  	char *directory;
> > @@ -243,8 +244,8 @@ static int add_exclude(const char *directory)
> >  		return 1;
> >  	}
> >  	if (lstat(directory, &sb)) {
> > -		fprintf(stderr, "Directory \"%s\" not found, ignoring.\n",
> > -			directory);
> > +		fprintf(stderr, "Can't stat directory \"%s\", %s.\n",
> > +			directory, strerror(errno));
> 
> Might want to keep the "ignoring" part as otherwise this appears to be a
> fatal error.
> 
> >  		return 0;
> >  	}
> >  	if ((sb.st_mode & S_IFDIR) == 0) {
> > @@ -275,6 +276,20 @@ static int add_exclude(const char *directory)
> >  	return 0;
> >  }
> >  
> > +static void remove_exclude(const char *directory)
> > +{
> > +	int i = 0;
> > +	for (i = 0; i < excludeCtr; i++) {
> > +		if (strcmp(directory, excludeArray[i].directory) == 0) {
> > +			if (i != excludeCtr-1)
> > +				excludeArray[i] = excludeArray[excludeCtr-1];
> > +			excludeCtr--;
> > +			return;
> > +		}
It doesn't only work if removing the last entry.  If the current entry
matches the directory we passed in, it swaps it with the last one and
then decrements excludeCtr, "removing" the current entry.
> The function name and interface doesn't clearly indicate that this
> function only works if removing the last entry (and if so, why are we
> scanning the entire array)?  But see below as well.
> 
> > +	}
> > +	return;
> > +}
> > +
> >  static int exclude(const char *file)
> >  {
> >  	int i = 0;
> > @@ -699,6 +714,62 @@ static void maybe_audit_mass_relabel(void)
> >  #endif
> >  }
> >  
> > +/*
> > +   Search /proc mounts for all file systems that do not support extended
> > +   attributes and add them to the exclude directory table.  File systems
> > +   support seclabel are labeled seclabel
> > +*/
> > +static void exclude_non_seclabel_mounts()
> > +{
> > +	struct utsname uts;
> > +	FILE *fp = fopen("/proc/mounts", "r");
> > +	size_t len;
> > +	ssize_t num;
> > +	int index = 0, found = 0;
> > +	char *mount_info[4];
> > +	char *buf = NULL, *item;
> > +	/* Check to see if the kernel supports seclabel */
> > +	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
> > +		return;
> 
> Move the fopen down here, or fclose(fp) before returning above.
> 
> > +	if (!fp)
> > +		return;
> > +
> > +	while ((num = getline(&buf, &len, fp)) != -1) {
> > +		found = 0;
> > +		index = 0;
> > +		item = strtok(buf, " ");
> > +		while (item != NULL) {
> > +			mount_info[index] = item;
> > +			if (index == 3)
> > +				break;
> > +			index++;
> > +			item = strtok(NULL, " ");
> > +		}
> > +		if (index < 3) {
> > +			fprintf(stderr,
> > +				"/proc/mounts record \"%s\" has incorrect format.\n",
> > +				buf);
> > +			continue;
> > +		}
> > +
> > +		/* remove pre-existing entry */
> > +		remove_exclude(mount_info[1]);
Our impression is that it is needed, because of bind mounts or if two
things are just mounted in the same place.

Although it seems that maybe the logic should be always add and remove
if found instead of always removing and adding if not found.

Thoughts?

> Do we really need this?  Are there multiple entries on the same mount
> point directory with different option lists?  Is this to handle special
> cases like rootfs (which should likely just get ignored)?
> 
> > +
> > +		item = strtok(mount_info[3], ",");
> > +		while (item != NULL) {
> > +			if (strcmp(item, "seclabel") == 0) {
> > +				found = 1;
> > +				break;
> > +			}
> > +			item = strtok(NULL, ",");
> > +		}
> > +
> > +		/* exclude mount points with out seclabel flag */
> > +		if (!found)
> > +			add_exclude(mount_info[1]);
> > +	}
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >  	struct stat sb;
> > @@ -766,6 +837,9 @@ int main(int argc, char **argv)
> >  			exit(0);
> >  	}
> >  
> > +	/* This must happen before getops */
> > +	exclude_non_seclabel_mounts();
That's true.  I guess I could either have add_exclude check to see if it's
already in the list or just always remove first.

> What happens if a directory gets excluded by both this logic and via
> explicit option?  We don't want multiple entries in the array for that.
> 
> > +
> >  	/* Process any options. */
> >  	while ((opt = getopt(argc, argv, "c:de:f:ilnpqrsvo:FRW0")) > 0) {
> >  		switch (opt) {
> > 
> > 
> > 
> > --
> > 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.


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

* Re: [PATCH] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-14 16:04   ` Thomas Liu
@ 2009-07-14 17:50     ` Daniel J Walsh
  2009-07-17 14:48       ` [PATCH -v2] " Thomas Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel J Walsh @ 2009-07-14 17:50 UTC (permalink / raw)
  To: Thomas Liu; +Cc: Stephen Smalley, selinux, jmorris, eparis

On 07/14/2009 12:04 PM, Thomas Liu wrote:
> On Tue, 2009-07-14 at 11:30 -0400, Stephen Smalley wrote:
>> On Tue, 2009-07-14 at 10:15 -0400, Thomas Liu wrote:
>>> Get setfiles to check paths for seclabel and skip them
>>> if it is not supported.
>>>
>>> Parse /proc/mounts and add paths that do not have seclabel
>>> to the exclude list.  If another path shows up that does
>>> have seclabel, remove it from the exclude list, since setfiles
>>> will try and when it fails it will skip it.
>>>
>>> Also made one of the error messages in add_exclude more
>>> descriptive.
>>>
>>> Signed-off-by: Thomas Liu <tliu@redhat.com>
>>> Signed-off-by: Dan Walsh <dwalsh@redhat.com>
>>> ---
>>>
>>>  policycoreutils/setfiles/setfiles.c |   80 ++++++++++++++++++++++++++++++++++-
>>>  1 files changed, 77 insertions(+), 3 deletions(-)
>>>
>>>
>>> diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
>>> index 1356fb9..9f27a1d 100644
>>> --- a/policycoreutils/setfiles/setfiles.c
>>> +++ b/policycoreutils/setfiles/setfiles.c
>>> @@ -11,6 +11,7 @@
>>>  #include <ctype.h>
>>>  #include <regex.h>
>>>  #include <sys/vfs.h>
>>> +#include <sys/utsname.h>
>>>  #define __USE_XOPEN_EXTENDED 1	/* nftw */
>>>  #define SKIP -2
>>>  #define ERR -1
>>> @@ -39,7 +40,7 @@ static int force = 0;
>>>  static int progress = 0;
>>>  static unsigned long long count = 0;
>>>  
>>> -#define MAX_EXCLUDES 100
>>> +#define MAX_EXCLUDES 1000
>>>  static int excludeCtr = 0;
>>>  struct edir {
>>>  	char *directory;
>>> @@ -243,8 +244,8 @@ static int add_exclude(const char *directory)
>>>  		return 1;
>>>  	}
>>>  	if (lstat(directory, &sb)) {
>>> -		fprintf(stderr, "Directory \"%s\" not found, ignoring.\n",
>>> -			directory);
>>> +		fprintf(stderr, "Can't stat directory \"%s\", %s.\n",
>>> +			directory, strerror(errno));
>> Might want to keep the "ignoring" part as otherwise this appears to be a
>> fatal error.
>>
>>>  		return 0;
>>>  	}
>>>  	if ((sb.st_mode & S_IFDIR) == 0) {
>>> @@ -275,6 +276,20 @@ static int add_exclude(const char *directory)
>>>  	return 0;
>>>  }
>>>  
>>> +static void remove_exclude(const char *directory)
>>> +{
>>> +	int i = 0;
>>> +	for (i = 0; i < excludeCtr; i++) {
>>> +		if (strcmp(directory, excludeArray[i].directory) == 0) {
>>> +			if (i != excludeCtr-1)
>>> +				excludeArray[i] = excludeArray[excludeCtr-1];
>>> +			excludeCtr--;
>>> +			return;
>>> +		}
> It doesn't only work if removing the last entry.  If the current entry
> matches the directory we passed in, it swaps it with the last one and
> then decrements excludeCtr, "removing" the current entry.
>> The function name and interface doesn't clearly indicate that this
>> function only works if removing the last entry (and if so, why are we
>> scanning the entire array)?  But see below as well.
>>
>>> +	}
>>> +	return;
>>> +}
>>> +
>>>  static int exclude(const char *file)
>>>  {
>>>  	int i = 0;
>>> @@ -699,6 +714,62 @@ static void maybe_audit_mass_relabel(void)
>>>  #endif
>>>  }
>>>  
>>> +/*
>>> +   Search /proc mounts for all file systems that do not support extended
>>> +   attributes and add them to the exclude directory table.  File systems
>>> +   support seclabel are labeled seclabel
>>> +*/
>>> +static void exclude_non_seclabel_mounts()
>>> +{
>>> +	struct utsname uts;
>>> +	FILE *fp = fopen("/proc/mounts", "r");
>>> +	size_t len;
>>> +	ssize_t num;
>>> +	int index = 0, found = 0;
>>> +	char *mount_info[4];
>>> +	char *buf = NULL, *item;
>>> +	/* Check to see if the kernel supports seclabel */
>>> +	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
>>> +		return;
>> Move the fopen down here, or fclose(fp) before returning above.
>>
>>> +	if (!fp)
>>> +		return;
>>> +
>>> +	while ((num = getline(&buf, &len, fp)) != -1) {
>>> +		found = 0;
>>> +		index = 0;
>>> +		item = strtok(buf, " ");
>>> +		while (item != NULL) {
>>> +			mount_info[index] = item;
>>> +			if (index == 3)
>>> +				break;
>>> +			index++;
>>> +			item = strtok(NULL, " ");
>>> +		}
>>> +		if (index < 3) {
>>> +			fprintf(stderr,
>>> +				"/proc/mounts record \"%s\" has incorrect format.\n",
>>> +				buf);
>>> +			continue;
>>> +		}
>>> +
>>> +		/* remove pre-existing entry */
>>> +		remove_exclude(mount_info[1]);
> Our impression is that it is needed, because of bind mounts or if two
> things are just mounted in the same place.
> 
> Although it seems that maybe the logic should be always add and remove
> if found instead of always removing and adding if not found.
> 
> Thoughts?
>
If you mount  /tmp with ext3 and then mount nfs over /tmp then we want to remove the /tmp directory.  Currently when this happens there are two entries in the /proc/mounts directory.  rootfs mounts as / as no support for seclabel and the another entry which supports seclabel, so we need to remove the first.  

We are making an assumption that the last entry is always the final say.  Which may or may not be the case.
 
>> Do we really need this?  Are there multiple entries on the same mount
>> point directory with different option lists?  Is this to handle special
>> cases like rootfs (which should likely just get ignored)?
>>
>>> +
>>> +		item = strtok(mount_info[3], ",");
>>> +		while (item != NULL) {
>>> +			if (strcmp(item, "seclabel") == 0) {
>>> +				found = 1;
>>> +				break;
>>> +			}
>>> +			item = strtok(NULL, ",");
>>> +		}
>>> +
>>> +		/* exclude mount points with out seclabel flag */
>>> +		if (!found)
>>> +			add_exclude(mount_info[1]);
>>> +	}
>>> +}
>>> +
>>>  int main(int argc, char **argv)
>>>  {
>>>  	struct stat sb;
>>> @@ -766,6 +837,9 @@ int main(int argc, char **argv)
>>>  			exit(0);
>>>  	}
>>>  
>>> +	/* This must happen before getops */
>>> +	exclude_non_seclabel_mounts();
> That's true.  I guess I could either have add_exclude check to see if it's
> already in the list or just always remove first.
> 
>> What happens if a directory gets excluded by both this logic and via
>> explicit option?  We don't want multiple entries in the array for that.
>>
It would end up being in the list multiple times, which would not be a problem unless you wanted to pathologically cause problems.

-e /etc -e /etc -e /etc -e /etc -e /usr

Would cause each directory looked at too search multiple entries as upposed to slowing down startup by checking for duplicates.

remove_exlcude()
add_exclude()

Would fix.  

>>> +
>>>  	/* Process any options. */
>>>  	while ((opt = getopt(argc, argv, "c:de:f:ilnpqrsvo:FRW0")) > 0) {
>>>  		switch (opt) {
>>>
>>>
>>>
>>> --
>>> 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.
> 


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

* [PATCH -v2] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-14 17:50     ` Daniel J Walsh
@ 2009-07-17 14:48       ` Thomas Liu
  2009-07-24 20:12         ` Stephen Smalley
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Liu @ 2009-07-17 14:48 UTC (permalink / raw)
  To: Daniel J Walsh; +Cc: Stephen Smalley, selinux, jmorris, eparis

Get setfiles to check paths for seclabel and skip them
if it is not supported.

Parse /proc/mounts and add paths that do not have seclabel
to the exclude list.  If another path shows up that does
have seclabel, remove it from the exclude list, since setfiles
will try and when it fails it will skip it.

Also made one of the error messages in add_exclude more
descriptive.

Signed-off-by: Thomas Liu <tliu@redhat.com>
Signed-off-by: Dan Walsh <dwalsh@redhat.com>
---
Fixed up a few things based on suggestions.
 policycoreutils/setfiles/setfiles.c |   82 ++++++++++++++++++++++++++++++++++-
 1 files changed, 79 insertions(+), 3 deletions(-)


diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index 1356fb9..00dfd87 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -11,6 +11,7 @@
 #include <ctype.h>
 #include <regex.h>
 #include <sys/vfs.h>
+#include <sys/utsname.h>
 #define __USE_XOPEN_EXTENDED 1	/* nftw */
 #define SKIP -2
 #define ERR -1
@@ -39,7 +40,7 @@ static int force = 0;
 static int progress = 0;
 static unsigned long long count = 0;
 
-#define MAX_EXCLUDES 100
+#define MAX_EXCLUDES 1000
 static int excludeCtr = 0;
 struct edir {
 	char *directory;
@@ -243,8 +244,8 @@ static int add_exclude(const char *directory)
 		return 1;
 	}
 	if (lstat(directory, &sb)) {
-		fprintf(stderr, "Directory \"%s\" not found, ignoring.\n",
-			directory);
+		fprintf(stderr, "Can't stat directory \"%s\", %s.\n",
+			directory, strerror(errno));
 		return 0;
 	}
 	if ((sb.st_mode & S_IFDIR) == 0) {
@@ -275,6 +276,20 @@ static int add_exclude(const char *directory)
 	return 0;
 }
 
+static void remove_exclude(const char *directory)
+{
+	int i = 0;
+	for (i = 0; i < excludeCtr; i++) {
+		if (strcmp(directory, excludeArray[i].directory) == 0) {
+			if (i != excludeCtr-1)
+				excludeArray[i] = excludeArray[excludeCtr-1];
+			excludeCtr--;
+			return;
+		}
+	}
+	return;
+}
+
 static int exclude(const char *file)
 {
 	int i = 0;
@@ -699,6 +714,63 @@ static void maybe_audit_mass_relabel(void)
 #endif
 }
 
+/*
+   Search /proc mounts for all file systems that do not support extended
+   attributes and add them to the exclude directory table.  File systems
+   support seclabel are labeled seclabel
+*/
+static void exclude_non_seclabel_mounts()
+{
+	struct utsname uts;
+	FILE *fp;
+	size_t len;
+	ssize_t num;
+	int index = 0, found = 0;
+	char *mount_info[4];
+	char *buf = NULL, *item;
+	/* Check to see if the kernel supports seclabel */
+	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
+		return;
+	fp = fopen("/proc/mounts", "r");
+	if (!fp)
+		return;
+
+	while ((num = getline(&buf, &len, fp)) != -1) {
+		found = 0;
+		index = 0;
+		item = strtok(buf, " ");
+		while (item != NULL) {
+			mount_info[index] = item;
+			if (index == 3)
+				break;
+			index++;
+			item = strtok(NULL, " ");
+		}
+		if (index < 3) {
+			fprintf(stderr,
+				"/proc/mounts record \"%s\" has incorrect format.\n",
+				buf);
+			continue;
+		}
+
+		/* remove pre-existing entry */
+		remove_exclude(mount_info[1]);
+
+		item = strtok(mount_info[3], ",");
+		while (item != NULL) {
+			if (strcmp(item, "seclabel") == 0) {
+				found = 1;
+				break;
+			}
+			item = strtok(NULL, ",");
+		}
+
+		/* exclude mount points with out seclabel flag */
+		if (!found)
+			add_exclude(mount_info[1]);
+	}
+}
+
 int main(int argc, char **argv)
 {
 	struct stat sb;
@@ -766,6 +838,9 @@ int main(int argc, char **argv)
 			exit(0);
 	}
 
+	/* This must happen before getops */
+	exclude_non_seclabel_mounts();
+
 	/* Process any options. */
 	while ((opt = getopt(argc, argv, "c:de:f:ilnpqrsvo:FRW0")) > 0) {
 		switch (opt) {
@@ -802,6 +877,7 @@ int main(int argc, char **argv)
 				break;
 			}
 		case 'e':
+			remove_exclude(optarg);
 			if (add_exclude(optarg))
 				exit(1);
 			break;



--
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 related	[flat|nested] 7+ messages in thread

* Re: [PATCH -v2] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-17 14:48       ` [PATCH -v2] " Thomas Liu
@ 2009-07-24 20:12         ` Stephen Smalley
  2009-07-27 13:21           ` Stephen Smalley
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Smalley @ 2009-07-24 20:12 UTC (permalink / raw)
  To: Thomas Liu; +Cc: Daniel J Walsh, selinux, jmorris, eparis

On Fri, 2009-07-17 at 10:48 -0400, Thomas Liu wrote:
> Get setfiles to check paths for seclabel and skip them
> if it is not supported.
> 
> Parse /proc/mounts and add paths that do not have seclabel
> to the exclude list.  If another path shows up that does
> have seclabel, remove it from the exclude list, since setfiles
> will try and when it fails it will skip it.
> 
> Also made one of the error messages in add_exclude more
> descriptive.
> 
> Signed-off-by: Thomas Liu <tliu@redhat.com>
> Signed-off-by: Dan Walsh <dwalsh@redhat.com>
> ---

Thanks, merged in policycoreutils 2.0.68.

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

* Re: [PATCH -v2] policycoreutils: get setfiles to skip mounts without seclabel
  2009-07-24 20:12         ` Stephen Smalley
@ 2009-07-27 13:21           ` Stephen Smalley
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Smalley @ 2009-07-27 13:21 UTC (permalink / raw)
  To: Thomas Liu; +Cc: Daniel J Walsh, selinux, jmorris, eparis

On Fri, 2009-07-24 at 16:12 -0400, Stephen Smalley wrote:
> On Fri, 2009-07-17 at 10:48 -0400, Thomas Liu wrote:
> > Get setfiles to check paths for seclabel and skip them
> > if it is not supported.
> > 
> > Parse /proc/mounts and add paths that do not have seclabel
> > to the exclude list.  If another path shows up that does
> > have seclabel, remove it from the exclude list, since setfiles
> > will try and when it fails it will skip it.
> > 
> > Also made one of the error messages in add_exclude more
> > descriptive.
> > 
> > Signed-off-by: Thomas Liu <tliu@redhat.com>
> > Signed-off-by: Dan Walsh <dwalsh@redhat.com>
> > ---
> 
> Thanks, merged in policycoreutils 2.0.68.

Applied this patch on top to free the buffer allocated by getline() and
to free any removed entries from the excludeArray.  valgrind
--leak-check=full then shows no leakage.

diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index 1c780a4..5e5d957 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -281,6 +281,7 @@ static void remove_exclude(const char *directory)
 	int i = 0;
 	for (i = 0; i < excludeCtr; i++) {
 		if (strcmp(directory, excludeArray[i].directory) == 0) {
+			free(excludeArray[i].directory);
 			if (i != excludeCtr-1)
 				excludeArray[i] = excludeArray[excludeCtr-1];
 			excludeCtr--;
@@ -728,9 +729,11 @@ static void exclude_non_seclabel_mounts()
 	int index = 0, found = 0;
 	char *mount_info[4];
 	char *buf = NULL, *item;
+
 	/* Check to see if the kernel supports seclabel */
 	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
 		return;
+
 	fp = fopen("/proc/mounts", "r");
 	if (!fp)
 		return;
@@ -769,6 +772,8 @@ static void exclude_non_seclabel_mounts()
 		if (!found)
 			add_exclude(mount_info[1]);
 	}
+
+	free(buf);
 }
 
 int main(int argc, char **argv)

-- 
Stephen Smalley
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 related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-07-27 13:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-14 14:15 [PATCH] policycoreutils: get setfiles to skip mounts without seclabel Thomas Liu
2009-07-14 15:30 ` Stephen Smalley
2009-07-14 16:04   ` Thomas Liu
2009-07-14 17:50     ` Daniel J Walsh
2009-07-17 14:48       ` [PATCH -v2] " Thomas Liu
2009-07-24 20:12         ` Stephen Smalley
2009-07-27 13:21           ` 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.