[PATCH v5] fix mbuf release function point corrupt in multi-process

[Stephen Hemminger]

On Fri, 13 May 2022 01:34:02 +0000
"Zhang, Ke1X" <ke1x.zhang at intel.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen at networkplumber.org>
> > Sent: Friday, May 13, 2022 1:27 AM
> > To: Zhang, Ke1X <ke1x.zhang at intel.com>
> > Cc: Li, Xiaoyun <xiaoyun.li at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>;
> > Xing, Beilei <beilei.xing at intel.com>; dev at dpdk.org
> > Subject: Re: [PATCH v5] fix mbuf release function point corrupt in multi-
> > process
> > 
> > On Thu, 12 May 2022 05:57:19 +0000
> > Ke Zhang <ke1x.zhang at intel.com> wrote:
> >   
> > >
> > > -static const struct iavf_rxq_ops def_rxq_ops = {
> > > -	.release_mbufs = release_rxq_mbufs,
> > > +static
> > > +struct iavf_rxq_ops iavf_rxq_release_mbufs_ops[] = {
> > > +	[IAVF_REL_MBUFS_DEFAULT].release_mbufs = release_rxq_mbufs,
> > > +	[IAVF_REL_MBUFS_SSE_VEC].release_mbufs =  
> > iavf_rx_queue_release_mbufs_sse,  
> > >  };
> > >
> > > -static const struct iavf_txq_ops def_txq_ops = {
> > > -	.release_mbufs = release_txq_mbufs,
> > > +static
> > > +struct iavf_txq_ops iavf_txq_release_mbufs_ops[] = {
> > > +	[IAVF_REL_MBUFS_DEFAULT].release_mbufs = release_txq_mbufs,
> > > +	[IAVF_REL_MBUFS_SSE_VEC].release_mbufs =  
> > iavf_tx_queue_release_mbufs_sse,  
> > > +	[IAVF_REL_MBUFS_AVX512_VEC].release_mbufs =  
> > iavf_tx_queue_release_mbufs_avx512,  
> > >  };  
> > 
> > Did you have to take const off of these?  
> 
> Thanks for your comments, I check the other code like linux kernel , I found there are no const for the function pointer, like:
> 
> static struct pci_driver ice_driver = {
> 	.name = KBUILD_MODNAME,
> 	.id_table = ice_pci_tbl,
> 	.probe = ice_probe,
> 	.remove = ice_remove,
> #ifdef CONFIG_PM
> 	.driver.pm = &ice_pm_ops,
> #endif /* CONFIG_PM */
> 	.shutdown = ice_shutdown,
> #ifndef STATIC_QOS_CFG_SUPPORT
> 	.sriov_configure = ice_sriov_configure,
> #endif /* !STATIC_QOS_CFG_SUPPORT */
> #ifdef HAVE_RHEL7_PCI_DRIVER_RH
> 	.pci_driver_rh = &ice_driver_rh,
> #endif /* HAVE_RHEL7_PCI_DRIVER_RH */
> 	.err_handler = &ice_pci_err_handler
> };
> 
> So I don't add the const.
> 

This is not the kernel!  The kernel pci device has other reasons
it can't be const. This is because the Linux kernel pci_driver structure
gets linked into the list of PCI devices. The kernel should be splitting the device object
(pci_driver) from the functions by introducing a new pci_driver_ops. 
But this would require lots of extra work; the kernel hardening project may get to it.

As a general rule: any table with function pointers should be const for security reasons.
The DPDK has less security requirements than the kernel and less security testing,
but developers should try to avoid issues if possible.