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