[dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Haiyue Wang]

Do the VF message basic validation such as OPCODE message length check,
some special OPCODE message format check, to protect the i40e PMD from
malicious VF message attack.

Fixes: 4861cde46116 ("i40e: new poll mode driver")

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 092e0d3..d6e83e3 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -1295,6 +1295,7 @@
 	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
 	struct rte_pmd_i40e_mb_event_param ret_param;
 	bool b_op = TRUE;
+	int ret;
 
 	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
 		PMD_DRV_LOG(ERR, "invalid argument");
@@ -1309,6 +1310,30 @@
 		return;
 	}
 
+	/* perform basic checks on the msg */
+	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg, msglen);
+
+	/* perform additional checks specific to this driver */
+	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
+		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
+
+		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
+			ret = VIRTCHNL_ERR_PARAM;
+	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
+		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
+
+		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4))
+			ret = VIRTCHNL_ERR_PARAM;
+	}
+
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode %u, len %u",
+			    vf_id, opcode, msglen);
+		i40e_pf_host_send_msg_to_vf(vf, opcode,
+					    I40E_ERR_PARAM, NULL, 0);
+		return;
+	}
+
 	/**
 	 * initialise structure to send to user application
 	 * will return response from user in retval field
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Kevin Traynor]

On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> Do the VF message basic validation such as OPCODE message length check,
> some special OPCODE message format check, to protect the i40e PMD from
> malicious VF message attack.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> 

Missing Cc: stable@dpdk.org ? or there is some reason not to backport?

> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index 092e0d3..d6e83e3 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -1295,6 +1295,7 @@
>  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
>  	struct rte_pmd_i40e_mb_event_param ret_param;
>  	bool b_op = TRUE;
> +	int ret;
>  
>  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
>  		PMD_DRV_LOG(ERR, "invalid argument");
> @@ -1309,6 +1310,30 @@
>  		return;
>  	}
>  
> +	/* perform basic checks on the msg */
> +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg, msglen);
> +
> +	/* perform additional checks specific to this driver */
> +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
> +
> +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
> +			ret = VIRTCHNL_ERR_PARAM;
> +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
> +
> +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4))
> +			ret = VIRTCHNL_ERR_PARAM;
> +	}
> +
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode %u, len %u",
> +			    vf_id, opcode, msglen);
> +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> +					    I40E_ERR_PARAM, NULL, 0);
> +		return;
> +	}
> +
>  	/**
>  	 * initialise structure to send to user application
>  	 * will return response from user in retval field
> 

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Varghese, Vipin]

Hi Kevin,

A question, since the patch is fixing issue for 'i40e vf'  should not the sections for 'known limitations' or 'i40e PMD' be updated too?

Thanks
Vipin Varghese

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
> Sent: Thursday, January 10, 2019 11:18 PM
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the
> VF messages
> 
> On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > Do the VF message basic validation such as OPCODE message length
> > check, some special OPCODE message format check, to protect the i40e
> > PMD from malicious VF message attack.
> >
> > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> >
> 
> Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
> 
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> >  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index 092e0d3..d6e83e3 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -1295,6 +1295,7 @@
> >  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> >  	struct rte_pmd_i40e_mb_event_param ret_param;
> >  	bool b_op = TRUE;
> > +	int ret;
> >
> >  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> >  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> +1310,30 @@
> >  		return;
> >  	}
> >
> > +	/* perform basic checks on the msg */
> > +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > +msglen);
> > +
> > +	/* perform additional checks specific to this driver */
> > +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
> > +
> > +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
> > +			ret = VIRTCHNL_ERR_PARAM;
> > +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
> > +
> > +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) *
> 4))
> > +			ret = VIRTCHNL_ERR_PARAM;
> > +	}
> > +
> > +	if (ret) {
> > +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode
> %u, len %u",
> > +			    vf_id, opcode, msglen);
> > +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> > +					    I40E_ERR_PARAM, NULL, 0);
> > +		return;
> > +	}
> > +
> >  	/**
> >  	 * initialise structure to send to user application
> >  	 * will return response from user in retval field
> >

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Kevin Traynor]

On 01/11/2019 02:53 AM, Varghese, Vipin wrote:
> Hi Kevin,
> 
> A question, since the patch is fixing issue for 'i40e vf'  should not the sections for 'known limitations' or 'i40e PMD' be updated too?
> 

Hi Vipin, I don't think so, but it's a question for i40e maintainer.

Kevin.


> Thanks
> Vipin Varghese
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
>> Sent: Thursday, January 10, 2019 11:18 PM
>> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the
>> VF messages
>>
>> On 01/10/2019 12:07 PM, Haiyue Wang wrote:
>>> Do the VF message basic validation such as OPCODE message length
>>> check, some special OPCODE message format check, to protect the i40e
>>> PMD from malicious VF message attack.
>>>
>>> Fixes: 4861cde46116 ("i40e: new poll mode driver")
>>>
>>
>> Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
>>
>>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>>> ---
>>>  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
>>>  1 file changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
>>> index 092e0d3..d6e83e3 100644
>>> --- a/drivers/net/i40e/i40e_pf.c
>>> +++ b/drivers/net/i40e/i40e_pf.c
>>> @@ -1295,6 +1295,7 @@
>>>  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
>>>  	struct rte_pmd_i40e_mb_event_param ret_param;
>>>  	bool b_op = TRUE;
>>> +	int ret;
>>>
>>>  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
>>>  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
>> +1310,30 @@
>>>  		return;
>>>  	}
>>>
>>> +	/* perform basic checks on the msg */
>>> +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
>>> +msglen);
>>> +
>>> +	/* perform additional checks specific to this driver */
>>> +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
>>> +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
>>> +
>>> +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
>>> +			ret = VIRTCHNL_ERR_PARAM;
>>> +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
>>> +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
>>> +
>>> +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) *
>> 4))
>>> +			ret = VIRTCHNL_ERR_PARAM;
>>> +	}
>>> +
>>> +	if (ret) {
>>> +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode
>> %u, len %u",
>>> +			    vf_id, opcode, msglen);
>>> +		i40e_pf_host_send_msg_to_vf(vf, opcode,
>>> +					    I40E_ERR_PARAM, NULL, 0);
>>> +		return;
>>> +	}
>>> +
>>>  	/**
>>>  	 * initialise structure to send to user application
>>>  	 * will return response from user in retval field
>>>
> 

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Zhang, Qi Z]

Hi Vipin:

> -----Original Message-----
> From: Varghese, Vipin
> Sent: Friday, January 11, 2019 10:54 AM
> To: Kevin Traynor <ktraynor@redhat.com>; Wang, Haiyue
> <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF
> messages
> 
> Hi Kevin,
> 
> A question, since the patch is fixing issue for 'i40e vf'  should not the sections
> for 'known limitations' or 'i40e PMD' be updated too?

The patch is going to fix some issue not be recorded as knowing limitation previously, so I didn’t see the necessary to update the doc.
But please let me know if I missed your point.

> 
> Thanks
> Vipin Varghese
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
> > Sent: Thursday, January 10, 2019 11:18 PM
> > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation
> > on the VF messages
> >
> > On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > > Do the VF message basic validation such as OPCODE message length
> > > check, some special OPCODE message format check, to protect the i40e
> > > PMD from malicious VF message attack.
> > >
> > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > >
> >
> > Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
> >
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > > index 092e0d3..d6e83e3 100644
> > > --- a/drivers/net/i40e/i40e_pf.c
> > > +++ b/drivers/net/i40e/i40e_pf.c
> > > @@ -1295,6 +1295,7 @@
> > >  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > >  	struct rte_pmd_i40e_mb_event_param ret_param;
> > >  	bool b_op = TRUE;
> > > +	int ret;
> > >
> > >  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > >  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> > +1310,30 @@
> > >  		return;
> > >  	}
> > >
> > > +	/* perform basic checks on the msg */
> > > +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > > +msglen);
> > > +
> > > +	/* perform additional checks specific to this driver */
> > > +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > > +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
> > > +
> > > +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
> > > +			ret = VIRTCHNL_ERR_PARAM;
> > > +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > > +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
> > > +
> > > +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) *
> > 4))
> > > +			ret = VIRTCHNL_ERR_PARAM;
> > > +	}
> > > +
> > > +	if (ret) {
> > > +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode
> > %u, len %u",
> > > +			    vf_id, opcode, msglen);
> > > +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> > > +					    I40E_ERR_PARAM, NULL, 0);
> > > +		return;
> > > +	}
> > > +
> > >  	/**
> > >  	 * initialise structure to send to user application
> > >  	 * will return response from user in retval field
> > >

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Varghese, Vipin]

Hi,

Thanks Kevin for redirecting to the maintainer. Appreciate the help.

Thanks Qi Z Zhang for the update. But the reason for request is because I did not find the update in patches for 'release notes, faq or i40e' documentation. Hence, I was forced to assume this is known bug.

Will wait to see an update on either release notes, faq or i40e documentation. 

Thanks
Vipin Varghese

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, January 11, 2019 6:59 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; Kevin Traynor
> <ktraynor@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the
> VF messages
> 
> Hi Vipin:
> 
> > -----Original Message-----
> > From: Varghese, Vipin
> > Sent: Friday, January 11, 2019 10:54 AM
> > To: Kevin Traynor <ktraynor@redhat.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation
> > on the VF messages
> >
> > Hi Kevin,
> >
> > A question, since the patch is fixing issue for 'i40e vf'  should not
> > the sections for 'known limitations' or 'i40e PMD' be updated too?
> 
> The patch is going to fix some issue not be recorded as knowing limitation
> previously, so I didn’t see the necessary to update the doc.
> But please let me know if I missed your point.
> 
> >
> > Thanks
> > Vipin Varghese
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
> > > Sent: Thursday, January 10, 2019 11:18 PM
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > validation on the VF messages
> > >
> > > On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > > > Do the VF message basic validation such as OPCODE message length
> > > > check, some special OPCODE message format check, to protect the
> > > > i40e PMD from malicious VF message attack.
> > > >
> > > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > > >
> > >
> > > Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
> > >
> > > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
> > > >  1 file changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_pf.c
> > > > b/drivers/net/i40e/i40e_pf.c index 092e0d3..d6e83e3 100644
> > > > --- a/drivers/net/i40e/i40e_pf.c
> > > > +++ b/drivers/net/i40e/i40e_pf.c
> > > > @@ -1295,6 +1295,7 @@
> > > >  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > > >  	struct rte_pmd_i40e_mb_event_param ret_param;
> > > >  	bool b_op = TRUE;
> > > > +	int ret;
> > > >
> > > >  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > > >  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> > > +1310,30 @@
> > > >  		return;
> > > >  	}
> > > >
> > > > +	/* perform basic checks on the msg */
> > > > +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > > > +msglen);
> > > > +
> > > > +	/* perform additional checks specific to this driver */
> > > > +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > > > +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key *)msg;
> > > > +
> > > > +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
> > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > > > +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut *)msg;
> > > > +
> > > > +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) *
> > > 4))
> > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > +	}
> > > > +
> > > > +	if (ret) {
> > > > +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode
> > > %u, len %u",
> > > > +			    vf_id, opcode, msglen);
> > > > +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> > > > +					    I40E_ERR_PARAM, NULL, 0);
> > > > +		return;
> > > > +	}
> > > > +
> > > >  	/**
> > > >  	 * initialise structure to send to user application
> > > >  	 * will return response from user in retval field
> > > >

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Zhang, Qi Z]

> -----Original Message-----
> From: Varghese, Vipin
> Sent: Monday, January 14, 2019 10:33 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Kevin Traynor
> <ktraynor@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF
> messages
> 
> Hi,
> 
> Thanks Kevin for redirecting to the maintainer. Appreciate the help.
> 
> Thanks Qi Z Zhang for the update. But the reason for request is because I did
> not find the update in patches for 'release notes, faq or i40e' documentation.
> Hence, I was forced to assume this is known bug.
> 
> Will wait to see an update on either release notes, faq or i40e documentation.

Sorry I still didn't get your point
The issue is not a knowing issue, as a common bug, it is observed by somebody and report to dev team.
So I didn't see anything we can update on the i40e documentation since the issue is fixed.
And it also does not impact any user experience, (no new feature, no knowing issue fix) so I didn’t see the point to update release notes also.


> 
> Thanks
> Vipin Varghese
> 
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Friday, January 11, 2019 6:59 PM
> > To: Varghese, Vipin <vipin.varghese@intel.com>; Kevin Traynor
> > <ktraynor@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> > dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation
> > on the VF messages
> >
> > Hi Vipin:
> >
> > > -----Original Message-----
> > > From: Varghese, Vipin
> > > Sent: Friday, January 11, 2019 10:54 AM
> > > To: Kevin Traynor <ktraynor@redhat.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > validation on the VF messages
> > >
> > > Hi Kevin,
> > >
> > > A question, since the patch is fixing issue for 'i40e vf'  should
> > > not the sections for 'known limitations' or 'i40e PMD' be updated too?
> >
> > The patch is going to fix some issue not be recorded as knowing
> > limitation previously, so I didn’t see the necessary to update the doc.
> > But please let me know if I missed your point.
> >
> > >
> > > Thanks
> > > Vipin Varghese
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
> > > > Sent: Thursday, January 10, 2019 11:18 PM
> > > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi
> > > > Z <qi.z.zhang@intel.com>
> > > > Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > > validation on the VF messages
> > > >
> > > > On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > > > > Do the VF message basic validation such as OPCODE message length
> > > > > check, some special OPCODE message format check, to protect the
> > > > > i40e PMD from malicious VF message attack.
> > > > >
> > > > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > > > >
> > > >
> > > > Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
> > > >
> > > > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > > > ---
> > > > >  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
> > > > >  1 file changed, 25 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/i40e/i40e_pf.c
> > > > > b/drivers/net/i40e/i40e_pf.c index 092e0d3..d6e83e3 100644
> > > > > --- a/drivers/net/i40e/i40e_pf.c
> > > > > +++ b/drivers/net/i40e/i40e_pf.c
> > > > > @@ -1295,6 +1295,7 @@
> > > > >  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > > > >  	struct rte_pmd_i40e_mb_event_param ret_param;
> > > > >  	bool b_op = TRUE;
> > > > > +	int ret;
> > > > >
> > > > >  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > > > >  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> > > > +1310,30 @@
> > > > >  		return;
> > > > >  	}
> > > > >
> > > > > +	/* perform basic checks on the msg */
> > > > > +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > > > > +msglen);
> > > > > +
> > > > > +	/* perform additional checks specific to this driver */
> > > > > +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > > > > +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key
> > > > > +*)msg;
> > > > > +
> > > > > +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1) * 4))
> > > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > > +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > > > > +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut
> > > > > +*)msg;
> > > > > +
> > > > > +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX + 1) *
> > > > 4))
> > > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > > +	}
> > > > > +
> > > > > +	if (ret) {
> > > > > +		PMD_DRV_LOG(ERR, "Invalid message from VF %u, opcode
> > > > %u, len %u",
> > > > > +			    vf_id, opcode, msglen);
> > > > > +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> > > > > +					    I40E_ERR_PARAM, NULL, 0);
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > >  	/**
> > > > >  	 * initialise structure to send to user application
> > > > >  	 * will return response from user in retval field
> > > > >

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Varghese, Vipin]

Hi Qi Z Zhang,

snipped
> >
> > Hi,
> >
> > Thanks Kevin for redirecting to the maintainer. Appreciate the help.
> >
> > Thanks Qi Z Zhang for the update. But the reason for request is
> > because I did not find the update in patches for 'release notes, faq or i40e'
> documentation.
> > Hence, I was forced to assume this is known bug.
> >
> > Will wait to see an update on either release notes, faq or i40e
> documentation.
> 
> Sorry I still didn't get your point
> The issue is not a knowing issue, as a common bug, it is observed by somebody
> and report to dev team.
> So I didn't see anything we can update on the i40e documentation since the
> issue is fixed.
> And it also does not impact any user experience, (no new feature, no knowing
> issue fix) so I didn’t see the point to update release notes also.

For the last couple of months, we have been receiving queries from customers like 'security issue for Side band channel, spectre, mail box, ring communication, vhost interface etc'. As per the update 'PMD from malicious VF message attack', having this tracked in release notes, faq or i40e serves the purpose of information update. So in my humble opinion security update patch should be mentioned in documentation with working Firmware.

I will leave this community opinion of either updating in a common place or PMD for such tracking.

> 
> 
> >
> > Thanks
> > Vipin Varghese
> >
> > > -----Original Message-----
> > > From: Zhang, Qi Z
> > > Sent: Friday, January 11, 2019 6:59 PM
> > > To: Varghese, Vipin <vipin.varghese@intel.com>; Kevin Traynor
> > > <ktraynor@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> > > dev@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > validation on the VF messages
> > >
> > > Hi Vipin:
> > >
> > > > -----Original Message-----
> > > > From: Varghese, Vipin
> > > > Sent: Friday, January 11, 2019 10:54 AM
> > > > To: Kevin Traynor <ktraynor@redhat.com>; Wang, Haiyue
> > > > <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > > validation on the VF messages
> > > >
> > > > Hi Kevin,
> > > >
> > > > A question, since the patch is fixing issue for 'i40e vf'  should
> > > > not the sections for 'known limitations' or 'i40e PMD' be updated too?
> > >
> > > The patch is going to fix some issue not be recorded as knowing
> > > limitation previously, so I didn’t see the necessary to update the doc.
> > > But please let me know if I missed your point.
> > >
> > > >
> > > > Thanks
> > > > Vipin Varghese
> > > >
> > > > > -----Original Message-----
> > > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Kevin Traynor
> > > > > Sent: Thursday, January 10, 2019 11:18 PM
> > > > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang,
> > > > > Qi Z <qi.z.zhang@intel.com>
> > > > > Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > > > validation on the VF messages
> > > > >
> > > > > On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > > > > > Do the VF message basic validation such as OPCODE message
> > > > > > length check, some special OPCODE message format check, to
> > > > > > protect the i40e PMD from malicious VF message attack.
> > > > > >
> > > > > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > > > > >
> > > > >
> > > > > Missing Cc: stable@dpdk.org ? or there is some reason not to backport?
> > > > >
> > > > > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > > > > ---
> > > > > >  drivers/net/i40e/i40e_pf.c | 25 +++++++++++++++++++++++++
> > > > > >  1 file changed, 25 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/net/i40e/i40e_pf.c
> > > > > > b/drivers/net/i40e/i40e_pf.c index 092e0d3..d6e83e3 100644
> > > > > > --- a/drivers/net/i40e/i40e_pf.c
> > > > > > +++ b/drivers/net/i40e/i40e_pf.c
> > > > > > @@ -1295,6 +1295,7 @@
> > > > > >  	uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > > > > >  	struct rte_pmd_i40e_mb_event_param ret_param;
> > > > > >  	bool b_op = TRUE;
> > > > > > +	int ret;
> > > > > >
> > > > > >  	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > > > > >  		PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> > > > > +1310,30 @@
> > > > > >  		return;
> > > > > >  	}
> > > > > >
> > > > > > +	/* perform basic checks on the msg */
> > > > > > +	ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > > > > > +msglen);
> > > > > > +
> > > > > > +	/* perform additional checks specific to this driver */
> > > > > > +	if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > > > > > +		struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key
> > > > > > +*)msg;
> > > > > > +
> > > > > > +		if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1)
> * 4))
> > > > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > > > +	} else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > > > > > +		struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut
> > > > > > +*)msg;
> > > > > > +
> > > > > > +		if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX
> + 1) *
> > > > > 4))
> > > > > > +			ret = VIRTCHNL_ERR_PARAM;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (ret) {
> > > > > > +		PMD_DRV_LOG(ERR, "Invalid message from VF %u,
> opcode
> > > > > %u, len %u",
> > > > > > +			    vf_id, opcode, msglen);
> > > > > > +		i40e_pf_host_send_msg_to_vf(vf, opcode,
> > > > > > +					    I40E_ERR_PARAM, NULL, 0);
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > >  	/**
> > > > > >  	 * initialise structure to send to user application
> > > > > >  	 * will return response from user in retval field
> > > > > >

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Zhang, Qi Z]

> -----Original Message-----
> From: Wang, Haiyue
> Sent: Thursday, January 10, 2019 8:08 PM
> To: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>
> Subject: [PATCH v1] net/i40e: perform basic validation on the VF messages
> 
> Do the VF message basic validation such as OPCODE message length check,
> some special OPCODE message format check, to protect the i40e PMD from
> malicious VF message attack.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")

> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Cc: stable@dpdk.org added when

Applied to dpdk-next-net-intel.

Thanks
Qi