DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] bugfix for raw ntb
@ 2021-04-21  2:08 Min Hu (Connor)
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index Min Hu (Connor)
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result Min Hu (Connor)
  0 siblings, 2 replies; 12+ messages in thread
From: Min Hu (Connor) @ 2021-04-21  2:08 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, xiaoyun.li, jingjing.wu

This patchset contains two bugfix for raw ntb.

Chengwen Feng (2):
  raw/ntb: check spad user index
  raw/ntb: check malloc result

 drivers/raw/ntb/ntb.c          | 13 +++++++++++++
 drivers/raw/ntb/ntb_hw_intel.c |  5 +++++
 2 files changed, 18 insertions(+)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  2:08 [dpdk-dev] [PATCH 0/2] bugfix for raw ntb Min Hu (Connor)
@ 2021-04-21  2:08 ` Min Hu (Connor)
  2021-04-21  3:31   ` Li, Xiaoyun
  2021-04-22  5:05   ` Li, Xiaoyun
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result Min Hu (Connor)
  1 sibling, 2 replies; 12+ messages in thread
From: Min Hu (Connor) @ 2021-04-21  2:08 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, xiaoyun.li, jingjing.wu

From: Chengwen Feng <fengchengwen@huawei.com>

This patch adds checking spad user index validity when set or get attr.

Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/raw/ntb/ntb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c
index 6dd213e..0f0e3f2 100644
--- a/drivers/raw/ntb/ntb.c
+++ b/drivers/raw/ntb/ntb.c
@@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const char *attr_name,
 		if (hw->ntb_ops->spad_write == NULL)
 			return -ENOTSUP;
 		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
+		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
+			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
+			return -EINVAL;
+		}
 		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
 					   1, attr_value);
 		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")",
@@ -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char *attr_name,
 		if (hw->ntb_ops->spad_read == NULL)
 			return -ENOTSUP;
 		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
+		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
+			NTB_LOG(ERR, "Attribute (%s) out of range", attr_name);
+			return -EINVAL;
+		}
 		*attr_value = (*hw->ntb_ops->spad_read)(dev,
 				hw->spad_user_list[index], 0);
 		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
-- 
2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result
  2021-04-21  2:08 [dpdk-dev] [PATCH 0/2] bugfix for raw ntb Min Hu (Connor)
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index Min Hu (Connor)
@ 2021-04-21  2:08 ` Min Hu (Connor)
  2021-04-21  3:33   ` Li, Xiaoyun
  1 sibling, 1 reply; 12+ messages in thread
From: Min Hu (Connor) @ 2021-04-21  2:08 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, xiaoyun.li, jingjing.wu

From: Chengwen Feng <fengchengwen@huawei.com>

This patch adds checking for rte_zmalloc() result when init intel ntb
device, also fix the same bug when start ntb device.

Fixes: 034c328eb025 ("raw/ntb: support Intel NTB")
Fixes: c39d1e082a4b ("raw/ntb: setup queues")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/raw/ntb/ntb.c          | 5 +++++
 drivers/raw/ntb/ntb_hw_intel.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c
index 0f0e3f2..6703bb5 100644
--- a/drivers/raw/ntb/ntb.c
+++ b/drivers/raw/ntb/ntb.c
@@ -923,6 +923,11 @@ ntb_dev_start(struct rte_rawdev *dev)
 
 	hw->peer_mw_base = rte_zmalloc("ntb_peer_mw_base", hw->mw_cnt *
 					sizeof(uint64_t), 0);
+	if (hw->peer_mw_base == NULL) {
+		NTB_LOG(ERR, "Cannot allocate memory for peer mw base.");
+		ret = -ENOMEM;
+		goto err_q_init;
+	}
 
 	if (hw->ntb_ops->spad_read == NULL) {
 		ret = -ENOTSUP;
diff --git a/drivers/raw/ntb/ntb_hw_intel.c b/drivers/raw/ntb/ntb_hw_intel.c
index 4427e11..a742e8f 100644
--- a/drivers/raw/ntb/ntb_hw_intel.c
+++ b/drivers/raw/ntb/ntb_hw_intel.c
@@ -148,6 +148,11 @@ intel_ntb_dev_init(const struct rte_rawdev *dev)
 
 	hw->mw_size = rte_zmalloc("ntb_mw_size",
 				  hw->mw_cnt * sizeof(uint64_t), 0);
+	if (hw->mw_size == NULL) {
+		NTB_LOG(ERR, "Cannot allocate memory for mw size.");
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < hw->mw_cnt; i++) {
 		bar = intel_ntb_bar[i];
 		hw->mw_size[i] = hw->pci_dev->mem_resource[bar].len;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index Min Hu (Connor)
@ 2021-04-21  3:31   ` Li, Xiaoyun
  2021-04-21  3:54     ` fengchengwen
  2021-04-21  4:36     ` Min Hu (Connor)
  2021-04-22  5:05   ` Li, Xiaoyun
  1 sibling, 2 replies; 12+ messages in thread
From: Li, Xiaoyun @ 2021-04-21  3:31 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: Yigit, Ferruh, Wu, Jingjing

Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Wednesday, April 21, 2021 10:08
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH 1/2] raw/ntb: check spad user index
> 
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> This patch adds checking spad user index validity when set or get attr.
> 
> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/raw/ntb/ntb.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
> 6dd213e..0f0e3f2 100644
> --- a/drivers/raw/ntb/ntb.c
> +++ b/drivers/raw/ntb/ntb.c
> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const char
> *attr_name,
>  		if (hw->ntb_ops->spad_write == NULL)
>  			return -ENOTSUP;
>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
> +			return -EINVAL;
> +		}

It's unnecessary. The value will be checked in intel_ntb_spad_write(). There will be error remind in that.

>  		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
>  					   1, attr_value);
>  		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
> *attr_name,
>  		if (hw->ntb_ops->spad_read == NULL)
>  			return -ENOTSUP;
>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> +			NTB_LOG(ERR, "Attribute (%s) out of range",
> attr_name);
> +			return -EINVAL;
> +		}

Same as above.

>  		*attr_value = (*hw->ntb_ops->spad_read)(dev,
>  				hw->spad_user_list[index], 0);
>  		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result Min Hu (Connor)
@ 2021-04-21  3:33   ` Li, Xiaoyun
  0 siblings, 0 replies; 12+ messages in thread
From: Li, Xiaoyun @ 2021-04-21  3:33 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: Yigit, Ferruh, Wu, Jingjing

Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Wednesday, April 21, 2021 10:08
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH 2/2] raw/ntb: check malloc result
> 
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> This patch adds checking for rte_zmalloc() result when init intel ntb device, also
> fix the same bug when start ntb device.
> 
> Fixes: 034c328eb025 ("raw/ntb: support Intel NTB")
> Fixes: c39d1e082a4b ("raw/ntb: setup queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/raw/ntb/ntb.c          | 5 +++++
>  drivers/raw/ntb/ntb_hw_intel.c | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
> 0f0e3f2..6703bb5 100644
> --- a/drivers/raw/ntb/ntb.c
> +++ b/drivers/raw/ntb/ntb.c
> @@ -923,6 +923,11 @@ ntb_dev_start(struct rte_rawdev *dev)
> 
>  	hw->peer_mw_base = rte_zmalloc("ntb_peer_mw_base", hw->mw_cnt
> *
>  					sizeof(uint64_t), 0);
> +	if (hw->peer_mw_base == NULL) {
> +		NTB_LOG(ERR, "Cannot allocate memory for peer mw base.");
> +		ret = -ENOMEM;
> +		goto err_q_init;
> +	}
> 
>  	if (hw->ntb_ops->spad_read == NULL) {
>  		ret = -ENOTSUP;
> diff --git a/drivers/raw/ntb/ntb_hw_intel.c b/drivers/raw/ntb/ntb_hw_intel.c
> index 4427e11..a742e8f 100644
> --- a/drivers/raw/ntb/ntb_hw_intel.c
> +++ b/drivers/raw/ntb/ntb_hw_intel.c
> @@ -148,6 +148,11 @@ intel_ntb_dev_init(const struct rte_rawdev *dev)
> 
>  	hw->mw_size = rte_zmalloc("ntb_mw_size",
>  				  hw->mw_cnt * sizeof(uint64_t), 0);
> +	if (hw->mw_size == NULL) {
> +		NTB_LOG(ERR, "Cannot allocate memory for mw size.");
> +		return -ENOMEM;
> +	}
> +
>  	for (i = 0; i < hw->mw_cnt; i++) {
>  		bar = intel_ntb_bar[i];
>  		hw->mw_size[i] = hw->pci_dev->mem_resource[bar].len;
> --
> 2.7.4

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  3:31   ` Li, Xiaoyun
@ 2021-04-21  3:54     ` fengchengwen
  2021-04-21  4:36     ` Min Hu (Connor)
  1 sibling, 0 replies; 12+ messages in thread
From: fengchengwen @ 2021-04-21  3:54 UTC (permalink / raw)
  To: Li, Xiaoyun, Min Hu (Connor), dev; +Cc: Yigit, Ferruh, Wu, Jingjing



On 2021/4/21 11:31, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Wednesday, April 21, 2021 10:08
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
>> Wu, Jingjing <jingjing.wu@intel.com>
>> Subject: [PATCH 1/2] raw/ntb: check spad user index
>>
>> From: Chengwen Feng <fengchengwen@huawei.com>
>>
>> This patch adds checking spad user index validity when set or get attr.
>>
>> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>  drivers/raw/ntb/ntb.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
>> 6dd213e..0f0e3f2 100644
>> --- a/drivers/raw/ntb/ntb.c
>> +++ b/drivers/raw/ntb/ntb.c
>> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const char
>> *attr_name,
>>  		if (hw->ntb_ops->spad_write == NULL)
>>  			return -ENOTSUP;
>>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
>> +			return -EINVAL;
>> +		}
> 
> It's unnecessary. The value will be checked in intel_ntb_spad_write(). There will be error remind in that.

index maybe large, and then hw->spad_user_list[index] may lead to segmentation fault.
so the verification is required.

> 
>>  		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
>>  					   1, attr_value);
>>  		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
>> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
>> *attr_name,
>>  		if (hw->ntb_ops->spad_read == NULL)
>>  			return -ENOTSUP;
>>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>> +			NTB_LOG(ERR, "Attribute (%s) out of range",
>> attr_name);
>> +			return -EINVAL;
>> +		}
> 
> Same as above.
> 
>>  		*attr_value = (*hw->ntb_ops->spad_read)(dev,
>>  				hw->spad_user_list[index], 0);
>>  		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
>> --
>> 2.7.4
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  3:31   ` Li, Xiaoyun
  2021-04-21  3:54     ` fengchengwen
@ 2021-04-21  4:36     ` Min Hu (Connor)
  2021-04-21  4:54       ` Li, Xiaoyun
  1 sibling, 1 reply; 12+ messages in thread
From: Min Hu (Connor) @ 2021-04-21  4:36 UTC (permalink / raw)
  To: Li, Xiaoyun, dev; +Cc: Yigit, Ferruh, Wu, Jingjing

Hi, xiaoyun,

在 2021/4/21 11:31, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Wednesday, April 21, 2021 10:08
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
>> Wu, Jingjing <jingjing.wu@intel.com>
>> Subject: [PATCH 1/2] raw/ntb: check spad user index
>>
>> From: Chengwen Feng <fengchengwen@huawei.com>
>>
>> This patch adds checking spad user index validity when set or get attr.
>>
>> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   drivers/raw/ntb/ntb.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
>> 6dd213e..0f0e3f2 100644
>> --- a/drivers/raw/ntb/ntb.c
>> +++ b/drivers/raw/ntb/ntb.c
>> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const char
>> *attr_name,
>>   		if (hw->ntb_ops->spad_write == NULL)
>>   			return -ENOTSUP;
>>   		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
>> +			return -EINVAL;
>> +		}
> 
> It's unnecessary. The value will be checked in intel_ntb_spad_write(). There will be error remind in that.
>
Nothing to do with intel_ntb_spad_write. If index is no checked,
hw->spad_user_list[index] may be be out of memory and result in
segmentation default.

	
>>   		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
>>   					   1, attr_value);
>>   		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
>> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
>> *attr_name,
>>   		if (hw->ntb_ops->spad_read == NULL)
>>   			return -ENOTSUP;
>>   		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>> +			NTB_LOG(ERR, "Attribute (%s) out of range",
>> attr_name);
>> +			return -EINVAL;
>> +		}
> 
> Same as above.
> 
>>   		*attr_value = (*hw->ntb_ops->spad_read)(dev,
>>   				hw->spad_user_list[index], 0);
>>   		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
>> --
>> 2.7.4
> 
> .
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  4:36     ` Min Hu (Connor)
@ 2021-04-21  4:54       ` Li, Xiaoyun
  2021-04-21  6:08         ` Min Hu (Connor)
  0 siblings, 1 reply; 12+ messages in thread
From: Li, Xiaoyun @ 2021-04-21  4:54 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: Yigit, Ferruh, Wu, Jingjing



> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Wednesday, April 21, 2021 12:37
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: Re: [PATCH 1/2] raw/ntb: check spad user index
> 
> Hi, xiaoyun,
> 
> 在 2021/4/21 11:31, Li, Xiaoyun 写道:
> > Hi
> >
> >> -----Original Message-----
> >> From: Min Hu (Connor) <humin29@huawei.com>
> >> Sent: Wednesday, April 21, 2021 10:08
> >> To: dev@dpdk.org
> >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun
> >> <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> >> Subject: [PATCH 1/2] raw/ntb: check spad user index
> >>
> >> From: Chengwen Feng <fengchengwen@huawei.com>
> >>
> >> This patch adds checking spad user index validity when set or get attr.
> >>
> >> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> ---
> >>   drivers/raw/ntb/ntb.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
> >> 6dd213e..0f0e3f2 100644
> >> --- a/drivers/raw/ntb/ntb.c
> >> +++ b/drivers/raw/ntb/ntb.c
> >> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const
> >> char *attr_name,
> >>   		if (hw->ntb_ops->spad_write == NULL)
> >>   			return -ENOTSUP;
> >>   		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> >> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> >> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
> >> +			return -EINVAL;
> >> +		}
> >
> > It's unnecessary. The value will be checked in intel_ntb_spad_write(). There
> will be error remind in that.
> >
> Nothing to do with intel_ntb_spad_write. If index is no checked,
> hw->spad_user_list[index] may be be out of memory and result in
> segmentation default.

Are you using this driver externally? Or you just check everything in DPDK.
This is actually only used for ntb example in file trans mode. And only 0 and 1 are used for index.

> 
> 
> >>   		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
> >>   					   1, attr_value);
> >>   		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
> >> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
> >> *attr_name,
> >>   		if (hw->ntb_ops->spad_read == NULL)
> >>   			return -ENOTSUP;
> >>   		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> >> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> >> +			NTB_LOG(ERR, "Attribute (%s) out of range",
> >> attr_name);
> >> +			return -EINVAL;
> >> +		}
> >
> > Same as above.
> >
> >>   		*attr_value = (*hw->ntb_ops->spad_read)(dev,
> >>   				hw->spad_user_list[index], 0);
> >>   		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
> >> --
> >> 2.7.4
> >
> > .
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  4:54       ` Li, Xiaoyun
@ 2021-04-21  6:08         ` Min Hu (Connor)
  2021-05-05 20:37           ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Min Hu (Connor) @ 2021-04-21  6:08 UTC (permalink / raw)
  To: Li, Xiaoyun, dev; +Cc: Yigit, Ferruh, Wu, Jingjing



在 2021/4/21 12:54, Li, Xiaoyun 写道:
> 
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Wednesday, April 21, 2021 12:37
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
>> Subject: Re: [PATCH 1/2] raw/ntb: check spad user index
>>
>> Hi, xiaoyun,
>>
>> 在 2021/4/21 11:31, Li, Xiaoyun 写道:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Min Hu (Connor) <humin29@huawei.com>
>>>> Sent: Wednesday, April 21, 2021 10:08
>>>> To: dev@dpdk.org
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun
>>>> <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
>>>> Subject: [PATCH 1/2] raw/ntb: check spad user index
>>>>
>>>> From: Chengwen Feng <fengchengwen@huawei.com>
>>>>
>>>> This patch adds checking spad user index validity when set or get attr.
>>>>
>>>> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> ---
>>>>    drivers/raw/ntb/ntb.c | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
>>>> 6dd213e..0f0e3f2 100644
>>>> --- a/drivers/raw/ntb/ntb.c
>>>> +++ b/drivers/raw/ntb/ntb.c
>>>> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const
>>>> char *attr_name,
>>>>    		if (hw->ntb_ops->spad_write == NULL)
>>>>    			return -ENOTSUP;
>>>>    		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>>>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>>>> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
>>>> +			return -EINVAL;
>>>> +		}
>>>
>>> It's unnecessary. The value will be checked in intel_ntb_spad_write(). There
>> will be error remind in that.
>>>
>> Nothing to do with intel_ntb_spad_write. If index is no checked,
>> hw->spad_user_list[index] may be be out of memory and result in
>> segmentation default.
> 
> Are you using this driver externally? Or you just check everything in DPDK.
> This is actually only used for ntb example in file trans mode. And only 0 and 1 are used for index.
> 
Well, I just reviewed codes and found this bug.
>>
>>
>>>>    		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
>>>>    					   1, attr_value);
>>>>    		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
>>>> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
>>>> *attr_name,
>>>>    		if (hw->ntb_ops->spad_read == NULL)
>>>>    			return -ENOTSUP;
>>>>    		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
>>>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
>>>> +			NTB_LOG(ERR, "Attribute (%s) out of range",
>>>> attr_name);
>>>> +			return -EINVAL;
>>>> +		}
>>>
>>> Same as above.
>>>
>>>>    		*attr_value = (*hw->ntb_ops->spad_read)(dev,
>>>>    				hw->spad_user_list[index], 0);
>>>>    		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
>>>> --
>>>> 2.7.4
>>>
>>> .
>>>
> .
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  2:08 ` [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index Min Hu (Connor)
  2021-04-21  3:31   ` Li, Xiaoyun
@ 2021-04-22  5:05   ` Li, Xiaoyun
  2021-05-05 21:04     ` Thomas Monjalon
  1 sibling, 1 reply; 12+ messages in thread
From: Li, Xiaoyun @ 2021-04-22  5:05 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: Yigit, Ferruh, Wu, Jingjing

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Wednesday, April 21, 2021 10:08
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH 1/2] raw/ntb: check spad user index
> 
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> This patch adds checking spad user index validity when set or get attr.
> 
> Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/raw/ntb/ntb.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
> 6dd213e..0f0e3f2 100644
> --- a/drivers/raw/ntb/ntb.c
> +++ b/drivers/raw/ntb/ntb.c
> @@ -1080,6 +1080,10 @@ ntb_attr_set(struct rte_rawdev *dev, const char
> *attr_name,
>  		if (hw->ntb_ops->spad_write == NULL)
>  			return -ENOTSUP;
>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
> +			return -EINVAL;
> +		}
>  		(*hw->ntb_ops->spad_write)(dev, hw->spad_user_list[index],
>  					   1, attr_value);
>  		NTB_LOG(DEBUG, "Set attribute (%s) Value (%" PRIu64 ")", @@
> -1174,6 +1178,10 @@ ntb_attr_get(struct rte_rawdev *dev, const char
> *attr_name,
>  		if (hw->ntb_ops->spad_read == NULL)
>  			return -ENOTSUP;
>  		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> +			NTB_LOG(ERR, "Attribute (%s) out of range",
> attr_name);
> +			return -EINVAL;
> +		}
>  		*attr_value = (*hw->ntb_ops->spad_read)(dev,
>  				hw->spad_user_list[index], 0);
>  		NTB_LOG(DEBUG, "Attribute (%s) Value (%" PRIu64 ")",
> --
> 2.7.4

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-21  6:08         ` Min Hu (Connor)
@ 2021-05-05 20:37           ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2021-05-05 20:37 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: Li, Xiaoyun, dev, Yigit, Ferruh, Wu, Jingjing, david.marchand

21/04/2021 08:08, Min Hu (Connor):
> 在 2021/4/21 12:54, Li, Xiaoyun 写道:
> > From: Min Hu (Connor) <humin29@huawei.com>
> >> 在 2021/4/21 11:31, Li, Xiaoyun 写道:
> >>> From: Min Hu (Connor) <humin29@huawei.com>
> >>>> From: Chengwen Feng <fengchengwen@huawei.com>
> >>>> --- a/drivers/raw/ntb/ntb.c
> >>>> +++ b/drivers/raw/ntb/ntb.c
> >>>>    		index = atoi(&attr_name[NTB_SPAD_USER_LEN]);
> >>>> +		if (index < 0 || index >= NTB_SPAD_USER_MAX_NUM) {
> >>>> +			NTB_LOG(ERR, "Invalid attribute (%s)", attr_name);
> >>>> +			return -EINVAL;
> >>>> +		}
> >>>
> >>> It's unnecessary. The value will be checked in intel_ntb_spad_write().
> >>> There will be error remind in that.
> >>>
> >> Nothing to do with intel_ntb_spad_write. If index is no checked,
> >> hw->spad_user_list[index] may be be out of memory and result in
> >> segmentation default.
> > 
> > Are you using this driver externally? Or you just check everything in DPDK.
> > This is actually only used for ntb example in file trans mode. And only 0 and 1 are used for index.
> 
> Well, I just reviewed codes and found this bug.

Are you using some tools to detect bugs?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index
  2021-04-22  5:05   ` Li, Xiaoyun
@ 2021-05-05 21:04     ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2021-05-05 21:04 UTC (permalink / raw)
  To: Min Hu (Connor); +Cc: dev, Yigit, Ferruh, Wu, Jingjing, Li, Xiaoyun

22/04/2021 07:05, Li, Xiaoyun:
> > From: Chengwen Feng <fengchengwen@huawei.com>
> > 
> > This patch adds checking spad user index validity when set or get attr.
> > 
> > Fixes: 277310027965 ("raw/ntb: introduce NTB raw device driver")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Series applied, thanks.




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-05-05 21:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21  2:08 [dpdk-dev] [PATCH 0/2] bugfix for raw ntb Min Hu (Connor)
2021-04-21  2:08 ` [dpdk-dev] [PATCH 1/2] raw/ntb: check spad user index Min Hu (Connor)
2021-04-21  3:31   ` Li, Xiaoyun
2021-04-21  3:54     ` fengchengwen
2021-04-21  4:36     ` Min Hu (Connor)
2021-04-21  4:54       ` Li, Xiaoyun
2021-04-21  6:08         ` Min Hu (Connor)
2021-05-05 20:37           ` Thomas Monjalon
2021-04-22  5:05   ` Li, Xiaoyun
2021-05-05 21:04     ` Thomas Monjalon
2021-04-21  2:08 ` [dpdk-dev] [PATCH 2/2] raw/ntb: check malloc result Min Hu (Connor)
2021-04-21  3:33   ` Li, Xiaoyun

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).