Re: DPDK Coverity issue 127559

Re: DPDK Coverity issue 127559

[Rahul Lakkireddy]

Hi all,

On Monday, July 07/04/16, 2016 at 08:29:49 -0700, john.mcnamara@intel.com wrote:
> Hi Rahul,
> 
> This is an automated email in relation to a new Coverity static code analysis
> issue in DPDK. Details of the issue are below.
> 
[...]

> Git commit data and Coverity defect information below.
> 
> Commit data
> ===========
> 
> Commit: net/cxgbe: support EEPROM access
> Id:     fe0bd9ee5da3fd52766458a5d0fa9a8728182be1
> Author: Rahul Lakkireddy
> Email:  rahul.lakkireddy@chelsio.com
> Date:   Fri May  6 08:43:18 2016 +0530
> 
> Defect information
> ==================
> 
> /drivers/net/cxgbe/cxgbe_ethdev.c: 919 in cxgbe_set_eeprom()
> *** CID 127559:    (TAINTED_SCALAR)
> 913     	}
> 914
> 915     	if (!err)
> 916     		err = t4_seeprom_wp(adapter, true);
> 917     out:
> 918     	if (buf != eeprom->data)
> >>>     CID 127559:    (TAINTED_SCALAR)
> >>>     Passing tainted variable "buf" to a tainted sink.
> 919     		rte_free(buf);
> 920     	return err;
> 921     }
> 922
> 923     static int cxgbe_get_regs_len(struct rte_eth_dev *eth_dev)
> 924     {
> /drivers/net/cxgbe/cxgbe_ethdev.c: 910 in cxgbe_set_eeprom()
> 904     	}
> 905
> 906     	err = t4_seeprom_wp(adapter, false);
> 907     	if (err)
> 908     		goto out;
> 909
> >>>     CID 127559:    (TAINTED_SCALAR)
> >>>     Assigning: "p" = "(u32 *)buf". Both are now tainted.
> 910     	for (p = (u32 *)buf; !err && aligned_len; aligned_len -= 4, p++) {
> 911     		err = eeprom_wr_phys(adapter, aligned_offset, *p);
> 912     		aligned_offset += 4;
> 913     	}
> 914
> 915     	if (!err)
> 

I'm not an expert in Coverity and am having trouble understanding what
the defect is and need some clarification.  Is it telling me that "buf"
is being used without doing lower and upper bounds check?

Thanks,
Rahul

Re: DPDK Coverity issue 127559

[Mcnamara, John]

> -----Original Message-----
> From: Rahul Lakkireddy [mailto:rahul.lakkireddy@chelsio.com]
> Sent: Tuesday, July 19, 2016 9:16 AM
> To: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org
> Cc: Kumar Sanghvi <kumaras@chelsio.com>; Nirranjan Kirubaharan
> <nirranjan@chelsio.com>; Arjun V <arjun@chelsio.com>
> Subject: Re: DPDK Coverity issue 127559
> 
> Hi all,
> 
> > 907     	if (err)
> > 908     		goto out;
> > 909
> > >>>     CID 127559:    (TAINTED_SCALAR)
> > >>>     Assigning: "p" = "(u32 *)buf". Both are now tainted.
> > 910     	for (p = (u32 *)buf; !err && aligned_len; aligned_len -= 4,
> p++) {
> > 911     		err = eeprom_wr_phys(adapter, aligned_offset, *p);
> > 912     		aligned_offset += 4;
> > 913     	}
> > 914
> > 915     	if (!err)
> >
> 
> I'm not an expert in Coverity and am having trouble understanding what the
> defect is and need some clarification.  Is it telling me that "buf"
> is being used without doing lower and upper bounds check?


Hi,

There is a lot more context when you view the defect through the Coverity web interface.

Basically it is saying that the data in buf comes from the user ("is tainted") and as such can't be trusted. Usually you need to provide some bound, or other, checks to protect against/untaint the data. However, in this case it looks like the data is coming from an eeprom rather than a "user" so it is probably a false positive.

However, you should look at the full context online and decide for yourself. Then update the status in the Coverity interface and add a comment on your decision.

John