Normally SELinux is configured correctly and this should not happen. Though, I think simply checking access right is fast enough so we don't strictly need to blacklist these. Feel free to provide a patch. Note that your pseudo code is incorrect, as it would not work on non-Linux platforms.

Created attachment 280414 [details] [review]
check with g_access if executable

check with g_access if executable. If no executable flag available e.g. Windows just test for access

Review of attachment 280414 [details] [review]:

Please, create a patch using git, on git master. Commit with your fullname and email address. While comiting, a post process script will tell you if you have coding style errors. When patch is ready, and tested, create a patch file using "git format-patch -1". Thanks for your time.

::: gstreamer-1.3.3.orig/gst/gstregistry.c
@@ +1263,3 @@
       continue;
+    } 
+     if (g_access(filename,X_OK)!=0) {

Indent and coding style error here.

Review of attachment 280414 [details] [review]:

::: gstreamer-1.3.3.orig/gst/gstregistry.c
@@ +1263,3 @@
       continue;
+    } 
+     if (g_access(filename,X_OK)!=0) {

This is also not correct. It's not required for a shared library to have the executable bit set, and this patch would even break everything on Debian based systems (where the packaging tools automatically *unset* the X bit of shared libraries!)

What exactly is the problem here again? SELinux requires shared libraries to have the X bit set and otherwise fails to load them?

You are right. SeLinux doesn't pay attention the normal execution rights is set.

But the shared object needs a SELInux rule that allows execution of the shared library and needs to be labeled for that.

SeLinux is working in this way. Enabled SeLinux prohibits execution on files that are not labeled to have SeLinuxs execute rights. And this also affects shared objects loaded by executables. So Selinux will simply not execute any code in a shared object as long it is not labeled.

You are completely right that check for execute is not the right way. We should introduce the complete SeLinux in gstreamer for such file operations. 

Or the handling at g_module_open should be different.

Yeah somewhere there needs to be SELinux integration then. And in a way that does not break existing code. Also SELinux really shouldn't abuse existing concepts like the X permission bit for something completely different...

What happens if I mount the directory xxx as a sub dir on /usr/lib/gstreamer-1.0/xxx with noexec. I am quite sure I get the same error

in gstplugin.c something like that would help ?

After a failed g_module_open we check it again with a g_module_open with LAZY flags so we see if this occured because of unknown symbol or general dlopen problems

  module = g_module_open (filename, flags);
  if (module == NULL) {
      flags |= G_MODULE_BIND_LAZY;
       module = g_module_open (filename, flags);
       if (module == NULL) {
           GST_CAT_WARNING (GST_CAT_PLUGIN_LOADING, "module_open failed: %s",
                  g_module_error ());
              g_set_error (error,
           GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE, "Opening module failed: %s",
             g_module_error ());
          /* If we failed to open the shared object, then it's probably because a
          * plugin is linked against the wrong libraries. Print out an easy-to-see
          * message in this case. */

Generally here is a misunderstanding of SELinux concepts and how it provides access and execution rights. It is widespreeded on all distributions. If you want to know more then please read this article https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/4/html/SELinux_Guide/index.html

But introducing the SELinux machinery for a one special case could be a little bit too much overhead. I have implemented this my code and it is working. 

Even if this directory is simply execute protected I run into the same issue. The question is : should a plugin be blacklist  permanently because at the time of the first read it I was not accessible/executeable ? 
IMO if we have a cache then it should intelligent enough to handle this. Otherwise the user has to manipulate its system manually. And this is never ok.

If I do not get any reply I will it rest in peace


in gst_registry.c : gst_registry_scan_path_level(
 
if (g_module_supported () != FALSE) {
	flags = G_MODULE_BIND_LOCAL | G_MODULE_BIND_LAZY;
        module = g_module_open (filename, flags);
        if (module == NULL) {
          GST_TRACE_OBJECT (context->registry, "%s file is secured by SELinux, ignoring",filename);
          g_free (filename);
          continue;
        }
        g_module_close(module);
    }

I think Fedora/RedHat gave us on using SELinux for regular applications and only use it for services. If this is still needed, we need a patch that works with SELinux systems but doesn't break Debian & Windows. Does GLib have such an API, if it doesn't we should probably create something and propose it to the GLib peeps.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/61.