Re: [Intel-gfx] What to do with xf86-video-intel backlight control when running Xorg as non root

[Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Hi,

On 02/13/2014 05:40 PM, Chris Wilson wrote:
> On Thu, Feb 13, 2014 at 04:52:59PM +0100, Hans de Goede wrote:
>> Hi All,
>>
>> Currently xf86-video-intel is unique in that it is the only video driver
>> which does backlight control inside the driver rather then letting
>> something else (ie the desktop environment) deal with it.
>>
>> This is a problem when running the xserver without root rights because
>> writing /sys/class/backlight/foo/brightness requires root rights.
>>
>> There are 2 possible (short term) solutions for this:
>>
>> 1) Detect that the xserver is not running as root, and don't register the
>> backlight property on the connectors, let something else deal with it,
>> as is done or other xf86-video-* drivers already.
>>
>> 2) Add a little xf86-video-intel-brightness helper on Linux which the driver
>> execs (through pkexec) each time it needs to set the brightness.
>>
>>
>> 1) of course is very KISS, so I like. 2) is not that hard either, and
>> 1) might cause some regressions in cases where ie gsd-brightness-helper
>> does not do the right thing, where as the intel driver does. OTOH it seems
>> that the intel video code is mostly there to deal with older kernels, and
>> rootless xorg will be used with newer kernels only anyways.
> 
> Not registering a property that is broken seems like the fundamental first
> step. Would it be possible to use udev to set the access mode on the
> backlight properties such that the display controller does have
> permission to write to those files?

When we were discussing this at Fosdem Kay Sievers was in the room, and I
can summarize his response to that suggestion as: *NO*.

> Otherwise, it seems like we need the
> proxy in order to keep the xrandr property available to users and
> prevent those who rely upon it in scripts from seeing regressions.

Right, that is what I was thinking too, so the question then becomes how
hard you will scream at me if I add something like this to xf86-video-intel
linux specific backlight code:

    if (geteuid() == 0) {
        /* Old write directly to /sys/class/backlight/... code */
    {
    else {
        /* The & is to avoid the xserver blocking */
        snprintf(command, sizeof(command), "pkexec %s/libexec/xf86-video-intel-backlight-helper %s %d&",
                 PREFIX, sna_output->backlight_iface, level);
        r = system(command);
        if (r) {
            /* complain */
        }
    }

If you won't scream too much, and more importantly, if you will accept such
a patch (including code for the helper), then I'll try to cook up something
like this tomorrow.

In case you're wondering what pkexec is, it stands for policy-kit-exec, it
is a little helper which will ask policykit if it is ok to run argv[1] as
root (for which there needs to be policy-file saying so), and if it is ok,
it will execute argv[1 .. x] as root (in a sanitized environment).

This will more or less give us the "display controller permission" you
were suggesting before without trying to get udev / logind to manage acls
on sysfs files.

To be specific we would need a policy file with aprox. this in there:

<policyconfig>
  <action id="org.x.xf86-video-intel.backlight-helper>
    <defaults>
      <allow_any>no</allow_any>
      <allow_inactive>no</allow_inactive>
      <allow_active>yes</allow_active>
    </defaults>
    <annotate key="org.freedesktop.policykit.exec.path">@PREFIX@/libexec/xf86-video-intel-backlight-helper
  </action>
</policyconfig>

Regards,

Hans
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel