[Spice-devel] [PATCH spice-server v2] gstreamer: Check if ORC library can work

[Christophe de Dinechin]

> On 25 Aug 2017, at 16:37, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>>> 
>>> 
>>> On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
>>>> ORC lirabry is used internally by GStreamer to generate code
>>>> dynamically.
>>> 
>>> "The ORC library"
>>> 
>>>> If ORC cannot allocate executable memory the failure cause
>>>> an abort(3) to be called.
>>> 
>>> "memory, the failure causes"
>>> 
>>>> This happens on some SELinux configuration that disable executable
>>>> memory allocation (execmem boolean).
>>> 
>>> Either "SELinux configurations" or "that disables"
>>> 
>>>> Check that ORC could work before attempting to use GStreamer to
>>>> avoid crashes. The log will report an error.
>>> 
>>>> 
>>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>>> ---
>>>> configure.ac               |  6 ++++++
>>>> server/Makefile.am         |  2 ++
>>>> server/gstreamer-encoder.c | 30 ++++++++++++++++++++++++++++++
>>>> 3 files changed, 38 insertions(+)
>>>> 
>>>> Changes since v1:
>>>> - rename orc_cached to orc_dynamic_code_ok.
>>>> 
>>>> diff --git a/configure.ac b/configure.ac
>>>> index e1e74862..4bdf8588 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -116,6 +116,12 @@ AS_IF([test x"$missing_gstreamer_elements" = xyes],
>>>>     [SPICE_WARNING([The GStreamer video encoder can be built but may not
>>>>     work.])
>>>> ])
>>>> 
>>>> +if test "x$have_gstreamer_0_10" = "xyes" -o "x$have_gstreamer_1_0" =
>>>> "xyes"; then
>>>> +    PKG_CHECK_MODULES(ORC, orc-0.4)
>>>> +    AC_SUBST(ORC_CFLAGS)
>>>> +    AC_SUBST(ORC_LIBS)
>>>> +fi
>>>> +
>>>> AC_ARG_ENABLE([automated_tests],
>>>>               AS_HELP_STRING([--enable-automated-tests], [Enable
>>>>               automated
>>>>               tests using spicy-screenshot (part of spice-gtk)]),,
>>>>               [enable_automated_tests="no"])
>>>> diff --git a/server/Makefile.am b/server/Makefile.am
>>>> index ef8d31fc..5d5590af 100644
>>>> --- a/server/Makefile.am
>>>> +++ b/server/Makefile.am
>>>> @@ -20,6 +20,7 @@ AM_CPPFLAGS =					\
>>>> 	$(SSL_CFLAGS)				\
>>>> 	$(VISIBILITY_HIDDEN_CFLAGS)		\
>>>> 	$(WARN_CFLAGS)				\
>>>> +	$(ORC_CFLAGS)				\
>>>> 	$(NULL)
>>>> 
>>>> noinst_LTLIBRARIES = libserver.la
>>>> @@ -54,6 +55,7 @@ libserver_la_LIBADD =							\
>>>> 	$(SSL_LIBS)							\
>>>> 	$(Z_LIBS)							\
>>>> 	$(SPICE_NONPKGCONFIG_LIBS)					\
>>>> +	$(ORC_LIBS)							\
>>>> 	$(NULL)
>>>> 
>>>> libspice_serverincludedir = $(includedir)/spice-server
>>>> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
>>>> index bb4f27ba..2a39154f 100644
>>>> --- a/server/gstreamer-encoder.c
>>>> +++ b/server/gstreamer-encoder.c
>>>> @@ -27,6 +27,7 @@
>>>> #include <gst/app/gstappsrc.h>
>>>> #include <gst/app/gstappsink.h>
>>>> #include <gst/video/video.h>
>>>> +#include <orc/orcprogram.h>
>>>> 
>>>> #include "red-common.h"
>>>> #include "video-encoder.h"
>>>> @@ -1702,6 +1703,30 @@ static void
>>>> spice_gst_encoder_get_stats(VideoEncoder
>>>> *video_encoder,
>>>>     }
>>>> }
>>>> 
>>>> +/* Check if ORC library can work.
>>>> + * ORC library is used quite extensively by GStreamer
>>>> + * to generate code dynamically. If ORC cannot work GStreamer
>>> 
>>> "cannot work, GStreamer"
>>> 
>>>> + * will abort(3) the entire process.
>>>> + */
>>>> +static bool orc_check(void)
>>>> +{
>>>> +    static bool orc_checked = false;
>>>> +    static bool orc_dynamic_code_ok = false;
>>>> +
>>>> +    if (SPICE_UNLIKELY(!orc_checked)) {
>>>> +        OrcCode *code = orc_code_new();
>>>> +        if (code) {
>>>> +            /* allocating 0 byte for code make the function not crash
>>> 
>>> "makes"
>>> 
>>>> +             * but doing all initializations and checks */
>>> 
>>> I would use "but it does all the initializations and checks"
>>> 
>>> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>>> 
>> 
>> I'll do the changes.
>> 
>> I just noted that if the check fails is quite silence.
>> Maybe the first time we discover the issue we should log
>> something?
>> 
> 
> Ignore that, I just remembered that ORC itself print a huge
> error.

Does it even in the case it does not abort() ?

I see this message in orccodemem.c:

  ORC_ERROR("Failed to create write and exec mmap regions.  This "
      "is probably because SELinux execmem check is enabled (good) "
      "and $TMPDIR and $HOME are mounted noexec (bad).");

Is that the one?

Is it possible to add a message on how to fix for our specific use case?

Thanks
Christophe


> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel