[PATCH] drm/i2c/ch7006: fix a possible null pointer dereference

[PATCH] drm/i2c/ch7006: fix a possible null pointer dereference

[Ma Ke]

In ch7006_encoder_get_modes(), the return value of drm_mode_duplicate()
is assigned to mode, which will lead to a NULL pointer dereference
on failure of drm_mode_duplicate(). Add a check to avoid npd.

Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
 drivers/gpu/drm/i2c/ch7006_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index 131512a5f3bd..27c2f02f5b43 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -236,8 +236,10 @@ static int ch7006_encoder_get_modes(struct drm_encoder *encoder,
 		    ~mode->valid_norms & 1<<priv->norm)
 			continue;
 
-		drm_mode_probed_add(connector,
-				drm_mode_duplicate(encoder->dev, &mode->mode));
+		struct drm_display_mode *encoder_mode;
+		encoder_mode = drm_mode_duplicate(encoder->dev, &mode->mode);
+		if (!mode)
+			continue;
 
 		n++;
 	}
-- 
2.37.2

Re: [PATCH] drm/i2c/ch7006: fix a possible null pointer dereference

[kernel test robot]

Hi Ma,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.6-rc4 next-20231006]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ma-Ke/drm-i2c-ch7006-fix-a-possible-null-pointer-dereference/20231007-112036
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231007031712.3997144-1-make_ruc2021%40163.com
patch subject: [PATCH] drm/i2c/ch7006: fix a possible null pointer dereference
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310071306.7oXVHYob-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310071306.7oXVHYob-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310071306.7oXVHYob-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i2c/ch7006_drv.c: In function 'ch7006_encoder_get_modes':
>> drivers/gpu/drm/i2c/ch7006_drv.c:239:42: warning: variable 'encoder_mode' set but not used [-Wunused-but-set-variable]
     239 |                 struct drm_display_mode *encoder_mode;
         |                                          ^~~~~~~~~~~~


vim +/encoder_mode +239 drivers/gpu/drm/i2c/ch7006_drv.c

   226	
   227	static int ch7006_encoder_get_modes(struct drm_encoder *encoder,
   228					    struct drm_connector *connector)
   229	{
   230		struct ch7006_priv *priv = to_ch7006_priv(encoder);
   231		const struct ch7006_mode *mode;
   232		int n = 0;
   233	
   234		for (mode = ch7006_modes; mode->mode.clock; mode++) {
   235			if (~mode->valid_scales & 1<<priv->scale ||
   236			    ~mode->valid_norms & 1<<priv->norm)
   237				continue;
   238	
 > 239			struct drm_display_mode *encoder_mode;
   240			encoder_mode = drm_mode_duplicate(encoder->dev, &mode->mode);
   241			if (!mode)
   242				continue;
   243	
   244			n++;
   245		}
   246	
   247		return n;
   248	}
   249	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm/i2c/ch7006: fix a possible null pointer dereference

[Lyude Paul]

On Sat, 2023-10-07 at 11:17 +0800, Ma Ke wrote:
> In ch7006_encoder_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a NULL pointer dereference
> on failure of drm_mode_duplicate(). Add a check to avoid npd.
> 
> Signed-off-by: Ma Ke <make_ruc2021@163.com>
> ---
>  drivers/gpu/drm/i2c/ch7006_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
> index 131512a5f3bd..27c2f02f5b43 100644
> --- a/drivers/gpu/drm/i2c/ch7006_drv.c
> +++ b/drivers/gpu/drm/i2c/ch7006_drv.c
> @@ -236,8 +236,10 @@ static int ch7006_encoder_get_modes(struct drm_encoder *encoder,
>  		    ~mode->valid_norms & 1<<priv->norm)
>  			continue;
>  
> -		drm_mode_probed_add(connector,
> -				drm_mode_duplicate(encoder->dev, &mode->mode));
> +		struct drm_display_mode *encoder_mode;
> +		encoder_mode = drm_mode_duplicate(encoder->dev, &mode->mode);

I think the kernel prefers to have variable declaration at the top of the
scope, and as well: this breaks things because you don't add back the
drm_mode_probed_add() call.

> +		if (!mode)
> +			continue;
>  
>  		n++;
>  	}

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat