Re: [dpdk-dev] [PATCH v2] net/tap: fix potential buffer overrun

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 29-Apr-19 2:58 PM, Wiles, Keith wrote:
> 
> 
>> On Apr 25, 2019, at 10:17 AM, Lipiec, Herakliusz <herakliusz.lipiec@intel.com> wrote:
>>
>> When secondary to primary process synchronization occours
>> there is no check for number of fds which could cause buffer overrun.
>>
>> Bugzilla ID: 252
>> Fixes: c9aa56edec8e ("net/tap: access primary process queues from secondary")
>> Cc: rasland@mellanox.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index e9fda8cf6..4a2ef5ce7 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -2111,6 +2111,10 @@ tap_mp_attach_queues(const char *port_name, struct rte_eth_dev *dev)
>> 	TAP_LOG(DEBUG, "Received IPC reply for %s", reply_param->port_name);
>>
>> 	/* Attach the queues from received file descriptors */
>> +	if (reply_param->rxq_count + reply_param->txq_count != reply->num_fds) {
>> +		TAP_LOG(ERR, "Unexpected number of fds received");
>> +		return -1;
>> +	}
> 
> This check is reasonable, but why is this being done on the receive side and not checked on the send side. There may need to be a check for num_fds being zero or greater than 8 as that is the limit to the number of FDs that can be handle by the IPC.

It is done below on the send side as well. This check is for 
sanity-checking external input. It's a socket, so anything (with 
matching UID) can write to it.

-- 
Thanks,
Anatoly