* [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
@ 2023-12-06 13:51 Lingkai Dong
2023-12-07 10:12 ` Linus Walleij
2023-12-07 10:24 ` Linus Walleij
0 siblings, 2 replies; 7+ messages in thread
From: Lingkai Dong @ 2023-12-06 13:51 UTC (permalink / raw)
To: dri-devel; +Cc: nd
The DRM subsystem keeps a record of the owner of a DRM device file
descriptor using thread group ID (TGID) instead of process ID (PID), to
ensures all threads within the same userspace process are considered the
owner. However, the DRM master ownership check compares the current
thread's PID against the record, so the thread is incorrectly considered to
be not the FD owner if the PID is not equal to the TGID. This causes DRM
ioctls to be denied master privileges, even if the same thread that opened
the FD performs an ioctl. Fix this by checking TGID.
Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
---
drivers/gpu/drm/drm_auth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 2ed2585ded378..6899b3dc1f12a 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -236,7 +236,7 @@ static int
drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv)
{
if (file_priv->was_master &&
- rcu_access_pointer(file_priv->pid) == task_pid(current))
+ rcu_access_pointer(file_priv->pid) == task_tgid(current))
return 0;
if (!capable(CAP_SYS_ADMIN))
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-06 13:51 [PATCH] drm: Fix FD ownership check in drm_master_check_perm() Lingkai Dong
@ 2023-12-07 10:12 ` Linus Walleij
2023-12-07 10:18 ` Christian König
2023-12-07 10:24 ` Linus Walleij
1 sibling, 1 reply; 7+ messages in thread
From: Linus Walleij @ 2023-12-07 10:12 UTC (permalink / raw)
To: Lingkai Dong, Tvrtko Ursulin, Christian König; +Cc: nd, dri-devel
On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com> wrote:
> The DRM subsystem keeps a record of the owner of a DRM device file
> descriptor using thread group ID (TGID) instead of process ID (PID), to
> ensures all threads within the same userspace process are considered the
> owner. However, the DRM master ownership check compares the current
> thread's PID against the record, so the thread is incorrectly considered to
> be not the FD owner if the PID is not equal to the TGID. This causes DRM
> ioctls to be denied master privileges, even if the same thread that opened
> the FD performs an ioctl. Fix this by checking TGID.
>
> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
Paging the patch author (Tvrko) and committer (Christian).
Here is the patch if you don't have it in your mailbox:
https://lore.kernel.org/dri-devel/PA6PR08MB107665920BE9A96658CDA04CE8884A@PA6PR08MB10766.eurprd08.prod.outlook.com/
I'm seeing this as well (on Android).
Tvrko, Christian: can you look at this?
Will you apply it to the AMD tree for fixes if it looks OK
or does it go elsewhere?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-07 10:12 ` Linus Walleij
@ 2023-12-07 10:18 ` Christian König
2023-12-07 10:22 ` Tvrtko Ursulin
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2023-12-07 10:18 UTC (permalink / raw)
To: Linus Walleij, Lingkai Dong, Tvrtko Ursulin; +Cc: nd, dri-devel
Am 07.12.23 um 11:12 schrieb Linus Walleij:
> On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com> wrote:
>
>> The DRM subsystem keeps a record of the owner of a DRM device file
>> descriptor using thread group ID (TGID) instead of process ID (PID), to
>> ensures all threads within the same userspace process are considered the
>> owner. However, the DRM master ownership check compares the current
>> thread's PID against the record, so the thread is incorrectly considered to
>> be not the FD owner if the PID is not equal to the TGID. This causes DRM
>> ioctls to be denied master privileges, even if the same thread that opened
>> the FD performs an ioctl. Fix this by checking TGID.
>>
>> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
>> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
> Paging the patch author (Tvrko) and committer (Christian).
> Here is the patch if you don't have it in your mailbox:
> https://lore.kernel.org/dri-devel/PA6PR08MB107665920BE9A96658CDA04CE8884A@PA6PR08MB10766.eurprd08.prod.outlook.com/
>
> I'm seeing this as well (on Android).
>
> Tvrko, Christian: can you look at this?
Good catch, looks like we missed this occasion while switching from PID
to TGID.
> Will you apply it to the AMD tree for fixes if it looks OK
> or does it go elsewhere?
I can push this to drm-misc-fixes as long as nobody objects in the next
hour or so.
CC: stable? If yes which versions?
Regards,
Christian.
>
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-07 10:18 ` Christian König
@ 2023-12-07 10:22 ` Tvrtko Ursulin
2023-12-07 13:55 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-12-07 10:22 UTC (permalink / raw)
To: Christian König, Linus Walleij, Lingkai Dong, Tvrtko Ursulin
Cc: nd, dri-devel
On 07/12/2023 10:18, Christian König wrote:
> Am 07.12.23 um 11:12 schrieb Linus Walleij:
>> On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com> wrote:
>>
>>> The DRM subsystem keeps a record of the owner of a DRM device file
>>> descriptor using thread group ID (TGID) instead of process ID (PID), to
>>> ensures all threads within the same userspace process are considered the
>>> owner. However, the DRM master ownership check compares the current
>>> thread's PID against the record, so the thread is incorrectly
>>> considered to
>>> be not the FD owner if the PID is not equal to the TGID. This causes DRM
>>> ioctls to be denied master privileges, even if the same thread that
>>> opened
>>> the FD performs an ioctl. Fix this by checking TGID.
>>>
>>> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
>>> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
>> Paging the patch author (Tvrko) and committer (Christian).
>> Here is the patch if you don't have it in your mailbox:
>> https://lore.kernel.org/dri-devel/PA6PR08MB107665920BE9A96658CDA04CE8884A@PA6PR08MB10766.eurprd08.prod.outlook.com/
>>
>> I'm seeing this as well (on Android).
>>
>> Tvrko, Christian: can you look at this?
>
> Good catch, looks like we missed this occasion while switching from PID
> to TGID.
Oops, yes..
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Will you apply it to the AMD tree for fixes if it looks OK
>> or does it go elsewhere?
>
> I can push this to drm-misc-fixes as long as nobody objects in the next
> hour or so.
>
> CC: stable? If yes which versions?
Cc: <stable@vger.kernel.org> # v6.4+
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-06 13:51 [PATCH] drm: Fix FD ownership check in drm_master_check_perm() Lingkai Dong
2023-12-07 10:12 ` Linus Walleij
@ 2023-12-07 10:24 ` Linus Walleij
1 sibling, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2023-12-07 10:24 UTC (permalink / raw)
To: Lingkai Dong; +Cc: nd, dri-devel
On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com> wrote:
> The DRM subsystem keeps a record of the owner of a DRM device file
> descriptor using thread group ID (TGID) instead of process ID (PID), to
> ensures all threads within the same userspace process are considered the
> owner. However, the DRM master ownership check compares the current
> thread's PID against the record, so the thread is incorrectly considered to
> be not the FD owner if the PID is not equal to the TGID. This causes DRM
> ioctls to be denied master privileges, even if the same thread that opened
> the FD performs an ioctl. Fix this by checking TGID.
>
> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-07 10:22 ` Tvrtko Ursulin
@ 2023-12-07 13:55 ` Christian König
2023-12-07 14:04 ` Lingkai Dong
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2023-12-07 13:55 UTC (permalink / raw)
To: Tvrtko Ursulin, Linus Walleij, Lingkai Dong, Tvrtko Ursulin; +Cc: nd, dri-devel
Am 07.12.23 um 11:22 schrieb Tvrtko Ursulin:
>
>
> On 07/12/2023 10:18, Christian König wrote:
>> Am 07.12.23 um 11:12 schrieb Linus Walleij:
>>> On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com>
>>> wrote:
>>>
>>>> The DRM subsystem keeps a record of the owner of a DRM device file
>>>> descriptor using thread group ID (TGID) instead of process ID
>>>> (PID), to
>>>> ensures all threads within the same userspace process are
>>>> considered the
>>>> owner. However, the DRM master ownership check compares the current
>>>> thread's PID against the record, so the thread is incorrectly
>>>> considered to
>>>> be not the FD owner if the PID is not equal to the TGID. This
>>>> causes DRM
>>>> ioctls to be denied master privileges, even if the same thread that
>>>> opened
>>>> the FD performs an ioctl. Fix this by checking TGID.
>>>>
>>>> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
>>>> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
>>> Paging the patch author (Tvrko) and committer (Christian).
>>> Here is the patch if you don't have it in your mailbox:
>>> https://lore.kernel.org/dri-devel/PA6PR08MB107665920BE9A96658CDA04CE8884A@PA6PR08MB10766.eurprd08.prod.outlook.com/
>>>
>>>
>>> I'm seeing this as well (on Android).
>>>
>>> Tvrko, Christian: can you look at this?
>>
>> Good catch, looks like we missed this occasion while switching from
>> PID to TGID.
>
> Oops, yes..
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>>> Will you apply it to the AMD tree for fixes if it looks OK
>>> or does it go elsewhere?
>>
>> I can push this to drm-misc-fixes as long as nobody objects in the
>> next hour or so.
>>
>> CC: stable? If yes which versions?
>
> Cc: <stable@vger.kernel.org> # v6.4+
And pushed to drm-misc-fixes.
Thanks,
Christian.
>
> Regards,
>
> Tvrtko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
2023-12-07 13:55 ` Christian König
@ 2023-12-07 14:04 ` Lingkai Dong
0 siblings, 0 replies; 7+ messages in thread
From: Lingkai Dong @ 2023-12-07 14:04 UTC (permalink / raw)
To: Christian König, Tvrtko Ursulin, Linus Walleij, Tvrtko Ursulin
Cc: nd, dri-devel
[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]
Thank you all!
Regards,
Lingkai
________________________________
From: Christian König <christian.koenig@amd.com>
Sent: Thursday, December 7, 2023 1:55 PM
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>; Linus Walleij <linus.walleij@linaro.org>; Lingkai Dong <Lingkai.Dong@arm.com>; Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: nd <nd@arm.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: Fix FD ownership check in drm_master_check_perm()
Am 07.12.23 um 11:22 schrieb Tvrtko Ursulin:
>
>
> On 07/12/2023 10:18, Christian König wrote:
>> Am 07.12.23 um 11:12 schrieb Linus Walleij:
>>> On Wed, Dec 6, 2023 at 2:52 PM Lingkai Dong <Lingkai.Dong@arm.com>
>>> wrote:
>>>
>>>> The DRM subsystem keeps a record of the owner of a DRM device file
>>>> descriptor using thread group ID (TGID) instead of process ID
>>>> (PID), to
>>>> ensures all threads within the same userspace process are
>>>> considered the
>>>> owner. However, the DRM master ownership check compares the current
>>>> thread's PID against the record, so the thread is incorrectly
>>>> considered to
>>>> be not the FD owner if the PID is not equal to the TGID. This
>>>> causes DRM
>>>> ioctls to be denied master privileges, even if the same thread that
>>>> opened
>>>> the FD performs an ioctl. Fix this by checking TGID.
>>>>
>>>> Fixes: 4230cea89cafb ("drm: Track clients by tgid and not tid")
>>>> Signed-off-by: Lingkai Dong <lingkai.dong@arm.com>
>>> Paging the patch author (Tvrko) and committer (Christian).
>>> Here is the patch if you don't have it in your mailbox:
>>> https://lore.kernel.org/dri-devel/PA6PR08MB107665920BE9A96658CDA04CE8884A@PA6PR08MB10766.eurprd08.prod.outlook.com/
>>>
>>>
>>> I'm seeing this as well (on Android).
>>>
>>> Tvrko, Christian: can you look at this?
>>
>> Good catch, looks like we missed this occasion while switching from
>> PID to TGID.
>
> Oops, yes..
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>>> Will you apply it to the AMD tree for fixes if it looks OK
>>> or does it go elsewhere?
>>
>> I can push this to drm-misc-fixes as long as nobody objects in the
>> next hour or so.
>>
>> CC: stable? If yes which versions?
>
> Cc: <stable@vger.kernel.org> # v6.4+
And pushed to drm-misc-fixes.
Thanks,
Christian.
>
> Regards,
>
> Tvrtko
[-- Attachment #2: Type: text/html, Size: 4869 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-07 14:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 13:51 [PATCH] drm: Fix FD ownership check in drm_master_check_perm() Lingkai Dong
2023-12-07 10:12 ` Linus Walleij
2023-12-07 10:18 ` Christian König
2023-12-07 10:22 ` Tvrtko Ursulin
2023-12-07 13:55 ` Christian König
2023-12-07 14:04 ` Lingkai Dong
2023-12-07 10:24 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).