Re: [bbrezillon-0day:drm-bridge-busfmt-v4 8/11] drivers/gpu/drm/drm_bridge.c:196:5-11: ERROR: allocation function on line 189 returns NULL not ERR_PTR on failure

[Boris Brezillon <boris.brezillon@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3997 bytes --]

Hello Julia,

On Wed, 4 Dec 2019 03:32:18 +0800
kbuild test robot <lkp@intel.com> wrote:

> tree:   https://github.com/bbrezillon/linux-0day drm-bridge-busfmt-v4
> head:   d7ec36aec4ff0b6741ff15cfb1d61f9078a797f7
> commit: eef75946f72c81815a43d5f19eecda08a327146e [8/11] drm/bridge: Add a drm_bridge_state object
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> 
> coccinelle warnings: (new ones prefixed by >>)
> 
> >> drivers/gpu/drm/drm_bridge.c:196:5-11: ERROR: allocation function on line 189 returns NULL not ERR_PTR on failure  
> 
> vim +196 drivers/gpu/drm/drm_bridge.c
> 
>    134	
>    135	/**
>    136	 * drm_bridge_attach - attach the bridge to an encoder's chain
>    137	 *
>    138	 * @encoder: DRM encoder
>    139	 * @bridge: bridge to attach
>    140	 * @previous: previous bridge in the chain (optional)
>    141	 *
>    142	 * Called by a kms driver to link the bridge to an encoder's chain. The previous
>    143	 * argument specifies the previous bridge in the chain. If NULL, the bridge is
>    144	 * linked directly at the encoder's output. Otherwise it is linked at the
>    145	 * previous bridge's output.
>    146	 *
>    147	 * If non-NULL the previous bridge must be already attached by a call to this
>    148	 * function.
>    149	 *
>    150	 * Note that bridges attached to encoders are auto-detached during encoder
>    151	 * cleanup in drm_encoder_cleanup(), so drm_bridge_attach() should generally
>    152	 * *not* be balanced with a drm_bridge_detach() in driver code.
>    153	 *
>    154	 * RETURNS:
>    155	 * Zero on success, error code on failure
>    156	 */
>    157	int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>    158			      struct drm_bridge *previous)
>    159	{
>    160		struct drm_bridge_state *state;
>    161		int ret;
>    162	
>    163		if (!encoder || !bridge)
>    164			return -EINVAL;
>    165	
>    166		if (previous && (!previous->dev || previous->encoder != encoder))
>    167			return -EINVAL;
>    168	
>    169		if (bridge->dev)
>    170			return -EBUSY;
>    171	
>    172		bridge->dev = encoder->dev;
>    173		bridge->encoder = encoder;
>    174	
>    175		if (previous)
>    176			list_add(&bridge->chain_node, &previous->chain_node);
>    177		else
>    178			list_add(&bridge->chain_node, &encoder->bridge_chain);
>    179	
>    180		if (bridge->funcs->attach) {
>    181			ret = bridge->funcs->attach(bridge);
>    182			if (ret < 0)
>    183				goto err_reset_bridge;
>    184		}
>    185	
>    186		if (bridge->funcs->atomic_reset) {
>    187			state = bridge->funcs->atomic_reset(bridge);
>    188		} else {
>  > 189			state = kzalloc(sizeof(*state), GFP_KERNEL);  
>    190			if (state)
>    191				__drm_atomic_helper_bridge_reset(bridge, state);
>    192			else
>    193				state = ERR_PTR(-ENOMEM);
>    194		}
>    195	
>  > 196		if (IS_ERR(state)) {  
>    197			ret = PTR_ERR(state);
>    198			goto err_detach_bridge;
>    199		}

Looks like we have a false positive here: state is set to
ERR_PTR(-ENOMEM) when kzalloc() returns NULL, but the cocci script
fails to detect that.

Regards,

Boris

>    200	
>    201		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
>    202					    &state->base,
>    203					    &drm_bridge_priv_state_funcs);
>    204	
>    205		return 0;
>    206	
>    207	err_detach_bridge:
>    208		if (bridge->funcs->detach)
>    209			bridge->funcs->detach(bridge);
>    210	
>    211	err_reset_bridge:
>    212		bridge->dev = NULL;
>    213		bridge->encoder = NULL;
>    214		list_del(&bridge->chain_node);
>    215		return ret;
>    216	}
>    217	EXPORT_SYMBOL(drm_bridge_attach);
>    218	
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation