All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] reducing size of libselnux/libsepol for embedded
@ 2007-04-09  4:50 Yuichi Nakamura
  2007-04-09 12:34 ` Joshua Brindle
  0 siblings, 1 reply; 12+ messages in thread
From: Yuichi Nakamura @ 2007-04-09  4:50 UTC (permalink / raw)
  To: selinux; +Cc: ynakam, sds, busybox

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

Hello.

As we discussed before, 
we would like to propose patch that reduces size of libselinux/libsepol.

1. Background
Current libselinux+libsepol size is big for embedded devices.
libselinux.so.1: 115420 byte
libsepol.so.1:302085
Total: 417505 byte
It uses more than 400kbyte.

Needs for embedded device is various, because hardware resource is various.
If device is rich enough, people may want full-featured SELinux.
If device is not rich, people want to reduce binary size by removing 
some features. They may not want modular policy, even may not want boolean.

We thought size of libselinux+libsepol can be reduced by removing functions .

2. About patch
We prepare 2 build options in libselinux/libsepol.
1) make EMBEDDED=1
This is for people who want to use only monolitic policy with boolean support.
* libselinux
  Files related to userland avc is not compiled
* libsepol
  libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
  If people do not need modular policy, we need only to compile such functions.

2) make DISABLE_SEPOL=1
This can be combined with "EMBEDDED=1".
This is for people who uses poor resource devices.
libsepol functions are removed from libselinux 
so libsepol can be removed.
By that, size of libse*  can be reduced more, 
but some features(such as boolean) can not be used.

More detail:
* libselinux 
  - selinux_mk_loadpolicy is replaced with limited version:
     - policy filename is assumed as /etc/selinux/<policy type>/policy/policy.<version in selinuxfs>
     - preserve boolean is not supported
         -- so it is not recommended to use boolean for such system
     - system.users, local.users are not supported
   - -lsepol is removed
* libsepol
  - It is not required by libselinux, so you do not have to compile this.

3. Size measurement
Compiled by gcc(x86)

* Before:
libselinux.so.1: 115420
libsepol.so.1:302085
Total: 417505

* make EMBEDDED=1
libselinux:87024
libsepol:201301
Total: 288325

* make EMBEDDED=1 DISABLE_BOOLEAN=1
libselinux: 77187
libsepol: 0 
Total: 77187 -> Diet about 80%!

And, this work is intended to be applied to BusyBox.
We will fix load_policy applet for BusyBox to support boolean option.

Regards,
-- 
Yuichi Nakamura
Hitachi Software Engineering Co., Ltd.
Japan SELinux Users Group(JSELUG)
SELinux Policy Editor: http://seedit.sourceforge.net/


[-- Attachment #2: libse_diet.v1.patch --]
[-- Type: application/octet-stream, Size: 4349 bytes --]

Index: libsepol/src/Makefile
===================================================================
--- libsepol/src/Makefile	(revision 2318)
+++ libsepol/src/Makefile	(working copy)
@@ -8,11 +8,18 @@
 LIBA=libsepol.a 
 TARGET=libsepol.so
 LIBSO=$(TARGET).$(LIBVERSION)
-OBJS= $(patsubst %.c,%.o,$(wildcard *.c))
-LOBJS= $(patsubst %.c,%.lo,$(wildcard *.c))
+
+SRCS=$(wildcard *.c)
+ifeq ($(EMBEDDED),1)
+UNUSED_SRCS=link.c nodes.c roles.c iface_record.c module.c port_record.c user_record.c interfaces.c  node_record.c ports.c users.c
+SRCS= $(filter-out $(UNUSED_SRCS), $(wildcard *.c))
+endif
+OBJS= $(patsubst %.c,%.o,$(SRCS))
+LOBJS= $(patsubst %.c,%.lo,$(SRCS))
 CFLAGS ?= -Wall -W -Wundef -Wmissing-noreturn -Wmissing-format-attribute
 override CFLAGS += -I. -I../include -D_GNU_SOURCE
 
+
 all: $(LIBA) $(LIBSO)
 
 $(LIBA):  $(OBJS)
Index: libselinux/src/Makefile
===================================================================
--- libselinux/src/Makefile	(revision 2318)
+++ libselinux/src/Makefile	(working copy)
@@ -18,10 +18,27 @@
 SWIGSO=_selinux.so
 SWIGFILES=$(SWIGSO) selinux.py 
 LIBSO=$(TARGET).$(LIBVERSION)
-OBJS= $(patsubst %.c,%.o,$(filter-out $(SWIGCOUT),$(wildcard *.c))) 
-LOBJS= $(patsubst %.c,%.lo,$(filter-out $(SWIGCOUT),$(wildcard *.c)))
+
+LSEPOL=-lsepol
+SRCS=$(filter-out $(SWIGCOUT),$(wildcard *.c))
+ifeq ($(EMBEDDED),1)
+UNUSED_SRCS=avc.c avc_internal.c avc_sidtab.c
+SRCS= $(filter-out $(UNUSED_SRCS), $(filter-out $(SWIGCOUT),$(wildcard *.c)))
+endif
+ifeq ($(DISABLE_SEPOL),1)
+UNUSED_SRCS+=booleans.c
+LSEPOL=
+SRCS= $(filter-out $(UNUSED_SRCS), $(filter-out $(SWIGCOUT),$(wildcard *.c)))
+endif
+
+OBJS= $(patsubst %.c,%.o,$(SRCS))
+LOBJS= $(patsubst %.c,%.lo,$(SRCS))
 CFLAGS ?= -Wall -W -Wundef -Wmissing-noreturn -Wmissing-format-attribute
 override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
+ifeq ($(DISABLE_SEPOL),1)
+override CFLAGS += -DDISABLE_SEPOL
+endif
+
 RANLIB=ranlib
 
 ARCH := $(patsubst i%86,i386,$(shell uname -m))
@@ -48,7 +65,7 @@
 	$(CC) $(LDFLAGS) -shared -o $@ $< -L. -lselinux -L$(LIBDIR) -Wl,-soname,$@
 
 $(LIBSO): $(LOBJS)
-	$(CC) $(LDFLAGS) -shared -o $@ $^ -ldl -lsepol -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
+	$(CC) $(LDFLAGS) -shared -o $@ $^ -ldl $(LSEPOL) -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
 	ln -sf $@ $(TARGET) 
 
 %.o:  %.c policy.h
Index: libselinux/src/load_policy.c
===================================================================
--- libselinux/src/load_policy.c	(revision 2318)
+++ libselinux/src/load_policy.c	(working copy)
@@ -41,7 +41,56 @@
 
 int load_setlocaldefs hidden = 1;
 
-int selinux_mkload_policy(int preservebools)
+/* 
+  This function is used only if DISABLE_SEPOL is defined.
+  Size of libsepol is big, so you may want to disable libsepol for embedded devices.
+  This function is selinux_mkload_policy with limitations.
+  Limitations:
+  - Binary policy file name is assumed as "policy.<value in /selinux/policyvers>".
+  - Preserve boolean is not supported, so it is recommended not to use boolean,
+    if you want to disable sepol.
+  - system.users and local.users are not supported.
+*/
+static int selinux_mkload_policy_nosepol(int preservebools) {
+	int rc = -1;
+	char path[PATH_MAX];
+	size_t size;
+	void *data;
+	int fd;	
+	struct stat sb;
+
+	if (preservebools) {
+		return -1;
+	}
+
+	snprintf(path, sizeof(path), "%s", selinux_binary_policy_path());
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return -1;
+	
+	if (fstat(fd, &sb) < 0)
+		goto close;
+
+	size = sb.st_size;
+	data = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (data == MAP_FAILED)
+		goto close;
+
+	rc = security_load_policy(data, size);
+
+ close:
+	close(fd);
+	return rc;
+	
+}
+
+#ifndef DISABLE_SEPOL
+/*
+  selinux_mkload_policy with full features.
+  This is used usually(when DISABLE_SEPOL is not defined).
+*/
+static int selinux_mkload_policy_sepol(int preservebools)
 {
 	int vers = sepol_policy_kern_vers_max();
 	int kernvers = security_policyvers();
@@ -154,7 +203,16 @@
 	close(fd);
 	return rc;
 }
+#endif /*ifndef DISABLE_SEPOL*/
 
+int selinux_mkload_policy(int preservebools) {
+#ifdef DISABLE_SEPOL
+	return selinux_mkload_policy_nosepol(preservebools);
+#else
+	return selinux_mkload_policy_sepol(preservebools);
+#endif
+}
+
 hidden_def(selinux_mkload_policy)
 
 /*

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-09  4:50 [patch] reducing size of libselnux/libsepol for embedded Yuichi Nakamura
@ 2007-04-09 12:34 ` Joshua Brindle
  2007-04-09 13:27   ` Stephen Smalley
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Joshua Brindle @ 2007-04-09 12:34 UTC (permalink / raw)
  To: Yuichi Nakamura; +Cc: selinux, sds, busybox

Yuichi Nakamura wrote:
> <snip>
In the future could you please inline the patch?
> 2. About patch
> We prepare 2 build options in libselinux/libsepol.
> 1) make EMBEDDED=1
> This is for people who want to use only monolitic policy with boolean support.
> * libselinux
>   Files related to userland avc is not compiled
> * libsepol
>   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
>   If people do not need modular policy, we need only to compile such functions.
>
> 2) make DISABLE_SEPOL=1
> This can be combined with "EMBEDDED=1".
> This is for people who uses poor resource devices.
> libsepol functions are removed from libselinux 
> so libsepol can be removed.
> By that, size of libse*  can be reduced more, 
> but some features(such as boolean) can not be used.
>
>   
I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
etc.

Is there ever a reason to use DISABLE_SEPOL without EMBEDDED? it seems 
like you leave the option open but never describe why it would be used 
or what the side effects would be.

> More detail:
> * libselinux 
>   - selinux_mk_loadpolicy is replaced with limited version:
>      - policy filename is assumed as /etc/selinux/<policy type>/policy/policy.<version in selinuxfs>
>   
This won't reduce the code much, is there a reason that you don't try to 
search for the policy file at all? Granted this shouldn't be an issue in 
embedded environments since you should have control of the kernel and 
policy at all times but it could be useful to load a policy of differing 
version than the kernel supports.

OTOH this could be a good time to get rid of the .version convention 
anyway, there is little reason for it anymore and it causes stale 
policies to be left around when libsepol is upgraded.
>      - preserve boolean is not supported
>          -- so it is not recommended to use boolean for such system
>      - system.users, local.users are not supported
>    - -lsepol is removed
> * libsepol
>   - It is not required by libselinux, so you do not have to compile this.
>
> 3. Size measurement
> Compiled by gcc(x86)
>
> * Before:
> libselinux.so.1: 115420
> libsepol.so.1:302085
> Total: 417505
>
> * make EMBEDDED=1
> libselinux:87024
> libsepol:201301
> Total: 288325
>
> * make EMBEDDED=1 DISABLE_BOOLEAN=1
> libselinux: 77187
> libsepol: 0 
> Total: 77187 -> Diet about 80%!
>
> And, this work is intended to be applied to BusyBox.
> We will fix load_policy applet for BusyBox to support boolean option.
>
>   

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-09 12:34 ` Joshua Brindle
@ 2007-04-09 13:27   ` Stephen Smalley
  2007-04-09 14:44   ` Stephen Smalley
  2007-04-10  1:05   ` Yuichi Nakamura
  2 siblings, 0 replies; 12+ messages in thread
From: Stephen Smalley @ 2007-04-09 13:27 UTC (permalink / raw)
  To: Joshua Brindle
  Cc: Yuichi Nakamura, selinux, busybox, Eric Paris, James Morris

On Mon, 2007-04-09 at 08:34 -0400, Joshua Brindle wrote:
> Yuichi Nakamura wrote:
> > <snip>
> In the future could you please inline the patch?
> > 2. About patch
> > We prepare 2 build options in libselinux/libsepol.
> > 1) make EMBEDDED=1
> > This is for people who want to use only monolitic policy with boolean support.
> > * libselinux
> >   Files related to userland avc is not compiled
> > * libsepol
> >   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
> >   If people do not need modular policy, we need only to compile such functions.
> >
> > 2) make DISABLE_SEPOL=1
> > This can be combined with "EMBEDDED=1".
> > This is for people who uses poor resource devices.
> > libsepol functions are removed from libselinux 
> > so libsepol can be removed.
> > By that, size of libse*  can be reduced more, 
> > but some features(such as boolean) can not be used.
> >
> >   
> I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
> etc.
> 
> Is there ever a reason to use DISABLE_SEPOL without EMBEDDED? it seems 
> like you leave the option open but never describe why it would be used 
> or what the side effects would be.
> 
> > More detail:
> > * libselinux 
> >   - selinux_mk_loadpolicy is replaced with limited version:
> >      - policy filename is assumed as /etc/selinux/<policy type>/policy/policy.<version in selinuxfs>
> >   
> This won't reduce the code much, is there a reason that you don't try to 
> search for the policy file at all? Granted this shouldn't be an issue in 
> embedded environments since you should have control of the kernel and 
> policy at all times but it could be useful to load a policy of differing 
> version than the kernel supports.
> 
> OTOH this could be a good time to get rid of the .version convention 
> anyway, there is little reason for it anymore and it causes stale 
> policies to be left around when libsepol is upgraded.

The .version convention and the fallback code path in the (current)
selinux_mkload_policy() function is still useful as a way of handling
the case where the latest policy version cannot be downgraded to the
kernel-supported version.  If we were to ever make a non-backward
compatible change in the format or the flask definitions (e.g. major
rewrite of initial_sids, security_classes, and access_vectors to purge
obsolete definitions and optimize the layout), then we'd still want that
support.  I'm not sure we can definitively rule that out forever.

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-09 12:34 ` Joshua Brindle
  2007-04-09 13:27   ` Stephen Smalley
@ 2007-04-09 14:44   ` Stephen Smalley
  2007-04-10  1:09     ` Yuichi Nakamura
  2007-04-10  1:05   ` Yuichi Nakamura
  2 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2007-04-09 14:44 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Yuichi Nakamura, selinux, busybox

On Mon, 2007-04-09 at 08:34 -0400, Joshua Brindle wrote:
> Yuichi Nakamura wrote:
> > <snip>
> In the future could you please inline the patch?
> > 2. About patch
> > We prepare 2 build options in libselinux/libsepol.
> > 1) make EMBEDDED=1
> > This is for people who want to use only monolitic policy with boolean support.
> > * libselinux
> >   Files related to userland avc is not compiled
> > * libsepol
> >   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
> >   If people do not need modular policy, we need only to compile such functions.
> >
> > 2) make DISABLE_SEPOL=1
> > This can be combined with "EMBEDDED=1".
> > This is for people who uses poor resource devices.
> > libsepol functions are removed from libselinux 
> > so libsepol can be removed.
> > By that, size of libse*  can be reduced more, 
> > but some features(such as boolean) can not be used.
> >
> >   
> I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
> etc.

Agreed.  Look to the newrole Makefile for an example, where they defined
AUDIT_LOG_PRIV and NAMESPACE_PRIV flags for enabling/disabling specific
features, and then defined a LSPP_PRIV flag that turned them both on.
Thus, you can create AVC=y/n, BOOLEAN=y/n flags for individual feature
selection, and then define EMBEDDED as turning them all off.

This also avoids confusion, as your DISABLE_SEPOL flag turns off
booleans.c presently, but booleans.c doesn't appear to depend on
libsepol.

Other comments:
- I think you can simplify your src/load_policy.c changes, as you only
ever need to define one version of the function (we don't need the
_nosepol static function around at all when using libsepol).  So it just
becomes something like:
#ifdef USE_SEPOL
int selinux_mkload_policy(int preservebools)
{
<your logic>
}
#else
int selinux_mkload_policy(int preservebools)
{
<original logic>
}
#endif
- Your logic for the nosepol case doesn't actually include the policy
version in the pathname at all.  An oversight?
- You don't munmap the policy file on the exit path.
- Coding Style for functions puts the opening bracket on its own line.
- Avoid #ifdefs within function bodies.  Obsoleted if you take the
approach above.

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-09 12:34 ` Joshua Brindle
  2007-04-09 13:27   ` Stephen Smalley
  2007-04-09 14:44   ` Stephen Smalley
@ 2007-04-10  1:05   ` Yuichi Nakamura
  2 siblings, 0 replies; 12+ messages in thread
From: Yuichi Nakamura @ 2007-04-10  1:05 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: ynakam, selinux, sds, busybox

On Mon, 09 Apr 2007 08:34:40 -0400
Joshua Brindle  wrote:
> Yuichi Nakamura wrote:
> > <snip>
> In the future could you please inline the patch?
> > 2. About patch
> > We prepare 2 build options in libselinux/libsepol.
> > 1) make EMBEDDED=1
> > This is for people who want to use only monolitic policy with boolean support.
> > * libselinux
> >   Files related to userland avc is not compiled
> > * libsepol
> >   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
> >   If people do not need modular policy, we need only to compile such functions.
> >
> > 2) make DISABLE_SEPOL=1
> > This can be combined with "EMBEDDED=1".
> > This is for people who uses poor resource devices.
> > libsepol functions are removed from libselinux 
> > so libsepol can be removed.
> > By that, size of libse*  can be reduced more, 
> > but some features(such as boolean) can not be used.
> >
> >   
> I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
> etc.
> 
> Is there ever a reason to use DISABLE_SEPOL without EMBEDDED? it seems 
> like you leave the option open but never describe why it would be used 
> or what the side effects would be.
Thanks for comments.
I agree, I will fix in future patch.

> > More detail:
> > * libselinux 
> >   - selinux_mk_loadpolicy is replaced with limited version:
> >      - policy filename is assumed as /etc/selinux/<policy type>/policy/policy.<version in selinuxfs>
> >   
> This won't reduce the code much, is there a reason that you don't try to 
> search for the policy file at all? Granted this shouldn't be an issue in 
> embedded environments since you should have control of the kernel and 
> policy at all times but it could be useful to load a policy of differing 
> version than the kernel supports.
It is because I wanted to remove sepol functions.
In selinux_mkload_policy, to search policy, 
policy version is searched from sepol_policy_kern_vers_max() to sepol_policy_kern_vers_min().

Another way to remove sepol_policy_kern_vers* is, 
- #include <sepol/policydb/policydb.h> (but no -lsepol)
- search version from POLICYDB_VERSION_MAX to POLICYDB_VERSION_MIN.
Do you think it is reasonable ? 

> 
> OTOH this could be a good time to get rid of the .version convention 
> anyway, there is little reason for it anymore and it causes stale 
> policies to be left around when libsepol is upgraded.
> >      - preserve boolean is not supported
> >          -- so it is not recommended to use boolean for such system
> >      - system.users, local.users are not supported
> >    - -lsepol is removed
> > * libsepol
> >   - It is not required by libselinux, so you do not have to compile this.
> >
> > 3. Size measurement
> > Compiled by gcc(x86)
> >
> > * Before:
> > libselinux.so.1: 115420
> > libsepol.so.1:302085
> > Total: 417505
> >
> > * make EMBEDDED=1
> > libselinux:87024
> > libsepol:201301
> > Total: 288325
> >
> > * make EMBEDDED=1 DISABLE_BOOLEAN=1
> > libselinux: 77187
> > libsepol: 0 
> > Total: 77187 -> Diet about 80%!
> >
> > And, this work is intended to be applied to BusyBox.
> > We will fix load_policy applet for BusyBox to support boolean option.
> >
> >   
> 
> --
> 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.


-- 
Yuichi Nakamura
Hitachi Software Engineering Co., Ltd.
Japan SELinux Users Group(JSELUG)
SELinux Policy Editor: http://seedit.sourceforge.net/




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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-09 14:44   ` Stephen Smalley
@ 2007-04-10  1:09     ` Yuichi Nakamura
  2007-04-10 15:27       ` Joshua Brindle
  0 siblings, 1 reply; 12+ messages in thread
From: Yuichi Nakamura @ 2007-04-10  1:09 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: ynakam, Joshua Brindle, selinux, busybox

On Mon, 09 Apr 2007 10:44:41 -0400
Stephen Smalley  wrote:
> On Mon, 2007-04-09 at 08:34 -0400, Joshua Brindle wrote:
> > Yuichi Nakamura wrote:
> > > <snip>
> > In the future could you please inline the patch?
> > > 2. About patch
> > > We prepare 2 build options in libselinux/libsepol.
> > > 1) make EMBEDDED=1
> > > This is for people who want to use only monolitic policy with boolean support.
> > > * libselinux
> > >   Files related to userland avc is not compiled
> > > * libsepol
> > >   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
> > >   If people do not need modular policy, we need only to compile such functions.
> > >
> > > 2) make DISABLE_SEPOL=1
> > > This can be combined with "EMBEDDED=1".
> > > This is for people who uses poor resource devices.
> > > libsepol functions are removed from libselinux 
> > > so libsepol can be removed.
> > > By that, size of libse*  can be reduced more, 
> > > but some features(such as boolean) can not be used.
> > >
> > >   
> > I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
> > etc.
> 
> Agreed.  Look to the newrole Makefile for an example, where they defined
> AUDIT_LOG_PRIV and NAMESPACE_PRIV flags for enabling/disabling specific
> features, and then defined a LSPP_PRIV flag that turned them both on.
> Thus, you can create AVC=y/n, BOOLEAN=y/n flags for individual feature
> selection, and then define EMBEDDED as turning them all off.
> 
> This also avoids confusion, as your DISABLE_SEPOL flag turns off
> booleans.c presently, but booleans.c doesn't appear to depend on
> libsepol.
Thanks for advice, I will fix like above.
 
> Other comments:
> - I think you can simplify your src/load_policy.c changes, as you only
> ever need to define one version of the function (we don't need the
> _nosepol static function around at all when using libsepol).  So it just
> becomes something like:
> #ifdef USE_SEPOL
> int selinux_mkload_policy(int preservebools)
> {
> <your logic>
> }
> #else
> int selinux_mkload_policy(int preservebools)
> {
> <original logic>
> }
> #endif
> - Your logic for the nosepol case doesn't actually include the policy
> version in the pathname at all.  An oversight?
Oops, it was only mistake.

> - You don't munmap the policy file on the exit path.
> - Coding Style for functions puts the opening bracket on its own line.
> - Avoid #ifdefs within function bodies.  Obsoleted if you take the
> approach above.
I see, I will fix them in next version.

> 
> -- 
> Stephen Smalley
> National Security Agency
> 


-- 
Yuichi Nakamura
Hitachi Software Engineering Co., Ltd.
Japan SELinux Users Group(JSELUG)
SELinux Policy Editor: http://seedit.sourceforge.net/


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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-10  1:09     ` Yuichi Nakamura
@ 2007-04-10 15:27       ` Joshua Brindle
  2007-04-10 16:50         ` Stephen Smalley
  0 siblings, 1 reply; 12+ messages in thread
From: Joshua Brindle @ 2007-04-10 15:27 UTC (permalink / raw)
  To: Yuichi Nakamura; +Cc: Stephen Smalley, selinux, busybox

Yuichi Nakamura wrote:
> On Mon, 09 Apr 2007 10:44:41 -0400
> Stephen Smalley  wrote:
>   
>> On Mon, 2007-04-09 at 08:34 -0400, Joshua Brindle wrote:
>>     
>>> Yuichi Nakamura wrote:
>>>       
>>>> <snip>
>>>>         
>>> In the future could you please inline the patch?
>>>       
>>>> 2. About patch
>>>> We prepare 2 build options in libselinux/libsepol.
>>>> 1) make EMBEDDED=1
>>>> This is for people who want to use only monolitic policy with boolean support.
>>>> * libselinux
>>>>   Files related to userland avc is not compiled
>>>> * libsepol
>>>>   libselinux uses sepol functions(such as sepol_genusers, sepol_genboolean). 
>>>>   If people do not need modular policy, we need only to compile such functions.
>>>>
>>>> 2) make DISABLE_SEPOL=1
>>>> This can be combined with "EMBEDDED=1".
>>>> This is for people who uses poor resource devices.
>>>> libsepol functions are removed from libselinux 
>>>> so libsepol can be removed.
>>>> By that, size of libse*  can be reduced more, 
>>>> but some features(such as boolean) can not be used.
>>>>
>>>>   
>>>>         
>>> I think the defines should be more descriptive, perhaps NO_AVC, NO_BOOL, 
>>> etc.
>>>       
>> Agreed.  Look to the newrole Makefile for an example, where they defined
>> AUDIT_LOG_PRIV and NAMESPACE_PRIV flags for enabling/disabling specific
>> features, and then defined a LSPP_PRIV flag that turned them both on.
>> Thus, you can create AVC=y/n, BOOLEAN=y/n flags for individual feature
>> selection, and then define EMBEDDED as turning them all off.
>>
>> This also avoids confusion, as your DISABLE_SEPOL flag turns off
>> booleans.c presently, but booleans.c doesn't appear to depend on
>> libsepol.
>>     
> Thanks for advice, I will fix like above.
>  
>   

Looking in the patch the libsepol unused sources has this:

+UNUSED_SRCS=link.c nodes.c roles.c iface_record.c module.c 
port_record.c user_record.c interfaces.c  node_record.c
 ports.c users.c

Why is link.c unused but not expand.c? Note that this will build a 
libsepol that checkpolicy cannot link against which is fine for an 
embedded environment but it needs to be explicitly stated I think. The 
options are basically going to be to turn off module support, boolean 
support, compiler support. The security server (services.c) can be 
removed also. If compiler and persistent boolean support isn't necessary 
anymore then write.c and read.c won't be necessary either.

After all that is removed I feel like libsepol won't actually exist, I'm 
not sure what the use case is right now, what are you trying to get out 
of libsepol and how fine grained can we get on taking out support.


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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-10 15:27       ` Joshua Brindle
@ 2007-04-10 16:50         ` Stephen Smalley
  2007-04-10 19:31           ` Stephen Smalley
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2007-04-10 16:50 UTC (permalink / raw)
  To: Joshua Brindle; +Cc: Yuichi Nakamura, selinux, busybox

On Tue, 2007-04-10 at 11:27 -0400, Joshua Brindle wrote:
> Looking in the patch the libsepol unused sources has this:
> 
> +UNUSED_SRCS=link.c nodes.c roles.c iface_record.c module.c 
> port_record.c user_record.c interfaces.c  node_record.c
>  ports.c users.c
> 
> Why is link.c unused but not expand.c? Note that this will build a 
> libsepol that checkpolicy cannot link against which is fine for an 
> embedded environment but it needs to be explicitly stated I think. The 
> options are basically going to be to turn off module support, boolean 
> support, compiler support. The security server (services.c) can be 
> removed also. If compiler and persistent boolean support isn't necessary 
> anymore then write.c and read.c won't be necessary either.
> 
> After all that is removed I feel like libsepol won't actually exist, I'm 
> not sure what the use case is right now, what are you trying to get out 
> of libsepol and how fine grained can we get on taking out support.

Possibly the patch should selectively filter out objects from LOBJS
(i.e. the shared library) while leaving them all in OBJS (i.e. the
static library).  Then you could build a smaller shared libsepol.so
library for installation on the embedded device that would only be used
for e.g. boolean preservation, while still having the full static
libsepol.a library on the build host for compiling checkpolicy and other
users of the static library.  I think they are just trying to allow for
boolean preservation without pulling in all of libsepol.

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-10 16:50         ` Stephen Smalley
@ 2007-04-10 19:31           ` Stephen Smalley
  2007-04-10 19:47             ` Karl MacMillan
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2007-04-10 19:31 UTC (permalink / raw)
  To: Joshua Brindle
  Cc: Yuichi Nakamura, selinux, busybox, Karl MacMillan, Eric Paris

On Tue, 2007-04-10 at 12:50 -0400, Stephen Smalley wrote:
> On Tue, 2007-04-10 at 11:27 -0400, Joshua Brindle wrote:
> > Looking in the patch the libsepol unused sources has this:
> > 
> > +UNUSED_SRCS=link.c nodes.c roles.c iface_record.c module.c 
> > port_record.c user_record.c interfaces.c  node_record.c
> >  ports.c users.c
> > 
> > Why is link.c unused but not expand.c? Note that this will build a 
> > libsepol that checkpolicy cannot link against which is fine for an 
> > embedded environment but it needs to be explicitly stated I think. The 
> > options are basically going to be to turn off module support, boolean 
> > support, compiler support. The security server (services.c) can be 
> > removed also. If compiler and persistent boolean support isn't necessary 
> > anymore then write.c and read.c won't be necessary either.
> > 
> > After all that is removed I feel like libsepol won't actually exist, I'm 
> > not sure what the use case is right now, what are you trying to get out 
> > of libsepol and how fine grained can we get on taking out support.
> 
> Possibly the patch should selectively filter out objects from LOBJS
> (i.e. the shared library) while leaving them all in OBJS (i.e. the
> static library).  Then you could build a smaller shared libsepol.so
> library for installation on the embedded device that would only be used
> for e.g. boolean preservation, while still having the full static
> libsepol.a library on the build host for compiling checkpolicy and other
> users of the static library.  I think they are just trying to allow for
> boolean preservation without pulling in all of libsepol.

Another option would be to "fix" the kernel to preserve booleans
atomically across policy reloads.  Which would eliminate the need for
sepol_genbools_array altogether at policy load, and solve some other
problems, e.g. try running load_policy in a loop and then start another
loop that also runs load_policy, and they'll collide pretty fast (one of
them will end up trying to read an invalidated /selinux/booleans node
from the other's reload).

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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-10 19:31           ` Stephen Smalley
@ 2007-04-10 19:47             ` Karl MacMillan
  2007-04-11  0:22               ` Joshua Brindle
  0 siblings, 1 reply; 12+ messages in thread
From: Karl MacMillan @ 2007-04-10 19:47 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Joshua Brindle, Yuichi Nakamura, selinux, busybox, Eric Paris

On Tue, 2007-04-10 at 15:31 -0400, Stephen Smalley wrote:
> On Tue, 2007-04-10 at 12:50 -0400, Stephen Smalley wrote:

> > 
> > Possibly the patch should selectively filter out objects from LOBJS
> > (i.e. the shared library) while leaving them all in OBJS (i.e. the
> > static library).  Then you could build a smaller shared libsepol.so
> > library for installation on the embedded device that would only be used
> > for e.g. boolean preservation, while still having the full static
> > libsepol.a library on the build host for compiling checkpolicy and other
> > users of the static library.  I think they are just trying to allow for
> > boolean preservation without pulling in all of libsepol.
> 
> Another option would be to "fix" the kernel to preserve booleans
> atomically across policy reloads.  Which would eliminate the need for
> sepol_genbools_array altogether at policy load, and solve some other
> problems, e.g. try running load_policy in a loop and then start another
> loop that also runs load_policy, and they'll collide pretty fast (one of
> them will end up trying to read an invalidated /selinux/booleans node
> from the other's reload).
> 

I like this idea. Of course, it would mean that permanent boolean
changes would either require managed policy or a policy recompile, which
is likely acceptable.

KArl


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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-10 19:47             ` Karl MacMillan
@ 2007-04-11  0:22               ` Joshua Brindle
  2007-04-11 15:29                 ` Yuichi Nakamura
  0 siblings, 1 reply; 12+ messages in thread
From: Joshua Brindle @ 2007-04-11  0:22 UTC (permalink / raw)
  To: Karl MacMillan
  Cc: Stephen Smalley, Yuichi Nakamura, selinux, busybox, Eric Paris

Karl MacMillan wrote:
> On Tue, 2007-04-10 at 15:31 -0400, Stephen Smalley wrote:
>   
>> On Tue, 2007-04-10 at 12:50 -0400, Stephen Smalley wrote:
>>     
>
>   
>>> Possibly the patch should selectively filter out objects from LOBJS
>>> (i.e. the shared library) while leaving them all in OBJS (i.e. the
>>> static library).  Then you could build a smaller shared libsepol.so
>>> library for installation on the embedded device that would only be used
>>> for e.g. boolean preservation, while still having the full static
>>> libsepol.a library on the build host for compiling checkpolicy and other
>>> users of the static library.  I think they are just trying to allow for
>>> boolean preservation without pulling in all of libsepol.
>>>       
Even given the below fix I like this idea, since the static lib need not 
be present on any production device this should solve the embedded size. 
For example, even on a managed device there may be no need for the 
security server code. One thing that worries me about having so many 
configurations of the library is that when bug reports come in it may be 
difficult to find out how the library was built. Perhaps we should add 
build information into the library that can be printed out with sestatus?

>> Another option would be to "fix" the kernel to preserve booleans
>> atomically across policy reloads.  Which would eliminate the need for
>> sepol_genbools_array altogether at policy load, and solve some other
>> problems, e.g. try running load_policy in a loop and then start another
>> loop that also runs load_policy, and they'll collide pretty fast (one of
>> them will end up trying to read an invalidated /selinux/booleans node
>> from the other's reload).
>>
>>     
>
> I like this idea. Of course, it would mean that permanent boolean
> changes would either require managed policy or a policy recompile, which
> is likely acceptable.
>
>   

I like it too. Permanent boolean changes already require managed policy 
or a policy recompile in trunk (due to removal of setlocaldefs support) 
so there is a good chance that noone will notice :)

Also, AFAIK libselinux only depends on libsepol for the boolean stuff so 
libsepol will no longer be necessary on unmanaged machines with no 
policy compiler, this is good.


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

* Re: [patch] reducing size of libselnux/libsepol for embedded
  2007-04-11  0:22               ` Joshua Brindle
@ 2007-04-11 15:29                 ` Yuichi Nakamura
  0 siblings, 0 replies; 12+ messages in thread
From: Yuichi Nakamura @ 2007-04-11 15:29 UTC (permalink / raw)
  To: Joshua Brindle
  Cc: himainu-ynakam, kmacmillan, sds, ynakam, selinux, busybox, eparis

On Tue, 10 Apr 2007 20:22:11 -0400
Joshua Brindle  wrote:
> For example, even on a managed device there may be no need for the 
> security server code. One thing that worries me about having so many 
> configurations of the library is that when bug reports come in it may be 
> difficult to find out how the library was built. Perhaps we should add 
> build information into the library that can be printed out with sestatus?
I see, new function such as libselinux_buildinfo will be prepared in libselinux, 
and sestatus will use that.
I will try to add that one in next patch.

> >> Another option would be to "fix" the kernel to preserve booleans
> >> atomically across policy reloads.  Which would eliminate the need for
> >> sepol_genbools_array altogether at policy load, and solve some other
> >> problems, e.g. try running load_policy in a loop and then start another
> >> loop that also runs load_policy, and they'll collide pretty fast (one of
> >> them will end up trying to read an invalidated /selinux/booleans node
> >> from the other's reload).
> > I like this idea. Of course, it would mean that permanent boolean
> > changes would either require managed policy or a policy recompile, which
> > is likely acceptable.
> I like it too. Permanent boolean changes already require managed policy 
> or a policy recompile in trunk (due to removal of setlocaldefs support) 
> so there is a good chance that noone will notice :)
Me too.

I wanted only sepol_genbools_array in libsepol, 
but it was hard to pick up least set of functions, 
so many files were remaining.

> Also, AFAIK libselinux only depends on libsepol for the boolean stuff so 
> libsepol will no longer be necessary on unmanaged machines with no 
> policy compiler, this is good.
Sounds fine.
I will prepare patch for only libselinux next time.
#Please wait for updated patch..

Regards,
Yuichi Nakamura


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

end of thread, other threads:[~2007-04-11 15:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-09  4:50 [patch] reducing size of libselnux/libsepol for embedded Yuichi Nakamura
2007-04-09 12:34 ` Joshua Brindle
2007-04-09 13:27   ` Stephen Smalley
2007-04-09 14:44   ` Stephen Smalley
2007-04-10  1:09     ` Yuichi Nakamura
2007-04-10 15:27       ` Joshua Brindle
2007-04-10 16:50         ` Stephen Smalley
2007-04-10 19:31           ` Stephen Smalley
2007-04-10 19:47             ` Karl MacMillan
2007-04-11  0:22               ` Joshua Brindle
2007-04-11 15:29                 ` Yuichi Nakamura
2007-04-10  1:05   ` Yuichi Nakamura

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.