Re: [dpdk-dev] [PATCH] rte_ethdev: fix unsafe memory access by calling RX callback.(Internet mail)

["tgw_team(腾讯网关团队)" <tgw_team@tencent.com>]

Sorry, I`ll use a real name in patch v2.

I don't think this is a TOCTOU question.
The original code works fine when compiled with the -O3 option.
At this point the compiler will optimize to one memory access.
But when compiled with -O0, there will be two memory accesses, which is wrong.
This change was modified with reference to the rte_eth_tx_burst function.

On Wed,  4 Mar 2020 22:05:43 +0800
Tencent TGW team <quzeyao@gmail.com> wrote:

> When compiling with -O0,
> the compiler does not optimize two memory accesses into one.
> Leads to accessing a null pointer when calling the RX callback.
> The way to access the TX callback is correct.
> 
> Signed-off-by: Tencent TGW team <tgw_team@tencent.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index d1a593ad1..35eb580ff 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4388,10 +4388,8 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
>                                     rx_pkts, nb_pkts);
>  
>  #ifdef RTE_ETHDEV_RXTX_CALLBACKS
> -     if (unlikely(dev->post_rx_burst_cbs[queue_id] != NULL)) {
> -             struct rte_eth_rxtx_callback *cb =
> -                             dev->post_rx_burst_cbs[queue_id];
> -
> +     struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
> +     if (unlikely(cb != NULL)) {
>                do {
>                        nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
>                                                nb_pkts, cb->param);

To fix TOCTOU like this you need to use something like READ_ONCE()
which does cast to volatile.

For Developer Certificate of Origin (Signed-off-by) you must use a person
not a team. It is a legal requirement. If you don't know what DCO is you should
read what it means.

Developer's Certificate of Origin 1.1
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.

then you just add a line saying::

        Signed-off-by: Random J Developer <random@developer.example.org>

using your real name (sorry, no pseudonyms or anonymous contributions.)