Re: [drm-misc:drm-misc-next 2/3] drivers/gpu/drm/loongson/lsdc_plane.c:199 lsdc_cursor_plane_atomic_async_check() warn: variable dereferenced before check 'state' (see line 180)

[suijingfeng <suijingfeng@xxxxxxxxxxx>]

Hi,

On 2023/7/10 14:29, Dan Carpenter wrote:
> tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> head:   8d1077cf2e43b15fefd76ebec2b71541eb27ef2c
> commit: f39db26c54281da6a785259498ca74b5e470476f [2/3] drm: Add kms driver for loongson display controller
> config: i386-randconfig-m021-20230710 (https://download.01.org/0day-ci/archive/20230710/202307100423.rV7D05Uq-lkp@xxxxxxxxx/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20230710/202307100423.rV7D05Uq-lkp@xxxxxxxxx/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@xxxxxxxxx>
> | Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> | Closes: https://lore.kernel.org/r/202307100423.rV7D05Uq-lkp@xxxxxxxxx/
>
> smatch warnings:
> drivers/gpu/drm/loongson/lsdc_plane.c:199 lsdc_cursor_plane_atomic_async_check() warn: variable dereferenced before check 'state' (see line 180)
>
> vim +/state +199 drivers/gpu/drm/loongson/lsdc_plane.c
>
> f39db26c54281d Sui Jingfeng 2023-06-15  174  static int lsdc_cursor_plane_atomic_async_check(struct drm_plane *plane,
> f39db26c54281d Sui Jingfeng 2023-06-15  175  						struct drm_atomic_state *state)
> f39db26c54281d Sui Jingfeng 2023-06-15  176  {
> f39db26c54281d Sui Jingfeng 2023-06-15  177  	struct drm_plane_state *new_state;
> f39db26c54281d Sui Jingfeng 2023-06-15  178  	struct drm_crtc_state *crtc_state;
> f39db26c54281d Sui Jingfeng 2023-06-15  179
> f39db26c54281d Sui Jingfeng 2023-06-15 @180  	new_state = drm_atomic_get_new_plane_state(state, plane);
>                                                                                             ^^^^^
> state is dereferenced inside this function
>
> f39db26c54281d Sui Jingfeng 2023-06-15  181
> f39db26c54281d Sui Jingfeng 2023-06-15  182  	if (!plane->state || !plane->state->fb) {
> f39db26c54281d Sui Jingfeng 2023-06-15  183  		drm_dbg(plane->dev, "%s: state is NULL\n", plane->name);
> f39db26c54281d Sui Jingfeng 2023-06-15  184  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  185  	}
> f39db26c54281d Sui Jingfeng 2023-06-15  186
> f39db26c54281d Sui Jingfeng 2023-06-15  187  	if (new_state->crtc_w != new_state->crtc_h) {
> f39db26c54281d Sui Jingfeng 2023-06-15  188  		drm_dbg(plane->dev, "unsupported cursor size: %ux%u\n",
> f39db26c54281d Sui Jingfeng 2023-06-15  189  			new_state->crtc_w, new_state->crtc_h);
> f39db26c54281d Sui Jingfeng 2023-06-15  190  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  191  	}
> f39db26c54281d Sui Jingfeng 2023-06-15  192
> f39db26c54281d Sui Jingfeng 2023-06-15  193  	if (new_state->crtc_w != 64 && new_state->crtc_w != 32) {
> f39db26c54281d Sui Jingfeng 2023-06-15  194  		drm_dbg(plane->dev, "unsupported cursor size: %ux%u\n",
> f39db26c54281d Sui Jingfeng 2023-06-15  195  			new_state->crtc_w, new_state->crtc_h);
> f39db26c54281d Sui Jingfeng 2023-06-15  196  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  197  	}
> f39db26c54281d Sui Jingfeng 2023-06-15  198
> f39db26c54281d Sui Jingfeng 2023-06-15 @199  	if (state) {
>                                                      ^^^^^
> Checked too late


Yes, there no need to check here, simply writing the code as following 
is OK.


```
        crtc_state = drm_atomic_get_existing_crtc_state(state, 
new_state->crtc);
```

> f39db26c54281d Sui Jingfeng 2023-06-15  200  		crtc_state = drm_atomic_get_existing_crtc_state(state, new_state->crtc);
> f39db26c54281d Sui Jingfeng 2023-06-15  201  	} else {
> f39db26c54281d Sui Jingfeng 2023-06-15  202  		crtc_state = plane->crtc->state;
> f39db26c54281d Sui Jingfeng 2023-06-15  203  		drm_dbg(plane->dev, "%s: atomic state is NULL\n", plane->name);
> f39db26c54281d Sui Jingfeng 2023-06-15  204  	}
> f39db26c54281d Sui Jingfeng 2023-06-15  205
> f39db26c54281d Sui Jingfeng 2023-06-15  206  	if (!crtc_state->active)
> f39db26c54281d Sui Jingfeng 2023-06-15  207  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  208
> f39db26c54281d Sui Jingfeng 2023-06-15  209  	if (plane->state->crtc != new_state->crtc ||
> f39db26c54281d Sui Jingfeng 2023-06-15  210  	    plane->state->src_w != new_state->src_w ||
> f39db26c54281d Sui Jingfeng 2023-06-15  211  	    plane->state->src_h != new_state->src_h ||
> f39db26c54281d Sui Jingfeng 2023-06-15  212  	    plane->state->crtc_w != new_state->crtc_w ||
> f39db26c54281d Sui Jingfeng 2023-06-15  213  	    plane->state->crtc_h != new_state->crtc_h)
> f39db26c54281d Sui Jingfeng 2023-06-15  214  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  215
> f39db26c54281d Sui Jingfeng 2023-06-15  216  	if (new_state->visible != plane->state->visible)
> f39db26c54281d Sui Jingfeng 2023-06-15  217  		return -EINVAL;
> f39db26c54281d Sui Jingfeng 2023-06-15  218
> f39db26c54281d Sui Jingfeng 2023-06-15  219  	return drm_atomic_helper_check_plane_state(plane->state,
> f39db26c54281d Sui Jingfeng 2023-06-15  220  						   crtc_state,
> f39db26c54281d Sui Jingfeng 2023-06-15  221  						   DRM_PLANE_NO_SCALING,
> f39db26c54281d Sui Jingfeng 2023-06-15  222  						   DRM_PLANE_NO_SCALING,
> f39db26c54281d Sui Jingfeng 2023-06-15  223  						   true, true);
> f39db26c54281d Sui Jingfeng 2023-06-15  224  }