Re: [PATCH] libselinux: Add support for pcre2 to pkgconfig definition
On Wed, 2017-10-11 at 10:53 +0200, Petr Lautrbach wrote: > When libselinux is built using USE_PCRE2 libselinux.pc needs to > require > libpcre2-8 instead of libpcre. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1487521 > > Signed-off-by: Petr Lautrbach Thanks, applied. We still need to revisit the value proposition of file_contexts.bin after the move to pcre2, given the large increase in file size and the runtime overhead. We can add -r to the sefcontext_compile args via semanage.conf, but then I'm wondering whether it is worth having file_contexts.bin at all. > --- > libselinux/Makefile | 11 ++- > libselinux/src/Makefile | 2 +- > libselinux/src/libselinux.pc.in | 2 +- > 3 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/libselinux/Makefile b/libselinux/Makefile > index 1ecab178..16531fe9 100644 > --- a/libselinux/Makefile > +++ b/libselinux/Makefile > @@ -21,13 +21,14 @@ export DISABLE_SETRANS DISABLE_RPM DISABLE_FLAGS > ANDROID_HOST > > USE_PCRE2 ?= n > ifeq ($(USE_PCRE2),y) > - PCRE_CFLAGS := -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8 $(shell > $(PKG_CONFIG) --cflags libpcre2-8) > - PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs libpcre2-8) > + PCRE_MODULE := libpcre2-8 > + PCRE_CFLAGS := -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8 > else > - PCRE_CFLAGS := $(shell $(PKG_CONFIG) --cflags libpcre) > - PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs libpcre) > + PCRE_MODULE := libpcre > endif > -export PCRE_CFLAGS PCRE_LDLIBS > +PCRE_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(PCRE_MODULE)) > +PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs $(PCRE_MODULE)) > +export PCRE_MODULE PCRE_CFLAGS PCRE_LDLIBS > > OS := $(shell uname) > export OS > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index 2408faea..18df75c8 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -148,7 +148,7 @@ $(LIBSO): $(LOBJS) > ln -sf $@ $(TARGET) > > $(LIBPC): $(LIBPC).in ../VERSION > - sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; > s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ > + sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; > s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):; > s:@PCRE_MODULE@:$(PCRE_MODULE):' < $< > $@ > > selinuxswig_python_exception.i: ../include/selinux/selinux.h > bash -e exception.sh > $@ || (rm -f $@ ; false) > diff --git a/libselinux/src/libselinux.pc.in > b/libselinux/src/libselinux.pc.in > index 2cd04d38..2e90a844 100644 > --- a/libselinux/src/libselinux.pc.in > +++ b/libselinux/src/libselinux.pc.in > @@ -7,6 +7,6 @@ Name: libselinux > Description: SELinux utility library > Version: @VERSION@ > URL: http://userspace.selinuxproject.org/ > -Requires.private: libsepol libpcre > +Requires.private: libsepol @PCRE_MODULE@ > Libs: -L${libdir} -lselinux > Cflags: -I${includedir}
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 8:06 PM, Stephen Smalley wrote: > On 09/16/2016 02:57 PM, Janis Danisevskis wrote: > > I have started implementing an arch string patch. Unfortunately, i did > > not manage to finish it before I had to leave the office today. > > In essence I did this: > > The regex_arch_string has three components: the pointer width determined > > by sizeof(void*), PCRE2_SIZE width determined by sizeof(), and > > endianess determined by > > __BYTE_ORDER__==__ORDER_BIG/LITTEL_ENDIAN__ > > > > For example, the resulting string for x86_64 and aarch64el should look > like > > this: "8-8-el". > > > > I bumped the compiled context version number and added the string > > right after the version in the output. > > Comments? > > What's the error handling when the versions do not match? Just ignore > the compiled regexes in file_contexts.bin? > > > Yes, If the version and arch match attempt to load the compiled regexes from the file. Otherwise, skip the corresponding regions of the file and leave the regex field NULL, so that they will be compiled on first use. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On 09/16/2016 02:57 PM, Janis Danisevskis wrote: > I have started implementing an arch string patch. Unfortunately, i did > not manage to finish it before I had to leave the office today. > In essence I did this: > The regex_arch_string has three components: the pointer width determined > by sizeof(void*), PCRE2_SIZE width determined by sizeof(), and > endianess determined by > __BYTE_ORDER__==__ORDER_BIG/LITTEL_ENDIAN__ > > For example, the resulting string for x86_64 and aarch64el should look like > this: "8-8-el". > > I bumped the compiled context version number and added the string > right after the version in the output. > Comments? What's the error handling when the versions do not match? Just ignore the compiled regexes in file_contexts.bin? ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
I have started implementing an arch string patch. Unfortunately, i did not manage to finish it before I had to leave the office today. In essence I did this: The regex_arch_string has three components: the pointer width determined by sizeof(void*), PCRE2_SIZE width determined by sizeof(), and endianess determined by __BYTE_ORDER__==__ORDER_BIG/LITTEL_ENDIAN__ For example, the resulting string for x86_64 and aarch64el should look like this: "8-8-el". I bumped the compiled context version number and added the string right after the version in the output. Comments? On Fri, Sep 16, 2016 at 3:52 PM, Stephen Smalley wrote: > On 09/16/2016 09:31 AM, Jason Zaman wrote: > > On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: > >> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis > wrote: > >>> I don't mind. Then before sefcontext_compile -r gets widely adapted we > >>> should change the semantic quickly. I'll prepare a patch. > >> > >> Did I miss something and this was merged? Iv'e been out recovering > >> from a surgery so I haven't been > >> following this as well as I normally would have, > >> > >> If its merged, just leave it. > > > > Its the very latest thing in master yeah, but I do also agree with > changing it. > > > > I just wanted to add that from a distro perspective, compiling things by > > default makes more sense. In gentoo, the package post_install runs > > sefcontext_compile. Using the fcontext files happens a lot more than any > > updates to libselinux (and thus potential format changes) so I'm pretty > > sure most people would prefer to have the speedup. > > > > Gentoo does it on the machine itself, I am not sure about redhat or > > debian but I wouldnt be surprised if they do it per-arch at the very > > least so cross-arch probably isnt an issue. > > In Red Hat, SELinux policy is noarch, and they switched to precompiling > both policy and file_contexts.bin at build time to minimize the cost at > package install time. Otherwise, in small VMs, they had issues with > running out of memory during semodule -B. So file_contexts.bin > presently has to be arch-independent, or we need the arch properties > detection logic and fallback. That said, none of this matters unless > you build with USE_PCRE2=y, and no one outside of Android is doing that > today. > > > Also, I think we should add the arch to the version string stored. I > > would rather have false negatives than positives especially since we are > > not 100% sure exactly what part of the arch is important. We can always > > loosen it up later if that gets locked down. > > We don't want the arch string itself, because that would invalidate use > of file_contexts.bin entirely on typical Android use cases (build on > x86_64, install to ARM), but only the relevant properties. And for > Android, that is fatal - there is no file_contexts text file on which to > fallback anymore. They only ship file_contexts.bin. > > ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On 09/16/2016 09:31 AM, Jason Zaman wrote: > On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: >> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis wrote: >>> I don't mind. Then before sefcontext_compile -r gets widely adapted we >>> should change the semantic quickly. I'll prepare a patch. >> >> Did I miss something and this was merged? Iv'e been out recovering >> from a surgery so I haven't been >> following this as well as I normally would have, >> >> If its merged, just leave it. > > Its the very latest thing in master yeah, but I do also agree with changing > it. > > I just wanted to add that from a distro perspective, compiling things by > default makes more sense. In gentoo, the package post_install runs > sefcontext_compile. Using the fcontext files happens a lot more than any > updates to libselinux (and thus potential format changes) so I'm pretty > sure most people would prefer to have the speedup. > > Gentoo does it on the machine itself, I am not sure about redhat or > debian but I wouldnt be surprised if they do it per-arch at the very > least so cross-arch probably isnt an issue. In Red Hat, SELinux policy is noarch, and they switched to precompiling both policy and file_contexts.bin at build time to minimize the cost at package install time. Otherwise, in small VMs, they had issues with running out of memory during semodule -B. So file_contexts.bin presently has to be arch-independent, or we need the arch properties detection logic and fallback. That said, none of this matters unless you build with USE_PCRE2=y, and no one outside of Android is doing that today. > Also, I think we should add the arch to the version string stored. I > would rather have false negatives than positives especially since we are > not 100% sure exactly what part of the arch is important. We can always > loosen it up later if that gets locked down. We don't want the arch string itself, because that would invalidate use of file_contexts.bin entirely on typical Android use cases (build on x86_64, install to ARM), but only the relevant properties. And for Android, that is fatal - there is no file_contexts text file on which to fallback anymore. They only ship file_contexts.bin. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On Sep 16, 2016 07:06, "Jason Zaman" wrote: > > On Fri, Sep 16, 2016 at 06:51:25AM -0700, William Roberts wrote: > > On Fri, Sep 16, 2016 at 6:43 AM, William Roberts > > wrote: > > > On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman wrote: > > >> On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: > > >>> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis < jda...@google.com> wrote: > > >>> > I don't mind. Then before sefcontext_compile -r gets widely adapted we > > >>> > should change the semantic quickly. I'll prepare a patch. > > >>> > > >>> Did I miss something and this was merged? Iv'e been out recovering > > >>> from a surgery so I haven't been > > >>> following this as well as I normally would have, > > >>> > > >>> If its merged, just leave it. > > >> > > >> Its the very latest thing in master yeah, but I do also agree with changing it. > > > > > > I'd prefer it changed myself. Another argument pro-change is that one doesn't > > > know if its a PCRE2 or PCRE1 compatable version of the libraries, we should > > > probably have an ability to intergoate that via API so we can print it > > > out in help > > > dialogue, that this tool so we at least know what it can support. > > > > > > The biggest thing that needs to get fixed with this, is that no matter > > > if it contains the > > > pre-compiled regexs or not, it should always load and work on Android. > > > In distros, > > > it will fall back to file_contexts, but we don't have this in Android. > > > This ties into the arch > > > version information below. But if the arch differs, always recompile. > > > The alternative to > > > this, is just go back to textual fc file son Android, since we won't > > > be using any of the > > > features of binary fc's. Better yet, in the Android build, I would > > > check to see if host arch > > > is the same as target arch, or let an OEM set a flag, to do the > > > compilation. But I would > > > consider those stretch goals, and just revert android back to textual files. > > > > My ramblings might not be clear here. If we just version it with arch > > and require > > an exact match, we don't need -r, so that option goes away. > > This is what I was aiming for too, currently the pcre_version() is just > strcmp'd so changing it to cat(pcre2_version(), arch_string()) should > avoid any problems. It may turn out to be overzealous but it will always > be safe. I dont think splitting pcre_version and arch into separate > fields makes a huge difference if you prefer that. Currently the version It can make a difference if we ever need to parse any information for some reason in the future. We'd then have to split them back apart. This is just a bit more future proof IMO. > numbers are not parsed tho, so checking arch only for pcre2 seems like a > lot more work. > > -- Jason ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 06:51:25AM -0700, William Roberts wrote: > On Fri, Sep 16, 2016 at 6:43 AM, William Roberts > wrote: > > On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman wrote: > >> On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: > >>> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis > >>> wrote: > >>> > I don't mind. Then before sefcontext_compile -r gets widely adapted we > >>> > should change the semantic quickly. I'll prepare a patch. > >>> > >>> Did I miss something and this was merged? Iv'e been out recovering > >>> from a surgery so I haven't been > >>> following this as well as I normally would have, > >>> > >>> If its merged, just leave it. > >> > >> Its the very latest thing in master yeah, but I do also agree with > >> changing it. > > > > I'd prefer it changed myself. Another argument pro-change is that one > > doesn't > > know if its a PCRE2 or PCRE1 compatable version of the libraries, we should > > probably have an ability to intergoate that via API so we can print it > > out in help > > dialogue, that this tool so we at least know what it can support. > > > > The biggest thing that needs to get fixed with this, is that no matter > > if it contains the > > pre-compiled regexs or not, it should always load and work on Android. > > In distros, > > it will fall back to file_contexts, but we don't have this in Android. > > This ties into the arch > > version information below. But if the arch differs, always recompile. > > The alternative to > > this, is just go back to textual fc file son Android, since we won't > > be using any of the > > features of binary fc's. Better yet, in the Android build, I would > > check to see if host arch > > is the same as target arch, or let an OEM set a flag, to do the > > compilation. But I would > > consider those stretch goals, and just revert android back to textual files. > > My ramblings might not be clear here. If we just version it with arch > and require > an exact match, we don't need -r, so that option goes away. This is what I was aiming for too, currently the pcre_version() is just strcmp'd so changing it to cat(pcre2_version(), arch_string()) should avoid any problems. It may turn out to be overzealous but it will always be safe. I dont think splitting pcre_version and arch into separate fields makes a huge difference if you prefer that. Currently the version numbers are not parsed tho, so checking arch only for pcre2 seems like a lot more work. -- Jason ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 6:43 AM, William Roberts wrote: > On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman wrote: >> On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: >>> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis >>> wrote: >>> > I don't mind. Then before sefcontext_compile -r gets widely adapted we >>> > should change the semantic quickly. I'll prepare a patch. >>> >>> Did I miss something and this was merged? Iv'e been out recovering >>> from a surgery so I haven't been >>> following this as well as I normally would have, >>> >>> If its merged, just leave it. >> >> Its the very latest thing in master yeah, but I do also agree with changing >> it. > > I'd prefer it changed myself. Another argument pro-change is that one doesn't > know if its a PCRE2 or PCRE1 compatable version of the libraries, we should > probably have an ability to intergoate that via API so we can print it > out in help > dialogue, that this tool so we at least know what it can support. > > The biggest thing that needs to get fixed with this, is that no matter > if it contains the > pre-compiled regexs or not, it should always load and work on Android. > In distros, > it will fall back to file_contexts, but we don't have this in Android. > This ties into the arch > version information below. But if the arch differs, always recompile. > The alternative to > this, is just go back to textual fc file son Android, since we won't > be using any of the > features of binary fc's. Better yet, in the Android build, I would > check to see if host arch > is the same as target arch, or let an OEM set a flag, to do the > compilation. But I would > consider those stretch goals, and just revert android back to textual files. My ramblings might not be clear here. If we just version it with arch and require an exact match, we don't need -r, so that option goes away. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman wrote: > On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: >> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis wrote: >> > I don't mind. Then before sefcontext_compile -r gets widely adapted we >> > should change the semantic quickly. I'll prepare a patch. >> >> Did I miss something and this was merged? Iv'e been out recovering >> from a surgery so I haven't been >> following this as well as I normally would have, >> >> If its merged, just leave it. > > Its the very latest thing in master yeah, but I do also agree with changing > it. I'd prefer it changed myself. Another argument pro-change is that one doesn't know if its a PCRE2 or PCRE1 compatable version of the libraries, we should probably have an ability to intergoate that via API so we can print it out in help dialogue, that this tool so we at least know what it can support. The biggest thing that needs to get fixed with this, is that no matter if it contains the pre-compiled regexs or not, it should always load and work on Android. In distros, it will fall back to file_contexts, but we don't have this in Android. This ties into the arch version information below. But if the arch differs, always recompile. The alternative to this, is just go back to textual fc file son Android, since we won't be using any of the features of binary fc's. Better yet, in the Android build, I would check to see if host arch is the same as target arch, or let an OEM set a flag, to do the compilation. But I would consider those stretch goals, and just revert android back to textual files. > > I just wanted to add that from a distro perspective, compiling things by > default makes more sense. In gentoo, the package post_install runs > sefcontext_compile. Using the fcontext files happens a lot more than any > updates to libselinux (and thus potential format changes) so I'm pretty > sure most people would prefer to have the speedup. For the distros it will make sense most of the time from what I can tell. > > Gentoo does it on the machine itself, I am not sure about redhat or > debian but I wouldnt be surprised if they do it per-arch at the very > least so cross-arch probably isnt an issue. > > Also, I think we should add the arch to the version string stored. I > would rather have false negatives than positives especially since we are > not 100% sure exactly what part of the arch is important. We can always > loosen it up later if that gets locked down. No don't append it onto the PCRE version string, just check the PCRE version string and if its greater than X read out the arch field next. This should work like policy versioning in libsepol. > > Isnt the selinux policy in android stored in the rootfs or /system? > isnt that all read-only? how would re-compiling on the device work? > Does android put the fcontext.bin files on /data? that seems insecure. This is the cross compile aspect, we build on the host and shove it into the device rootfs at build time. We typically build on x86_64 and deploy on ARM. But this is NOT always the case, just the most common. For instance, with Intel tablets it probably can use the pre-compiled regexes. > > -- Jason > >> > On Fri, Sep 16, 2016 at 1:35 PM William Roberts >> > wrote: >> >> >> >> >> >> > >> >> > >> >> > That's just the thing. Without -r the phone _will_ boot because the >> >> > regexes >> >> > are compiled on first use. With -r and an arch mismatch we have an >> >> > undefined >> >> > behavior, which is bad. >> >> >> >> That's just a limitation of the current design. >> >> >> >> > >> >> > See, I don't currently know what part of the architecture is important. >> >> > Will >> >> > word size and endianess be enough, e.g., will regexes generated on >> >> > x86_64el >> >> > work on armv8el (apparently this is what I observed but it could change >> >> > at >> >> > the whim of the PCRE2 maintainers)? So what will be the encoding? If we >> >> > encode the complete architecture then the typical case for android >> >> > (build on >> >> > x86_64 for armv7/armv8) will yield the same result as without the -r >> >> > option, >> >> > namely, we rebuild the regexes on the device. If we just encode word >> >> > size >> >> > and endianess it may work for a while... I just don't feel comfortable >> >> > enough to make a decision at this point. >> >> >> >> Then shelve it and use textual file contexts. The only purpose of the >> >> binary format is to >> >> include the pre-compiled regex;s so if you cant use that feature, >> >> there's no point in >> >> using binary format. >> >> >> >> My thought would be that it has to be an exact match for architecture >> >> upfront, then >> >> possibly investigate relaxing the requirement later. But, IIRC, if we get >> >> a 30% >> >> increase in text file load time, theirs really no point, for the >> >> binary format on Android. >> >> Android fic files are smaller, and have much simpler regex's compared >> >> to our desktop >> >> brethr
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote: > On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis wrote: > > I don't mind. Then before sefcontext_compile -r gets widely adapted we > > should change the semantic quickly. I'll prepare a patch. > > Did I miss something and this was merged? Iv'e been out recovering > from a surgery so I haven't been > following this as well as I normally would have, > > If its merged, just leave it. Its the very latest thing in master yeah, but I do also agree with changing it. I just wanted to add that from a distro perspective, compiling things by default makes more sense. In gentoo, the package post_install runs sefcontext_compile. Using the fcontext files happens a lot more than any updates to libselinux (and thus potential format changes) so I'm pretty sure most people would prefer to have the speedup. Gentoo does it on the machine itself, I am not sure about redhat or debian but I wouldnt be surprised if they do it per-arch at the very least so cross-arch probably isnt an issue. Also, I think we should add the arch to the version string stored. I would rather have false negatives than positives especially since we are not 100% sure exactly what part of the arch is important. We can always loosen it up later if that gets locked down. Isnt the selinux policy in android stored in the rootfs or /system? isnt that all read-only? how would re-compiling on the device work? Does android put the fcontext.bin files on /data? that seems insecure. -- Jason > > On Fri, Sep 16, 2016 at 1:35 PM William Roberts > > wrote: > >> > >> > >> > > >> > > >> > That's just the thing. Without -r the phone _will_ boot because the > >> > regexes > >> > are compiled on first use. With -r and an arch mismatch we have an > >> > undefined > >> > behavior, which is bad. > >> > >> That's just a limitation of the current design. > >> > >> > > >> > See, I don't currently know what part of the architecture is important. > >> > Will > >> > word size and endianess be enough, e.g., will regexes generated on > >> > x86_64el > >> > work on armv8el (apparently this is what I observed but it could change > >> > at > >> > the whim of the PCRE2 maintainers)? So what will be the encoding? If we > >> > encode the complete architecture then the typical case for android > >> > (build on > >> > x86_64 for armv7/armv8) will yield the same result as without the -r > >> > option, > >> > namely, we rebuild the regexes on the device. If we just encode word > >> > size > >> > and endianess it may work for a while... I just don't feel comfortable > >> > enough to make a decision at this point. > >> > >> Then shelve it and use textual file contexts. The only purpose of the > >> binary format is to > >> include the pre-compiled regex;s so if you cant use that feature, > >> there's no point in > >> using binary format. > >> > >> My thought would be that it has to be an exact match for architecture > >> upfront, then > >> possibly investigate relaxing the requirement later. But, IIRC, if we get > >> a 30% > >> increase in text file load time, theirs really no point, for the > >> binary format on Android. > >> Android fic files are smaller, and have much simpler regex's compared > >> to our desktop > >> brethren. > >> > >> > > >> > My limited understanding is that the automata built by PCRE2 use > >> > pointers to > >> > link states or other structures. These pointers are symbolized when > >> > serialized, but they keep their width, which is why the regexes are at > >> > least > >> > sensitive to the word size. > >> > > >> > Just a wild Idea: > >> > PCRE2 also supports JIT compiling. I did not use this here because I did > >> > not > >> > feel comfortable for libselinux to depend on the execmem permission. But > >> > this could be developed into pre cross compiling the regexes into native > >> > code, which can than be linked into a device specific library. But I > >> > guess > >> > this would require quite some cooperation with the PCRE2 developers. > >> > >> I was going to ask if the arch dependency was on JIT'd code or just > >> something else > >> which you elaborated above with word size/endiness, etc. > >> > >> > > >> >> > >> >> The other thing is, this is only an issue on binary formated > >> >> file_context > >> >> files, > >> >> this is the ideal reason to move back to text files, since their is no > >> >> speedup > >> >> gained if we have to compile them. > >> > > >> > > >> > Right, or you could see it as an intermediate solution that does not > >> > require > >> > changes to the build system until we can properly cross compile the > >> > regexes. > >> > >> If you add an option to sefcontext_compile it should be to remove > >> them. not add it in. > >> This keeps it consistent with teh behavior for PCRE, it would be add > >> to say, "make me > >> a binary fc, but don't actually embed the regex information", since > >> that is currently not > >> the default behavior. Changing the Android make
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis wrote: > I don't mind. Then before sefcontext_compile -r gets widely adapted we > should change the semantic quickly. I'll prepare a patch. Did I miss something and this was merged? Iv'e been out recovering from a surgery so I haven't been following this as well as I normally would have, If its merged, just leave it. > > On Fri, Sep 16, 2016 at 1:35 PM William Roberts > wrote: >> >> >> > >> > >> > That's just the thing. Without -r the phone _will_ boot because the >> > regexes >> > are compiled on first use. With -r and an arch mismatch we have an >> > undefined >> > behavior, which is bad. >> >> That's just a limitation of the current design. >> >> > >> > See, I don't currently know what part of the architecture is important. >> > Will >> > word size and endianess be enough, e.g., will regexes generated on >> > x86_64el >> > work on armv8el (apparently this is what I observed but it could change >> > at >> > the whim of the PCRE2 maintainers)? So what will be the encoding? If we >> > encode the complete architecture then the typical case for android >> > (build on >> > x86_64 for armv7/armv8) will yield the same result as without the -r >> > option, >> > namely, we rebuild the regexes on the device. If we just encode word >> > size >> > and endianess it may work for a while... I just don't feel comfortable >> > enough to make a decision at this point. >> >> Then shelve it and use textual file contexts. The only purpose of the >> binary format is to >> include the pre-compiled regex;s so if you cant use that feature, >> there's no point in >> using binary format. >> >> My thought would be that it has to be an exact match for architecture >> upfront, then >> possibly investigate relaxing the requirement later. But, IIRC, if we get >> a 30% >> increase in text file load time, theirs really no point, for the >> binary format on Android. >> Android fic files are smaller, and have much simpler regex's compared >> to our desktop >> brethren. >> >> > >> > My limited understanding is that the automata built by PCRE2 use >> > pointers to >> > link states or other structures. These pointers are symbolized when >> > serialized, but they keep their width, which is why the regexes are at >> > least >> > sensitive to the word size. >> > >> > Just a wild Idea: >> > PCRE2 also supports JIT compiling. I did not use this here because I did >> > not >> > feel comfortable for libselinux to depend on the execmem permission. But >> > this could be developed into pre cross compiling the regexes into native >> > code, which can than be linked into a device specific library. But I >> > guess >> > this would require quite some cooperation with the PCRE2 developers. >> >> I was going to ask if the arch dependency was on JIT'd code or just >> something else >> which you elaborated above with word size/endiness, etc. >> >> > >> >> >> >> The other thing is, this is only an issue on binary formated >> >> file_context >> >> files, >> >> this is the ideal reason to move back to text files, since their is no >> >> speedup >> >> gained if we have to compile them. >> > >> > >> > Right, or you could see it as an intermediate solution that does not >> > require >> > changes to the build system until we can properly cross compile the >> > regexes. >> >> If you add an option to sefcontext_compile it should be to remove >> them. not add it in. >> This keeps it consistent with teh behavior for PCRE, it would be add >> to say, "make me >> a binary fc, but don't actually embed the regex information", since >> that is currently not >> the default behavior. Changing the Android make recipe is trivial, so >> adding -r shouldn't >> for Android shouldn't be a show stopper. >> >> -- Respectfully, William C Roberts ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
> > > That's just the thing. Without -r the phone _will_ boot because the regexes > are compiled on first use. With -r and an arch mismatch we have an undefined > behavior, which is bad. That's just a limitation of the current design. > > See, I don't currently know what part of the architecture is important. Will > word size and endianess be enough, e.g., will regexes generated on x86_64el > work on armv8el (apparently this is what I observed but it could change at > the whim of the PCRE2 maintainers)? So what will be the encoding? If we > encode the complete architecture then the typical case for android (build on > x86_64 for armv7/armv8) will yield the same result as without the -r option, > namely, we rebuild the regexes on the device. If we just encode word size > and endianess it may work for a while... I just don't feel comfortable > enough to make a decision at this point. Then shelve it and use textual file contexts. The only purpose of the binary format is to include the pre-compiled regex;s so if you cant use that feature, there's no point in using binary format. My thought would be that it has to be an exact match for architecture upfront, then possibly investigate relaxing the requirement later. But, IIRC, if we get a 30% increase in text file load time, theirs really no point, for the binary format on Android. Android fic files are smaller, and have much simpler regex's compared to our desktop brethren. > > My limited understanding is that the automata built by PCRE2 use pointers to > link states or other structures. These pointers are symbolized when > serialized, but they keep their width, which is why the regexes are at least > sensitive to the word size. > > Just a wild Idea: > PCRE2 also supports JIT compiling. I did not use this here because I did not > feel comfortable for libselinux to depend on the execmem permission. But > this could be developed into pre cross compiling the regexes into native > code, which can than be linked into a device specific library. But I guess > this would require quite some cooperation with the PCRE2 developers. I was going to ask if the arch dependency was on JIT'd code or just something else which you elaborated above with word size/endiness, etc. > >> >> The other thing is, this is only an issue on binary formated file_context >> files, >> this is the ideal reason to move back to text files, since their is no >> speedup >> gained if we have to compile them. > > > Right, or you could see it as an intermediate solution that does not require > changes to the build system until we can properly cross compile the regexes. If you add an option to sefcontext_compile it should be to remove them. not add it in. This keeps it consistent with teh behavior for PCRE, it would be add to say, "make me a binary fc, but don't actually embed the regex information", since that is currently not the default behavior. Changing the Android make recipe is trivial, so adding -r shouldn't for Android shouldn't be a show stopper. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 11:23 AM William Roberts wrote: > On Fri, Sep 16, 2016 at 3:13 AM, Janis Danisevskis > wrote: > > First of all, I would like to thank you, Stephen and William, for your > > patience and support. > > > > On Thu, Sep 15, 2016 at 8:34 PM William Roberts < > bill.c.robe...@gmail.com> > > wrote: > >> > >> On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley > >> wrote: > >> > On 09/15/2016 10:04 AM, Janis Danisevskis wrote: > >> >> From: Janis Danisevskis > >> >> > >> >> This patch moves all pcre1/2 dependencies into the new files regex.h > >> >> and regex.c implementing the common denominator of features needed > >> >> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the > >> >> used implementations. > >> >> > >> >> As of this patch libselinux supports either pcre or pcre2 but not > >> >> both at the same time. The persistently stored file contexts > >> >> information differs. This means libselinux can only load file > >> >> context files generated by sefcontext_compile build with the > >> >> same pcre variant. > >> >> > >> >> Also, for pcre2 the persistent format is architecture dependent. > >> >> Stored precompiled regular expressions can only be used on the > >> >> same architecture they were generated on. If pcre2 is used, > >> >> sefcontext_compile now respects the "-r". This flag makes > >> >> sefcontext_compile include the precompiled regular expressions > >> >> in the output file. The default is to omit them, so that the > >> >> output remains portable at the cost of having to recompile > >> >> the regular expressions at load time, or rather on first use. > >> > > >> > Is that really the default behavior you want? > >> > >> I would make the default include the pre-compiled regex's, if there > >> is an arch mismatch then ditch the pre-compiled ones and reload > >> them. I would make it an option to NOT generate pre-compiled > >> regexes. For the upstream consumers, who compile and deploy > >> on the same system, this means they don't need to update to add > >> -r. For android, we just need to update a makefile to include -r to > >> omit the precompiled regex's. > > > > > > The arch is currently not encoded in the binary, so a mismatch cannot be > > detected. I would sleep better if we left the -r option as is for now, > > because it is kind of "use only if you know what you are doing". Also, > this > > way it does not break the android build process. > > We probably would want to encode the architecture since this matters.This > way > we can detect a mismatch and compile them. This,prevents the > why isn't my device booting issue for cross compilations that don't have > -r. > That's just the thing. Without -r the phone _will_ boot because the regexes are compiled on first use. With -r and an arch mismatch we have an undefined behavior, which is bad. See, I don't currently know what part of the architecture is important. Will word size and endianess be enough, e.g., will regexes generated on x86_64el work on armv8el (apparently this is what I observed but it could change at the whim of the PCRE2 maintainers)? So what will be the encoding? If we encode the complete architecture then the typical case for android (build on x86_64 for armv7/armv8) will yield the same result as without the -r option, namely, we rebuild the regexes on the device. If we just encode word size and endianess it may work for a while... I just don't feel comfortable enough to make a decision at this point. My limited understanding is that the automata built by PCRE2 use pointers to link states or other structures. These pointers are symbolized when serialized, but they keep their width, which is why the regexes are at least sensitive to the word size. Just a wild Idea: PCRE2 also supports JIT compiling. I did not use this here because I did not feel comfortable for libselinux to depend on the execmem permission. But this could be developed into pre cross compiling the regexes into native code, which can than be linked into a device specific library. But I guess this would require quite some cooperation with the PCRE2 developers. > The other thing is, this is only an issue on binary formated file_context > files, > this is the ideal reason to move back to text files, since their is no > speedup > gained if we have to compile them. > Right, or you could see it as an intermediate solution that does not require changes to the build system until we can properly cross compile the regexes. > > > > >> > >> > >> In the case of Android, where we cross compile the file_contexts.bin, > >> does using binary fc's even make sense now? The major reason is that > >> the binary format loads faster by skipping the regex compilation, we > >> can no longer skip that with PCRE2 and cross compilation. We should > >> likely move Android back to text based if this is the case. I have > >> speedups in the neighborhood of 30% for text file processing in > >> the pipeline. > > > > > > I realize that the binary PCR
Re: [PATCH] libselinux: add support for pcre2
On Fri, Sep 16, 2016 at 3:13 AM, Janis Danisevskis wrote: > First of all, I would like to thank you, Stephen and William, for your > patience and support. > > On Thu, Sep 15, 2016 at 8:34 PM William Roberts > wrote: >> >> On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley >> wrote: >> > On 09/15/2016 10:04 AM, Janis Danisevskis wrote: >> >> From: Janis Danisevskis >> >> >> >> This patch moves all pcre1/2 dependencies into the new files regex.h >> >> and regex.c implementing the common denominator of features needed >> >> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the >> >> used implementations. >> >> >> >> As of this patch libselinux supports either pcre or pcre2 but not >> >> both at the same time. The persistently stored file contexts >> >> information differs. This means libselinux can only load file >> >> context files generated by sefcontext_compile build with the >> >> same pcre variant. >> >> >> >> Also, for pcre2 the persistent format is architecture dependent. >> >> Stored precompiled regular expressions can only be used on the >> >> same architecture they were generated on. If pcre2 is used, >> >> sefcontext_compile now respects the "-r". This flag makes >> >> sefcontext_compile include the precompiled regular expressions >> >> in the output file. The default is to omit them, so that the >> >> output remains portable at the cost of having to recompile >> >> the regular expressions at load time, or rather on first use. >> > >> > Is that really the default behavior you want? >> >> I would make the default include the pre-compiled regex's, if there >> is an arch mismatch then ditch the pre-compiled ones and reload >> them. I would make it an option to NOT generate pre-compiled >> regexes. For the upstream consumers, who compile and deploy >> on the same system, this means they don't need to update to add >> -r. For android, we just need to update a makefile to include -r to >> omit the precompiled regex's. > > > The arch is currently not encoded in the binary, so a mismatch cannot be > detected. I would sleep better if we left the -r option as is for now, > because it is kind of "use only if you know what you are doing". Also, this > way it does not break the android build process. We probably would want to encode the architecture since this matters.This way we can detect a mismatch and compile them. This,prevents the why isn't my device booting issue for cross compilations that don't have -r. The other thing is, this is only an issue on binary formated file_context files, this is the ideal reason to move back to text files, since their is no speedup gained if we have to compile them. > >> >> >> In the case of Android, where we cross compile the file_contexts.bin, >> does using binary fc's even make sense now? The major reason is that >> the binary format loads faster by skipping the regex compilation, we >> can no longer skip that with PCRE2 and cross compilation. We should >> likely move Android back to text based if this is the case. I have >> speedups in the neighborhood of 30% for text file processing in >> the pipeline. > > > I realize that the binary PCRE2 variant is currently much slower than the > PCRE stored regexes. But it still a little faster than the text file > processing. If you can make text file processing 30% faster, than this may > well be an option because serializing pcre2 regexes will probably never beat > pcre, even if we did stunts like compiling for the correct architecture > using qemu. > I'd be happy to help perform benchmarks and choose the best option. Once you dump the pre-compiled regex's from the binary format, your back to the text files performance. Also, with PCRE, the binary format was 3x the size on disk. > > Also I am exploring another option, which may sound a bit exotic. So I would > like to have a running POC first before I come forward. Maybe nothing comes > of it. > >> >> >> > >> >> Signed-off-by: Janis Danisevskis >> >> --- >> > >> >> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h >> >> index 6d1e890..3df7972 100644 >> >> --- a/libselinux/src/label_file.h >> >> +++ b/libselinux/src/label_file.h >> >> @@ -453,12 +429,14 @@ static inline int process_line(struct >> >> selabel_handle *rec, >> >>*/ >> >> data->nspec++; >> >> >> >> - if (rec->validating && >> >> - compile_regex(data, &spec_arr[nspec], >> >> &errbuf)) { >> >> + if (rec->validating >> >> + && compile_regex(data, &spec_arr[nspec], >> >> &error_data)) { >> >> + regex_format_error(&error_data, >> >> regex_error_format_buffer, >> >> + sizeof(regex_error_format_buffer)); >> >> COMPAT_LOG(SELINUX_ERROR, >> >> "%s: line %u has invalid regex %s: %s\n", >> >> path, lineno, regex, >> >> -(errbuf ? errbuf : "out of memory")); >> >> +regex_err
Re: [PATCH] libselinux: add support for pcre2
On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley wrote: > On 09/15/2016 10:04 AM, Janis Danisevskis wrote: >> From: Janis Danisevskis >> >> This patch moves all pcre1/2 dependencies into the new files regex.h >> and regex.c implementing the common denominator of features needed >> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the >> used implementations. >> >> As of this patch libselinux supports either pcre or pcre2 but not >> both at the same time. The persistently stored file contexts >> information differs. This means libselinux can only load file >> context files generated by sefcontext_compile build with the >> same pcre variant. >> >> Also, for pcre2 the persistent format is architecture dependent. >> Stored precompiled regular expressions can only be used on the >> same architecture they were generated on. If pcre2 is used, >> sefcontext_compile now respects the "-r". This flag makes >> sefcontext_compile include the precompiled regular expressions >> in the output file. The default is to omit them, so that the >> output remains portable at the cost of having to recompile >> the regular expressions at load time, or rather on first use. > > Is that really the default behavior you want? I would make the default include the pre-compiled regex's, if there is an arch mismatch then ditch the pre-compiled ones and reload them. I would make it an option to NOT generate pre-compiled regexes. For the upstream consumers, who compile and deploy on the same system, this means they don't need to update to add -r. For android, we just need to update a makefile to include -r to omit the precompiled regex's. In the case of Android, where we cross compile the file_contexts.bin, does using binary fc's even make sense now? The major reason is that the binary format loads faster by skipping the regex compilation, we can no longer skip that with PCRE2 and cross compilation. We should likely move Android back to text based if this is the case. I have speedups in the neighborhood of 30% for text file processing in the pipeline. > >> Signed-off-by: Janis Danisevskis >> --- > >> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h >> index 6d1e890..3df7972 100644 >> --- a/libselinux/src/label_file.h >> +++ b/libselinux/src/label_file.h >> @@ -453,12 +429,14 @@ static inline int process_line(struct selabel_handle >> *rec, >>*/ >> data->nspec++; >> >> - if (rec->validating && >> - compile_regex(data, &spec_arr[nspec], &errbuf)) { >> + if (rec->validating >> + && compile_regex(data, &spec_arr[nspec], &error_data)) >> { >> + regex_format_error(&error_data, regex_error_format_buffer, >> + sizeof(regex_error_format_buffer)); >> COMPAT_LOG(SELINUX_ERROR, >> "%s: line %u has invalid regex %s: %s\n", >> path, lineno, regex, >> -(errbuf ? errbuf : "out of memory")); >> +regex_error_format_buffer); > > compile_regex() may fail on an out of memory condition before > regex_error_format_buffer is initialized, which is why we previously > passed errbuf ?: "out of memory" above. I suppose you could initialize > regex_error_format_buffer with "out of memory" prior to calling > compile_regex(). > >> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c >> new file mode 100644 >> index 000..1c4a84d >> --- /dev/null >> +++ b/libselinux/src/regex.c > >> +int regex_writef(struct regex_data *regex, FILE *fp) > > This needs to be updated with the new do_write_precompregex argument, > and either use the argument or mark it unused to permit compilation for > the USE_PCRE2=n. > > ___ > Seandroid-list mailing list > seandroid-l...@tycho.nsa.gov > To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov. > To get help, send an email containing "help" to > seandroid-list-requ...@tycho.nsa.gov. -- Respectfully, William C Roberts ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On 09/15/2016 12:14 PM, Janis Danisevskis wrote: > From: Janis Danisevskis > > This patch moves all pcre1/2 dependencies into the new files regex.h > and regex.c implementing the common denominator of features needed > by libselinux. The compiler flag -DUSE_PCRE2 toggles between the > used implementations. > > As of this patch libselinux supports either pcre or pcre2 but not > both at the same time. The persistently stored file contexts > information differs. This means libselinux can only load file > context files generated by sefcontext_compile build with the > same pcre variant. > > Also, for pcre2 the persistent format is architecture dependent. > Stored precompiled regular expressions can only be used on the > same architecture they were generated on. If pcre2 is used, > sefcontext_compile now respects the "-r". This flag makes > sefcontext_compile include the precompiled regular expressions > in the output file. The default is to omit them, so that the > output remains portable at the cost of having to recompile > the regular expressions at load time, or rather on first use. > > Signed-off-by: Janis Danisevskis Thanks, applied, with the attached fix on top to allow building. >From a9162c813adaadbdb632d1a71d7c6ffc3e43b1b0 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Thu, 15 Sep 2016 13:43:24 -0400 Subject: [PATCH] libselinux: regex_writef: Mark unused argument with __attribute__((unused)). Signed-off-by: Stephen Smalley --- libselinux/src/regex.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c index 646351b..750088e 100644 --- a/libselinux/src/regex.c +++ b/libselinux/src/regex.c @@ -312,7 +312,8 @@ static inline pcre_extra *get_pcre_extra(struct regex_data *regex) } } -int regex_writef(struct regex_data *regex, FILE *fp, int unused) +int regex_writef(struct regex_data *regex, FILE *fp, + int unused __attribute__((unused))) { int rc; size_t len; -- 2.7.4 ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libselinux: add support for pcre2
On 09/15/2016 10:04 AM, Janis Danisevskis wrote: > From: Janis Danisevskis > > This patch moves all pcre1/2 dependencies into the new files regex.h > and regex.c implementing the common denominator of features needed > by libselinux. The compiler flag -DUSE_PCRE2 toggles between the > used implementations. > > As of this patch libselinux supports either pcre or pcre2 but not > both at the same time. The persistently stored file contexts > information differs. This means libselinux can only load file > context files generated by sefcontext_compile build with the > same pcre variant. > > Also, for pcre2 the persistent format is architecture dependent. > Stored precompiled regular expressions can only be used on the > same architecture they were generated on. If pcre2 is used, > sefcontext_compile now respects the "-r". This flag makes > sefcontext_compile include the precompiled regular expressions > in the output file. The default is to omit them, so that the > output remains portable at the cost of having to recompile > the regular expressions at load time, or rather on first use. Is that really the default behavior you want? > Signed-off-by: Janis Danisevskis > --- > diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h > index 6d1e890..3df7972 100644 > --- a/libselinux/src/label_file.h > +++ b/libselinux/src/label_file.h > @@ -453,12 +429,14 @@ static inline int process_line(struct selabel_handle > *rec, >*/ > data->nspec++; > > - if (rec->validating && > - compile_regex(data, &spec_arr[nspec], &errbuf)) { > + if (rec->validating > + && compile_regex(data, &spec_arr[nspec], &error_data)) { > + regex_format_error(&error_data, regex_error_format_buffer, > + sizeof(regex_error_format_buffer)); > COMPAT_LOG(SELINUX_ERROR, > "%s: line %u has invalid regex %s: %s\n", > path, lineno, regex, > -(errbuf ? errbuf : "out of memory")); > +regex_error_format_buffer); compile_regex() may fail on an out of memory condition before regex_error_format_buffer is initialized, which is why we previously passed errbuf ?: "out of memory" above. I suppose you could initialize regex_error_format_buffer with "out of memory" prior to calling compile_regex(). > diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c > new file mode 100644 > index 000..1c4a84d > --- /dev/null > +++ b/libselinux/src/regex.c > +int regex_writef(struct regex_data *regex, FILE *fp) This needs to be updated with the new do_write_precompregex argument, and either use the argument or mark it unused to permit compilation for the USE_PCRE2=n. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
RE: [PATCH] libselinux: add support for pcre2
> -Original Message- > From: Janis Danisevskis [mailto:jda...@android.com] > Sent: Thursday, September 8, 2016 8:52 AM > To: selinux@tycho.nsa.gov; seandroid-l...@tycho.nsa.gov; s...@tycho.nsa.gov; > jwca...@tycho.nsa.gov > Cc: Janis Danisevskis ; Roberts, William C > > Subject: [PATCH] libselinux: add support for pcre2 > > From: Janis Danisevskis > > This patch moves all pcre1/2 dependencies into the new files regex.h and > regex.c > implementing the common denominator of features needed by libselinux. The > compiler flag -DUSE_PCRE2 toggles between the used implementations. > > As of this patch libselinux supports either pcre or pcre2 but not both at the > same > time. The persistently stored file contexts information differs. This means > libselinux can only load file context files generated by sefcontext_compile > build > with the same pcre variant. > > Also, for pcre2 the persistent format is architecture dependent. > Stored precompiled regular expressions can only be used on the same > architecture they were generated on. If pcre2 is used and sefcontext_compile > shall generate portable output, it and libselinux must be compiled with - > DNO_PERSISTENTLY_STORED_PATTERNS, at the cost of having to recompile the > regular expressions at load time. > > Signed-off-by: Janis Danisevskis > > This patch includes includes: Double includes > > libselinux: fix memory leak on pcre2 > > Introduced a malloc on pcre_version(). Libselinux expected this to be static, > just > use a static internal buffer. > > Signed-off-by: William Roberts You can remove any attribution since its squashed down, this really doesn't Apply, so don't feel obligated to include any of this information. > --- > libselinux/Makefile | 13 + > libselinux/src/Makefile | 4 +- > libselinux/src/label_file.c | 93 ++- > libselinux/src/label_file.h | 59 ++--- > libselinux/src/regex.c| 461 > ++ > libselinux/src/regex.h| 169 + > libselinux/utils/Makefile | 4 +- > libselinux/utils/sefcontext_compile.c | 55 +--- > 8 files changed, 697 insertions(+), 161 deletions(-) create mode 100644 > libselinux/src/regex.c create mode 100644 libselinux/src/regex.h > > diff --git a/libselinux/Makefile b/libselinux/Makefile index 6142b60..15d051e > 100644 > --- a/libselinux/Makefile > +++ b/libselinux/Makefile > @@ -24,6 +24,19 @@ ifeq ($(DISABLE_SETRANS),y) endif export DISABLE_AVC > DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS > > +USE_PCRE2 ?= n > +DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS ?= n ifeq ($(USE_PCRE2),y) > + PCRE_CFLAGS := -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8 > + ifeq ($(DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS), y) > + PCRE_CFLAGS += -DNO_PERSISTENTLY_STORED_PATTERNS > + endif > + PCRE_LDFLAGS := -lpcre2-8 > +else > + PCRE_LDFLAGS := -lpcre > +endif > +export PCRE_CFLAGS PCRE_LDFLAGS > + > all install relabel clean distclean indent: > @for subdir in $(SUBDIRS); do \ > (cd $$subdir && $(MAKE) $@) || exit 1; \ diff --git > a/libselinux/src/Makefile b/libselinux/src/Makefile index 37d01af..66687e6 > 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -74,7 +74,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat- > security -Winit-self -Wmissi >-fipa-pure-const -Wno-suggest-attribute=pure -Wno-suggest- > attribute=const \ >-Werror -Wno-aggregate-return -Wno-redundant-decls > > -override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE $(EMFLAGS) > +override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE > +$(EMFLAGS) $(PCRE_CFLAGS) > > SWIG_CFLAGS += -Wno-error -Wno-unused-variable -Wno-unused-but-set- > variable -Wno-unused-parameter \ > -Wno-shadow -Wno-uninitialized -Wno-missing-prototypes - > Wno-missing-declarations @@ -113,7 +113,7 @@ $(LIBA): $(OBJS) > $(RANLIB) $@ > > $(LIBSO): $(LOBJS) > - $(CC) $(CFLAGS) -shared -o $@ $^ -lpcre -ldl $(LDFLAGS) -L$(LIBDIR) > -Wl,- > soname,$(LIBSO),-z,defs,-z,relro > + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) > +-L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > ln -sf $@ $(TARGET) > > $(LIBPC): $(LIBPC).in ../VERSION > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index > c89bb35..e41c351 100644 > --- a/libselinux/src/label_file.c > +++ b/libselinux/src/label_file.c > @@ -15,7 +15,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -112,6 +111,7 @@ static int load_mmap(struct selabel_handle *rec, const > char *path, > struct mmap_area *mmap_area; > uint32_t i, magic, version; > uint32_t entry_len, stem_map_len, regex_array_len; > + const char *reg_version; > > if (isbinary) { > len = strlen(path); > @@ -175,8 +175,13 @
Re: [PATCH] libselinux: add support for pcre2
On 09/08/2016 11:52 AM, Janis Danisevskis wrote: > From: Janis Danisevskis > > This patch moves all pcre1/2 dependencies into the new files regex.h > and regex.c implementing the common denominator of features needed > by libselinux. The compiler flag -DUSE_PCRE2 toggles between the > used implementations. > > As of this patch libselinux supports either pcre or pcre2 but not > both at the same time. The persistently stored file contexts > information differs. This means libselinux can only load file > context files generated by sefcontext_compile build with the > same pcre variant. > > Also, for pcre2 the persistent format is architecture dependent. > Stored precompiled regular expressions can only be used on the > same architecture they were generated on. If pcre2 is used and > sefcontext_compile shall generate portable output, it and libselinux > must be compiled with -DNO_PERSISTENTLY_STORED_PATTERNS, at the > cost of having to recompile the regular expressions at load time. > > Signed-off-by: Janis Danisevskis > > This patch includes includes: > > libselinux: fix memory leak on pcre2 > > Introduced a malloc on pcre_version(). Libselinux > expected this to be static, just use a static > internal buffer. > > Signed-off-by: William Roberts > --- > libselinux/Makefile | 13 + > libselinux/src/Makefile | 4 +- > libselinux/src/label_file.c | 93 ++- > libselinux/src/label_file.h | 59 ++--- > libselinux/src/regex.c| 461 > ++ > libselinux/src/regex.h| 169 + > libselinux/utils/Makefile | 4 +- > libselinux/utils/sefcontext_compile.c | 55 +--- > 8 files changed, 697 insertions(+), 161 deletions(-) > create mode 100644 libselinux/src/regex.c > create mode 100644 libselinux/src/regex.h > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index 37d01af..66687e6 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -74,7 +74,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k > -Wformat-security -Winit-self -Wmissi >-fipa-pure-const -Wno-suggest-attribute=pure > -Wno-suggest-attribute=const \ >-Werror -Wno-aggregate-return -Wno-redundant-decls > > -override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE $(EMFLAGS) > +override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE $(EMFLAGS) > $(PCRE_CFLAGS) > > SWIG_CFLAGS += -Wno-error -Wno-unused-variable -Wno-unused-but-set-variable > -Wno-unused-parameter \ > -Wno-shadow -Wno-uninitialized -Wno-missing-prototypes > -Wno-missing-declarations > @@ -113,7 +113,7 @@ $(LIBA): $(OBJS) > $(RANLIB) $@ > > $(LIBSO): $(LOBJS) > - $(CC) $(CFLAGS) -shared -o $@ $^ -lpcre -ldl $(LDFLAGS) -L$(LIBDIR) > -Wl,-soname,$(LIBSO),-z,defs,-z,relro > + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) > -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro > ln -sf $@ $(TARGET) > > $(LIBPC): $(LIBPC).in ../VERSION We want make to still work in this subdirectory, so we need defaults assigned to PCRE_CFLAGS and PCRE_LDFLAGS if not set by the caller. > diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h > index 6d1e890..24cb9e0 100644 > --- a/libselinux/src/label_file.h > +++ b/libselinux/src/label_file.h > @@ -394,7 +368,8 @@ static inline int process_line(struct selabel_handle *rec, > struct saved_data *data = (struct saved_data *)rec->data; > struct spec *spec_arr; > unsigned int nspec = data->nspec; > - const char *errbuf = NULL; > + char const *errbuf; Unnecessary change? > + struct regex_error_data error_data; > > items = read_spec_entries(line_buf, &errbuf, 3, ®ex, &type, > &context); > if (items < 0) { > @@ -454,7 +429,7 @@ static inline int process_line(struct selabel_handle *rec, > data->nspec++; > > if (rec->validating && > - compile_regex(data, &spec_arr[nspec], &errbuf)) { > + compile_regex(data, &spec_arr[nspec], &error_data)) > { Same bug as before - errbuf was being used in the following log call, so you need to change it to use regex_format_error(). > COMPAT_LOG(SELINUX_ERROR, > "%s: line %u has invalid regex %s: %s\n", > path, lineno, regex, > diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c > new file mode 100644 > index 000..558a72a > --- /dev/null > +++ b/libselinux/src/regex.c > @@ -0,0 +1,461 @@ > +#include > +#include > +#include > +#include > + > +#include "regex.h" > +#include "label_file.h" > + > +#ifdef USE_PCRE2 > +int regex_prepare_data(struct regex_data ** regex, > + char const * pattern_string, > + struct regex_error_data * errordata) { All of the non-static functions in this file need to be marked