From: Stephen Hemminger <stephen@networkplumber.org>
To: "Zhang, Ke1X" <ke1x.zhang@intel.com>
Cc: "Li, Xiaoyun" <xiaoyun.li@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"Xing, Beilei" <beilei.xing@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH v5] fix mbuf release function point corrupt in multi-process
Date: Thu, 12 May 2022 18:57:29 -0700 [thread overview]
Message-ID: <20220512185729.6886f826@hermes.local> (raw)
In-Reply-To: <PH7PR11MB5984A8FA6CD9BE406E18D11ADCCA9@PH7PR11MB5984.namprd11.prod.outlook.com>
On Fri, 13 May 2022 01:34:02 +0000
"Zhang, Ke1X" <ke1x.zhang@intel.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Friday, May 13, 2022 1:27 AM
> > To: Zhang, Ke1X <ke1x.zhang@intel.com>
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>; dev@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@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.
next prev parent reply other threads:[~2022-05-13 1:57 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-02 9:51 [PATCH] net/iavf: fix iavf crashed on dev_stop when running in multi-process mode Ke Zhang
2022-04-03 2:07 ` Zhang, Qi Z
2022-04-14 9:29 ` [PATCH v2] " Ke Zhang
2022-04-18 6:41 ` Zhang, Qi Z
2022-05-07 6:24 ` [PATCH v3] fix mbuf release function point corrupt in multi-process Ke Zhang
2022-05-09 1:16 ` Ke Zhang
2022-05-09 1:35 ` Zhang, Qi Z
2022-05-10 2:54 ` [PATCH v4] " Ke Zhang
2022-05-12 2:21 ` [PATCH v5] " Ke Zhang
2022-05-12 5:57 ` Ke Zhang
2022-05-12 17:26 ` Stephen Hemminger
2022-05-13 1:34 ` Zhang, Ke1X
2022-05-13 1:57 ` Stephen Hemminger [this message]
2022-05-12 7:44 ` Ke Zhang
2022-05-16 6:41 ` [PATCH v6] " Ke Zhang
2022-05-16 6:55 ` Ke Zhang
2022-05-17 7:27 ` Zhang, Qi Z
2022-05-19 7:36 ` [PATCH v7] net/iavf: " Ke Zhang
2022-05-19 9:25 ` Zhang, Qi Z
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220512185729.6886f826@hermes.local \
--to=stephen@networkplumber.org \
--cc=beilei.xing@intel.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=ke1x.zhang@intel.com \
--cc=xiaoyun.li@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).