[bug report] cxgb4/ch_ktls: Clear resources when pf4 device is removed

[bug report] cxgb4/ch_ktls: Clear resources when pf4 device is removed

[Dan Carpenter]

Hello Ayush Sawal,

This is a semi-automatic email about new static checker warnings.

The patch 65e302a9bd57: "cxgb4/ch_ktls: Clear resources when pf4 
device is removed" from May 13, 2021, leads to the following Smatch 
complaint:

    drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c:393 chcr_ktls_dev_del()
    error: we previously assumed 'u_ctx' could be null (see line 374)

drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
   373		u_ctx = tx_info->adap->uld[CXGB4_ULD_KTLS].handle;
   374		if (u_ctx && u_ctx->detach)
                    ^^^^^
Can u_ctx be NULL?

   375			return;
   376		/* clear l2t entry */
   377		if (tx_info->l2te)
   378			cxgb4_l2t_release(tx_info->l2te);
   379	
   380	#if IS_ENABLED(CONFIG_IPV6)
   381		/* clear clip entry */
   382		if (tx_info->ip_family == AF_INET6)
   383			cxgb4_clip_release(netdev, (const u32 *)
   384					   &tx_info->sk->sk_v6_rcv_saddr,
   385					   1);
   386	#endif
   387	
   388		/* clear tid */
   389		if (tx_info->tid != -1) {
   390			cxgb4_remove_tid(&tx_info->adap->tids, tx_info->tx_chan,
   391					 tx_info->tid, tx_info->ip_family);
   392	
   393			xa_erase(&u_ctx->tid_list, tx_info->tid);
                                 ^^^^^^^^^^^^^^^^
Unchecked dereference.

   394		}
   395	

regards,
dan carpenter

Re: [bug report] cxgb4/ch_ktls: Clear resources when pf4 device is removed

[Ayush Sawal]

Hi Dan,

Thanks for pointing this out.
The u_ctx pointer in case of ch_ktls will always be valid.

So this condition if(u_ctx && u_ctx->detach) must be if(u_ctx->detach).
and xa_erase(&u_ctx->tid_list, tx_info->tid) doesnot requires a u_ctx check.

So I will send a fix which replaces if(u_ctx && u_ctx->detach) to 
if(u_ctx->detach)
and removes the checks for u_ctx pointer in ch_ktls driver.

Thanks,
Ayush

On 5/15/2021 4:12 PM, Dan Carpenter wrote:
> Hello Ayush Sawal,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 65e302a9bd57: "cxgb4/ch_ktls: Clear resources when pf4
> device is removed" from May 13, 2021, leads to the following Smatch
> complaint:
>
>      drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c:393 chcr_ktls_dev_del()
>      error: we previously assumed 'u_ctx' could be null (see line 374)
>
> drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
>     373		u_ctx = tx_info->adap->uld[CXGB4_ULD_KTLS].handle;
>     374		if (u_ctx && u_ctx->detach)
>                      ^^^^^
> Can u_ctx be NULL?
>
>     375			return;
>     376		/* clear l2t entry */
>     377		if (tx_info->l2te)
>     378			cxgb4_l2t_release(tx_info->l2te);
>     379	
>     380	#if IS_ENABLED(CONFIG_IPV6)
>     381		/* clear clip entry */
>     382		if (tx_info->ip_family == AF_INET6)
>     383			cxgb4_clip_release(netdev, (const u32 *)
>     384					   &tx_info->sk->sk_v6_rcv_saddr,
>     385					   1);
>     386	#endif
>     387	
>     388		/* clear tid */
>     389		if (tx_info->tid != -1) {
>     390			cxgb4_remove_tid(&tx_info->adap->tids, tx_info->tx_chan,
>     391					 tx_info->tid, tx_info->ip_family);
>     392	
>     393			xa_erase(&u_ctx->tid_list, tx_info->tid);
>                                   ^^^^^^^^^^^^^^^^
> Unchecked dereference.
>
>     394		}
>     395	
>
> regards,
> dan carpenter