drm/auth: Only drm_drop_master if it exists

[PATCH] drm/auth: Only drm_drop_master if it exists

Posted by Jonathan Cavitt 2 months ago

It is possible that both dev->master and file_priv->master are NULL when
passed to drm_master_release, which would result in dev being passed to
drm_drop_master (as NULL == NULL here).  This would result in a NULL
pointer dereference when passing dev->master to drm_master_put in
drm_drop_master.

Only call drm_drop_master if dev->master exists.  Also, make sure the
original calling requirement is maintained (dev->master ==
file_priv->master).

This fixes a static analysis issue.

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
---
 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 e17bb0f1f9e0..e5013b870ba0 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -347,7 +347,7 @@ void drm_master_release(struct drm_file *file_priv)
 	if (!drm_is_current_master_locked(file_priv))
 		goto out;
 
-	if (dev->master == file_priv->master)
+	if (dev->master && dev->master == file_priv->master)
 		drm_drop_master(dev, file_priv);
 out:
 	if (drm_core_check_feature(dev, DRIVER_MODESET) && file_priv->is_master) {
-- 
2.53.0

Re: [PATCH] drm/auth: Only drm_drop_master if it exists

Posted by Dmitry Baryshkov 1 month, 3 weeks ago

On Fri, Apr 17, 2026 at 05:00:47AM +0800, Jonathan Cavitt wrote:
> It is possible that both dev->master and file_priv->master are NULL when
> passed to drm_master_release, which would result in dev being passed to
> drm_drop_master (as NULL == NULL here).  This would result in a NULL
> pointer dereference when passing dev->master to drm_master_put in
> drm_drop_master.
> 
> Only call drm_drop_master if dev->master exists.  Also, make sure the
> original calling requirement is maintained (dev->master ==
> file_priv->master).
> 
> This fixes a static analysis issue.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> ---
>  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 e17bb0f1f9e0..e5013b870ba0 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -347,7 +347,7 @@ void drm_master_release(struct drm_file *file_priv)
>  	if (!drm_is_current_master_locked(file_priv))
>  		goto out;
>  
> -	if (dev->master == file_priv->master)

The previous call checks that file_priv->master != NULL. So, at this
point the NULL == NULL check is not possible.

I'm sorry, but this looks like a false positive.

> +	if (dev->master && dev->master == file_priv->master)
>  		drm_drop_master(dev, file_priv);
>  out:
>  	if (drm_core_check_feature(dev, DRIVER_MODESET) && file_priv->is_master) {
> -- 
> 2.53.0
> 

-- 
With best wishes
Dmitry

RE: [PATCH] drm/auth: Only drm_drop_master if it exists

Posted by Cavitt, Jonathan 1 month, 3 weeks ago

-----Original Message-----
From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
Sent: Wednesday, April 22, 2026 12:14 PM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: dri-devel@lists.freedesktop.org; Gupta, Saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; linux-kernel@vger.kernel.org; maarten.lankhorst@linux.intel.com; mripard@kernel.org; tzimmerman@suse.de; airlied@gmail.com; simona@ffwll.ch
Subject: Re: [PATCH] drm/auth: Only drm_drop_master if it exists
> 
> On Fri, Apr 17, 2026 at 05:00:47AM +0800, Jonathan Cavitt wrote:
> > It is possible that both dev->master and file_priv->master are NULL when
> > passed to drm_master_release, which would result in dev being passed to
> > drm_drop_master (as NULL == NULL here).  This would result in a NULL
> > pointer dereference when passing dev->master to drm_master_put in
> > drm_drop_master.
> > 
> > Only call drm_drop_master if dev->master exists.  Also, make sure the
> > original calling requirement is maintained (dev->master ==
> > file_priv->master).
> > 
> > This fixes a static analysis issue.
> > 
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@gmail.com>
> > Cc: Simona Vetter <simona@ffwll.ch>
> > ---
> >  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 e17bb0f1f9e0..e5013b870ba0 100644
> > --- a/drivers/gpu/drm/drm_auth.c
> > +++ b/drivers/gpu/drm/drm_auth.c
> > @@ -347,7 +347,7 @@ void drm_master_release(struct drm_file *file_priv)
> >  	if (!drm_is_current_master_locked(file_priv))
> >  		goto out;
> >  
> > -	if (dev->master == file_priv->master)
> 
> The previous call checks that file_priv->master != NULL. So, at this
> point the NULL == NULL check is not possible.

You mean drm_is_current_master_locked?

Internally, that performs the check
"fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master"

It doesn't explicitly check for fpriv->master != NULL, but I don't think the current execution
path allows for fpriv->is_master && !fpriv->master, so I'm guessing that's what you're referring to?
Otherwise, we could be passing a NULL pointer to drm_lease_owner, which would itself be a NULL
pointer dereference issue...

> 
> I'm sorry, but this looks like a false positive.

This has already been merged.  If you think it necessary, please feel free to file a revert
and send the request my way for review.
-Jonathan Cavitt

> 
> > +	if (dev->master && dev->master == file_priv->master)
> >  		drm_drop_master(dev, file_priv);
> >  out:
> >  	if (drm_core_check_feature(dev, DRIVER_MODESET) && file_priv->is_master) {
> > -- 
> > 2.53.0
> > 
> 
> -- 
> With best wishes
> Dmitry
>

Re: [PATCH] drm/auth: Only drm_drop_master if it exists

Posted by Luben Tuikov 2 months ago

On 2026-04-16 17:00, Jonathan Cavitt wrote:
> It is possible that both dev->master and file_priv->master are NULL when
> passed to drm_master_release, which would result in dev being passed to
> drm_drop_master (as NULL == NULL here).  This would result in a NULL
> pointer dereference when passing dev->master to drm_master_put in
> drm_drop_master.
> 
> Only call drm_drop_master if dev->master exists.  Also, make sure the
> original calling requirement is maintained (dev->master ==
> file_priv->master).
> 
> This fixes a static analysis issue.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> ---
>  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 e17bb0f1f9e0..e5013b870ba0 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -347,7 +347,7 @@ void drm_master_release(struct drm_file *file_priv)
>  	if (!drm_is_current_master_locked(file_priv))
>  		goto out;
>  
> -	if (dev->master == file_priv->master)
> +	if (dev->master && dev->master == file_priv->master)
>  		drm_drop_master(dev, file_priv);
>  out:
>  	if (drm_core_check_feature(dev, DRIVER_MODESET) && file_priv->is_master) {

This is a good catch.

Acked-by: Luben Tuikov <ltuikov89@gmail.com>

-- 
Regards,
Luben