linux-stable.git/drivers/gpu/drm/drm_auth.c, branch linux-5.14.y

drm: protect drm_master pointers in drm_lease.c

2021-09-18T11:43:38+00:00

[ Upstream commit 56f0729a510f92151682ff6c89f69724d5595d6e ]

drm_file->master pointers should be protected by
drm_device.master_mutex or drm_file.master_lookup_lock when being
dereferenced.

However, in drm_lease.c, there are multiple instances where
drm_file->master is accessed and dereferenced while neither lock is
held. This makes drm_lease.c vulnerable to use-after-free bugs.

We address this issue in 2 ways:

1. Add a new drm_file_get_master() function that calls drm_master_get
on drm_file->master while holding on to
drm_file.master_lookup_lock. Since drm_master_get increments the
reference count of master, this prevents master from being freed until
we unreference it with drm_master_put.

2. In each case where drm_file->master is directly accessed and
eventually dereferenced in drm_lease.c, we wrap the access in a call
to the new drm_file_get_master function, then unreference the master
pointer once we are done using it.

Reported-by: Daniel Vetter 
Signed-off-by: Desmond Cheong Zhi Xi 
Reviewed-by: Emil Velikov 
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-6-desmondcheongzx@gmail.com
Signed-off-by: Sasha Levin

drm: serialize drm_file.master with a new spinlock

2021-09-18T11:43:38+00:00

[ Upstream commit 0b0860a3cf5eccf183760b1177a1dcdb821b0b66 ]

Currently, drm_file.master pointers should be protected by
drm_device.master_mutex when being dereferenced. This is because
drm_file.master is not invariant for the lifetime of drm_file. If
drm_file is not the creator of master, then drm_file.is_master is
false, and a call to drm_setmaster_ioctl will invoke
drm_new_set_master, which then allocates a new master for drm_file and
puts the old master.

Thus, without holding drm_device.master_mutex, the old value of
drm_file.master could be freed while it is being used by another
concurrent process.

However, it is not always possible to lock drm_device.master_mutex to
dereference drm_file.master. Through the fbdev emulation code, this
might occur in a deep nest of other locks. But drm_device.master_mutex
is also the outermost lock in the nesting hierarchy, so this leads to
potential deadlocks.

To address this, we introduce a new spin lock at the bottom of the
lock hierarchy that only serializes drm_file.master. With this change,
the value of drm_file.master changes only when both
drm_device.master_mutex and drm_file.master_lookup_lock are
held. Hence, any process holding either of those locks can ensure that
the value of drm_file.master will not change concurrently.

Since no lock depends on the new drm_file.master_lookup_lock, when
drm_file.master is dereferenced, but drm_device.master_mutex cannot be
held, we can safely protect the master pointer with
drm_file.master_lookup_lock.

Reported-by: Daniel Vetter
Signed-off-by: Desmond Cheong Zhi Xi
Signed-off-by: Daniel Vetter
Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-5-desmondcheongzx@gmail.com
Signed-off-by: Sasha Levin

Backmerge tag 'v5.13-rc7' into drm-next

2021-06-23T00:07:48+00:00

Backmerge Linux 5.13-rc7 to make some pulls from later bases apply,
and to bake in the conflicts so far.

drm: Lock pointer access in drm_master_release()

2021-06-10T10:22:02+00:00

This patch eliminates the following smatch warning:
drivers/gpu/drm/drm_auth.c:320 drm_master_release() warn: unlocked access 'master' (line 318) expected lock '&dev->master_mutex'

The 'file_priv->master' field should be protected by the mutex lock to
'&dev->master_mutex'. This is because other processes can concurrently
modify this field and free the current 'file_priv->master'
pointer. This could result in a use-after-free error when 'master' is
dereferenced in subsequent function calls to
'drm_legacy_lock_master_cleanup()' or to 'drm_lease_revoke()'.

An example of a scenario that would produce this error can be seen
from a similar bug in 'drm_getunique()' that was reported by Syzbot:
https://syzkaller.appspot.com/bug?id=148d2f1dfac64af52ffd27b661981a540724f803

In the Syzbot report, another process concurrently acquired the
device's master mutex in 'drm_setmaster_ioctl()', then overwrote
'fpriv->master' in 'drm_new_set_master()'. The old value of
'fpriv->master' was subsequently freed before the mutex was unlocked.

Reported-by: Dan Carpenter 
Signed-off-by: Desmond Cheong Zhi Xi 
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210609092119.173590-1-desmondcheongzx@gmail.com

drm: drm_auth.c: Adjust end of block comment

2021-04-22T08:26:35+00:00

Add a new line with */ on the last line of a block comment to follow the
Linux kernel coding conventions.
Problem found by checkpatch.

Signed-off-by: Beatriz Martins de Carvalho 
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/cd1e8f6637b914825ee4b9643a4e9d9eba49f276.1618756333.git.martinsdecarvalhobeatriz@gmail.com

drm/auth: make drm_{set,drop}master_ioctl symmetrical

2020-06-15T13:49:50+00:00

Currently the ret handling is all over the place - with two redundant
assignments and another one addressed earlier.

Use the exact same flow in both functions.

v2: straighten the code flow, instead of just removing the assignments

Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 
Cc: Colin Ian King 
Signed-off-by: Emil Velikov 
Reviewed-by: Sam Ravnborg 
Link: https://patchwork.freedesktop.org/patch/msgid/20200530124640.4176323-2-emil.l.velikov@gmail.com

drm: vmwgfx: remove drm_driver::master_set() return type

2020-06-15T13:48:20+00:00

The function always returns zero (success). Ideally we'll remove it all
together - although that's requires a little more work.

For now, we can drop the return type and simplify the drm core code
surrounding it.

v2: remove redundant assignment (Sam)

Cc: David Airlie 
Cc: Daniel Vetter 
Cc: VMware Graphics 
Cc: Roland Scheidegger 
Signed-off-by: Emil Velikov 
Cc: Sam Ravnborg 
Reviewed-by: Sam Ravnborg 
Reviewed-by: Roland Scheidegger 
Link: https://patchwork.freedesktop.org/patch/msgid/20200530124640.4176323-1-emil.l.velikov@gmail.com

drm/auth: remove redundant assignment to variable ret

2020-05-25T14:35:52+00:00

The variable ret is being initialized with a value that is
never read and it is being updated later with a new value. The
initialization is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King 
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20200524222715.27305-1-colin.king@canonical.com

drm: error out with EBUSY when device has existing master

2020-03-30T11:20:41+00:00

As requested by Adam, provide different error message for when the
device has an existing master. An audit of the following projects, shows
that the errno is used only for printf() purposes.

xorg/xserver
xorg/drivers/xf86-video-ati
xorg/drivers/xf86-video-amdgpu
xorg/drivers/xf86-video-intel
xorg/drivers/xf86-video-tegra
xorg/drivers/xf86-video-freedreno
xorg/drivers/xf86-video-nouveau
xorg/drivers/xf86-video-vmwgfx

qt/kwin/plasma
gtk/mutter/gnomeshell
efl/enlightment

Cc: Adam Jackson 
Suggested-by: Adam Jackson 
Signed-off-by: Emil Velikov 
Reviewed-by: Adam Jackson 
Link: https://patchwork.freedesktop.org/patch/msgid/20200319172930.230583-2-emil.l.velikov@gmail.com

drm: rework SET_MASTER and DROP_MASTER perm handling

2020-03-30T11:20:32+00:00

This commit reworks the permission handling of the two ioctls. In
particular it enforced the CAP_SYS_ADMIN check only, if:
 - we're issuing the ioctl from process other than the one which opened
the node, and
 - we are, or were master in the past

This ensures that we:
 - do not regress the systemd-logind style of DRM_MASTER arbitrator
 - allow applications which do not use systemd-logind to drop their
master capabilities (and regain them at later point) ... w/o running as
root.

See the comment above drm_master_check_perm() for more details.

v1:
 - Tweak wording, fixup all checks, add igt test

v2:
 - Add a few more comments, grammar nitpicks.

Cc: Adam Jackson 
Cc: Daniel Vetter 
Cc: Pekka Paalanen 
Testcase: igt/core_setmaster/master-drop-set-user
Signed-off-by: Emil Velikov 
Reviewed-by: Adam Jackson 
Link: https://patchwork.freedesktop.org/patch/msgid/20200319172930.230583-1-emil.l.velikov@gmail.com