Re: FYI AppArmor abstraction for AMD proprietary driver

Am 09.10.2016 um 17:45 schrieb Christian Boltz:

> Can you please remove this rule and test if something complains?

Well, I have switched to the open source driver in Leap kernel 4.1, thus
can´t test it.....
The open source driver has a bit less performance, but is stable and
lacks some bugs, the proprietary driver has.

>>   /dev/video* rw,
>>   /dev/ati/* rw,
>>   /etc/ati r,
> 
> Same questions once more, this time for /etc/ati ;-)

I just added these permissions to satisfy AppArmor. Without setting
these, I had some AA log entries.

>>   /etc/ati/** r,
>>   /etc/ati/authatieventsd.sh Ux,
> 
> What does this script do?

This script grants access to some AMD "event daemon", I guess this thing
checks for updates or maybe some hardware events triggered by the GPU,
but this is just a guess.
I have attached the script, so you can take a look at it. It looks
pretty harmless to me.

> We avoid Ux rules whenever possible (because they allow to execute 
> something unconfined = without AppArmor restrictions), so you should have 
> a *very* good reason to use Ux ;-)
> 
>>   /dev/shm/ rwkl,
> 
> Hmm, wkl permissions for the directory? That looks superfluous to me - r 
> should be enough.

True

>>   /dev/shm/* rwkl,
> 
> Reading and writing all files in /dev/shm/ (which is world-writeable like 
> /tmp/) doesn't sound too neat. Would it be possible to restrict that 
> rule by using a filename pattern and/or adding the "owner" conditional?

I think adding "owner" should be fine. But, as mentioned, I switched to
the open source driver recently....

>>   /home/*/.AMD/ rwkl,
>>   /home/*/.AMD/** rwkl,
> 
> Interesting - does the AMD driver really need write access in the user's 
> home directory? Or is it only needed by the config tool? (assuming there 
> is a config tool ;-)

Yes, there is a config tool. This directory is a cache directory for
OpenGL stuff.
The proprietary nVidia driver also uses a user space cache directory
with the name ~/.nv/

> If these rules are really needed, adding the "owner" conditional would 
> be a good idea to ensure it doesn't touch someone else' home directory.

Sure, using "owner" should be fine there.

But, AMD is working on a new generation of proprietary driver, they
haven´t their current (=old) proprietary time for 10 months now!
Kernel 4.4 and above has an updated AMD driver named amdgpu which will
also be the basis for the new AMD proprietary driver. Thus, people with
the latest AMD GPUs actually use the latest kernels, because they have
the necessary support the old proprietary driver does not have.

In other words, folks who run the latest AMD GPU depend on the latest
kernels and not the current (old) AMD prop stuff, thus the number of
people using the current (= now old) AMD prop driver should be very,
very small.

IMHO it may be best to wait until AMD releases their new generation
proprietary driver and then adjust this AA rule. To add this rule to
Leap 42.2 may not make much sense, because people with latest AMD GPU
depend on the latest open source driver anyways....

Best regards

#!/bin/sh

#
# Control script grant/revoke access to X for the ATI External Events Daemon
#
# Distro maintainers may modify this reference script as necessary to conform
# to their distribution policies.
#
# Copyright (c) 2006, ATI Technologies Inc.  All rights reserved.
#

#
# Parameters:
#   $1 is a keyword, either "grant" or "revoke"
#   $2 is the display name
#   $3 is the X authorization file to be authorized
#
# Returns:
#   0 if authorization was successfully granted/revoked
#   nonzero on failure
#
# Note:
#   The third parameter only makes sense if xauth is being used.  If another
#   mechanism such as xhost is being used it can be ignored.  For setups that
#   do not do any form of authentication(!) this script can be trimmed down
#   to just "exit 0" and the daemon will assume that it is always authorized.
#

GetServerAuthFile()
{
    # Determine where the authorization key may be hiding.  The location will
    # vary depending upon whether X was started via xdm/kdm, gdm or startx, so
    # check each one in turn.

    # Check xdm/kdm

    XDM_AUTH_FILE=/var/lib/xdm/authdir/authfiles/A$1*
    if [ -e $XDM_AUTH_FILE ]; then
        SERVER_AUTH_FILE=`ls -t $XDM_AUTH_FILE | head -n 1`
        DISP_SEARCH_STRING="#ffff#"
        return 0
    fi

    # Check gdm

    GDM_AUTH_FILE=/var/lib/gdm/$1.Xauth
    if [ -e $GDM_AUTH_FILE ]; then
        SERVER_AUTH_FILE=$GDM_AUTH_FILE
        DISP_SEARCH_STRING="$1"
        return 0
    fi

    # Finally, check for startx

    for XPID in `pidof X`; do
        TRIAL_XAUTH_FILE=`tr '\0' '\n' < /proc/$XPID/environ | grep -e "^XAUTHORITY=" | cut -d= -f2`
        TRIAL_XAUTH_KEY=`xauth -f $TRIAL_XAUTH_FILE list | grep "unix$1"`
        if [ -n "$TRIAL_XAUTH_KEY" ]; then
            SERVER_AUTH_FILE=$TRIAL_XAUTH_FILE
            DISP_SEARCH_STRING="unix$1"
            return 0
        fi
    done

    # Couldn't find the key

    return -1
}

# Main part of script

#
# Since the daemon is usually started during init time before X comes up,
# $PATH may not yet contain the paths to the X binaries, particularly xauth.
# Add the usual location for where xauth may live and fail out if we still
# can't find it.
#

PATH=$PATH:/usr/bin:/usr/X11R6/bin
which xauth > /dev/null || exit -1

case "$1" in
    grant)
        GetServerAuthFile $2 || exit -1
        DISP_AUTH_KEY=`xauth -f $SERVER_AUTH_FILE list | grep $DISP_SEARCH_STRING | awk '{ print $3 }'`
        if [ -n "$DISP_AUTH_KEY" ]; then
            xauth -f $3 add $2 . $DISP_AUTH_KEY || exit -1
        else
            exit -1
        fi
        ;;

    revoke)
        xauth -f $3 remove $2 || exit -1
        ;;

    *)
        exit -1
        ;;
esac
exit 0