Re: [PATCH net-next v2 06/15] idpf: continue expanding init task

["Tantilov, Emil S" <emil.s.tantilov@intel.com>]

On 4/11/2023 2:04 AM, Simon Horman wrote:
> On Mon, Apr 10, 2023 at 06:13:45PM -0700, Pavan Kumar Linga wrote:
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> 
> ...
> 
>> +/**
>> + * idpf_send_get_rx_ptype_msg - Send virtchnl for ptype info
>> + * @vport: virtual port data structure
>> + *
>> + * Returns 0 on success, negative on failure.
>> + */
>> +int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
>> +{
> 
> ...
> 
>> +	while (ptypes_recvd < max_ptype) {
>> +		err = idpf_wait_for_event(adapter, NULL, IDPF_VC_GET_PTYPE_INFO,
>> +					  IDPF_VC_GET_PTYPE_INFO_ERR);
>> +		if (err)
>> +			goto get_ptype_rel;
>> +
>> +		len = IDPF_DFLT_MBX_BUF_SIZE;
>> +		ptype_info = kcalloc(1, len, GFP_KERNEL);
>> +		if (!ptype_info) {
>> +			err = -ENOMEM;
>> +			goto clear_vc_flag;
>> +		}
>> +
>> +		memcpy(ptype_info, adapter->vc_msg, len);
>> +
>> +		ptypes_recvd += le16_to_cpu(ptype_info->num_ptypes);
>> +		if (ptypes_recvd > max_ptype) {
>> +			err = -EINVAL;
>> +			goto ptype_rel;
>> +		}
>> +
>> +		ptype_offset = sizeof(struct virtchnl2_get_ptype_info);
>> +
>> +		for (i = 0; i < le16_to_cpu(ptype_info->num_ptypes); i++) {
>> +			struct idpf_ptype_state pstate = { };
>> +			struct virtchnl2_ptype *ptype;
>> +			u16 id;
>> +
>> +			ptype = (struct virtchnl2_ptype *)
>> +					((u8 *)ptype_info + ptype_offset);
>> +
>> +			ptype_offset += IDPF_GET_PTYPE_SIZE(ptype);
>> +			if (ptype_offset > len) {
>> +				err = -EINVAL;
>> +				goto ptype_rel;
>> +			}
>> +
>> +			if (le16_to_cpu(ptype->ptype_id_10) ==
>> +							IDPF_INVALID_PTYPE_ID)
>> +				goto ptype_rel;
> 
> Hi Pavan,
> 
> The ptype_rel label will return err.
> But err is 0 here. Perhaps it should be set to a negative error code?
> 
> Flagged by Smatch as:
> 
> drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:1198 idpf_send_get_rx_ptype_msg() warn: missing error code 'err'

We'll address this in v3 along with the other smatch related hits.

Thanks,
Emil

> ...
> 
>> +	kfree(get_ptype_info);
>> +
>> +	return 0;
>> +
>> +ptype_rel:
>> +	kfree(ptype_info);
>> +clear_vc_flag:
>> +	clear_bit(__IDPF_VC_MSG_PENDING, adapter->flags);
>> +get_ptype_rel:
>> +	kfree(get_ptype_info);
>> +
>> +	return err;
>> +}
> 
> ...