[dpdk-dev] [PATCH v6 08/12] net/dpaa2: fix possible use of uninitialized vars

[Andrzej Ostruszka]

On 11/4/19 12:46 PM, Hemant Agrawal wrote:
> Hi Andrzej

Hello Hemant,

Thank you for your comments.

>> -----Original Message-----
>> From: Andrzej Ostruszka <aostruszka at marvell.com>
[...]
>> a/drivers/net/dpaa2/mc/dpkg.c b/drivers/net/dpaa2/mc/dpkg.c index
>> 80f94f40e..7aa63ea12 100644
>> --- a/drivers/net/dpaa2/mc/dpkg.c
>> +++ b/drivers/net/dpaa2/mc/dpkg.c
>> @@ -63,7 +63,7 @@ dpkg_prepare_key_cfg(const struct dpkg_profile_cfg
>> *cfg, uint8_t *key_cfg_buf)
>>  		dpkg_set_field(extr->extract_type, EXTRACT_TYPE,
>>  			       cfg->extracts[i].type);
>>
> [Hemant] Please add a check here to avoid array overrun
> +		if (extr->num_of_byte_masks > DPKG_NUM_OF_MASKS)
> +			return -EINVAL;
> +

Will do, thank you.

>> -		for (j = 0; j < DPKG_NUM_OF_MASKS; j++) {
>> +		for (j = 0; j < extr->num_of_byte_masks; j++) {
>>  			extr->masks[j].mask = cfg-
>>> extracts[i].masks[j].mask;
>>  			extr->masks[j].offset =
>>  				cfg->extracts[i].masks[j].offset;
>> diff --git a/drivers/net/dpaa2/mc/dpni.c b/drivers/net/dpaa2/mc/dpni.c
>> index 0950ee007..89a64b0ab 100644
>> --- a/drivers/net/dpaa2/mc/dpni.c
>> +++ b/drivers/net/dpaa2/mc/dpni.c
>> @@ -1839,10 +1839,13 @@ int dpni_set_congestion_notification(struct
>> fsl_mc_io *mc_io,
>>  	cmd_params->qtype = qtype;
>>  	cmd_params->tc = tc_id;
>>  	cmd_params->congestion_point = cfg->cg_point;
>> -	cmd_params->cgid = (uint8_t)cfg->cgid;
>> -	cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
>> +	if (cfg->cg_point == DPNI_CP_CONGESTION_GROUP)
>> +		cmd_params->cgid = (uint8_t)cfg->cgid;
>> +	if (cfg->dest_cfg.dest_type != DPNI_DEST_NONE) {
>> +		cmd_params->dest_id = cpu_to_le32(cfg-
>>> dest_cfg.dest_id);
>> +		cmd_params->dest_priority = cfg->dest_cfg.priority;
>> +	}
> [Hemant]  What is the explicit error you got here? 
> In the calling function, we are resetting the structure to 0. Which should have avoided
> Any un-initialized var errors? 
> struct dpni_congestion_notification_cfg cong_notif_cfg = {0};

Indeed there is no warning from compiler here at the current version.
The original version was developed against 18.05 and this is what I used
to make compiler happy.  I've kept this change (even though compiler
does not report anything here) since to me it looked like a good change
(use the cgid only in case of "congestion group" and use dest_* only
when not DEST_NONE).

You have obviously more authority here so I will remove these changes
(unless you prefer them to stay - in which case please indicate so).

Again thank you for the comments.

Regards
Andrzej