[DPDK/ethdev Bug 1536] net/tap: crash in tap pmd when using more than RTE_MP_MAX_FD_NUM rx queues

[DPDK/ethdev Bug 1536] net/tap: crash in tap pmd when using more than RTE_MP_MAX_FD_NUM rx queues

[bugzilla]

[-- Attachment #1: Type: text/plain, Size: 3080 bytes --]

https://bugs.dpdk.org/show_bug.cgi?id=1536

            Bug ID: 1536
           Summary: net/tap: crash in tap pmd when using more than
                    RTE_MP_MAX_FD_NUM rx queues
           Product: DPDK
           Version: 22.03
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: edwin.brossette@6wind.com
  Target Milestone: ---

Hello,

I have recently stumbled into an issue with my DPDK-based application running
the failsafe pmd. This pmd uses a tap device, with which my application fails
to start if more than 8 rx queues are used. This issue appears to be related to
this patch:
https://git.dpdk.org/dpdk/commit/?id=c36ce7099c2187926cd62cff7ebd479823554929

I have seen in the documentation that there was a limitation to 8 max queues
shared when using a tap device shared between multiple processes. However, my
application uses a single primary process, with no secondary process, but it
appears that I am still running into this limitation.

Now if we look at this small chunk of code:

memset(&msg, 0, sizeof(msg));
strlcpy(msg.name, TAP_MP_REQ_START_RXTX, sizeof(msg.name));
strlcpy(request_param->port_name, dev->data->name,
sizeof(request_param->port_name));
msg.len_param = sizeof(*request_param);
for (i = 0; i < dev->data->nb_tx_queues; i++) {
    msg.fds[fd_iterator++] = process_private->txq_fds[i];
    msg.num_fds++;
    request_param->txq_count++;
}
for (i = 0; i < dev->data->nb_rx_queues; i++) {
    msg.fds[fd_iterator++] = process_private->rxq_fds[i];
    msg.num_fds++;
    request_param->rxq_count++;
}
(Note that I am not using the latest DPDK version, but stable v23.11.1. But I
believe the issue is still present on latest.)

There are no checks on the maximum value i can take in the for loops. Since the
size of msg.fds is limited by the maximum of 8 queues shared between process
because of the IPC API, there is a potential buffer overflow which can happen
here.

See the struct declaration:
struct rte_mp_msg {
     char name[RTE_MP_MAX_NAME_LEN];
     int len_param;
     int num_fds;
     uint8_t param[RTE_MP_MAX_PARAM_LEN];
     int fds[RTE_MP_MAX_FD_NUM];
};

This means that if the number of queues used is more than 8, the program will
crash. This is what happens on my end as I get the following log:
*** stack smashing detected ***: terminated

Reverting the commit mentioned above fixes my issue. Also setting a check like
this works for me:

if (dev->data->nb_tx_queues + dev->data->nb_rx_queues > RTE_MP_MAX_FD_NUM)
     return -1;

I've made the changes on my local branch to fix my issue.

----------

Potential fixes discussed: 

1. Add "nb_rx_queues > RTE_MP_MAX_FD_NUM" check to not blindly update the
'msg.fds[]'

2. Prevent this to be a limit for tap PMD when there is only a primary process.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #2: Type: text/html, Size: 5127 bytes --]

[DPDK/ethdev Bug 1536] net/tap: crash in tap pmd when using more than RTE_MP_MAX_FD_NUM rx queues

[bugzilla]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

https://bugs.dpdk.org/show_bug.cgi?id=1536

Stephen Hemminger (stephen@networkplumber.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stephen@networkplumber.org
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Stephen Hemminger (stephen@networkplumber.org) ---
This was addressed in 24.11 by increasing the size of RTE_MP_MAX_FD_NUM to the
kernel limit (253)

See:


commit 5ff00bbc04d8338108241b083b7a6238208cfbc6
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Thu Sep 5 09:20:18 2024 -0700

    ipc: increase file descriptor maximum number

    The TAP and XDP driver both are limited to only 8 queues because of the
    small limit imposed by EAL.
    Increase the limit now, since this release allows changing ABI.

    Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
    Acked-by: Chengwen Feng <fengchengwen@huawei.com>
    Acked-by: Morten Brørup <mb@smartsharesystems.com>
    Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #2: Type: text/html, Size: 3832 bytes --]