From: Ye Xiaolong <xiaolong.ye@intel.com>
To: "Zhang, Tianfei" <tianfei.zhang@intel.com>
Cc: dev@dpdk.org, rosen.xu@intel.com, stable@dpdk.org, Zhang@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1 1/2] raw/ifpga: fix use of untrusted scalar value
Date: Tue, 4 Jun 2019 15:29:56 +0800 [thread overview]
Message-ID: <20190604072956.GC94383@intel.com> (raw)
In-Reply-To: <20190604135148.21791-1-tianfei.zhang@intel.com>
On 06/04, Zhang, Tianfei wrote:
>Add checking the buffer size and use
>const char * for buffer declaration.
>
>Coverity issue: 279449
>Cc: stable@dpdk.org
Should also add a Fixes line.
Thanks,
Xiaolong
>
>Signed-off-by: Zhang, Tianfei <tianfei.zhang@intel.com>
>---
> drivers/raw/ifpga_rawdev/base/ifpga_api.c | 4 +--
> drivers/raw/ifpga_rawdev/base/ifpga_api.h | 2 +-
> .../raw/ifpga_rawdev/base/ifpga_feature_dev.h | 2 +-
> drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c | 27 +++++++++++--------
> drivers/raw/ifpga_rawdev/base/opae_hw_api.c | 4 +--
> drivers/raw/ifpga_rawdev/base/opae_hw_api.h | 4 +--
> drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 7 ++++-
> 7 files changed, 30 insertions(+), 20 deletions(-)
>
>diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_api.c b/drivers/raw/ifpga_rawdev/base/ifpga_api.c
>index 3ddbcdc2a..53d101daf 100644
>--- a/drivers/raw/ifpga_rawdev/base/ifpga_api.c
>+++ b/drivers/raw/ifpga_rawdev/base/ifpga_api.c
>@@ -182,7 +182,7 @@ struct opae_bridge_ops ifpga_br_ops = {
> };
>
> /* Manager APIs */
>-static int ifpga_mgr_flash(struct opae_manager *mgr, int id, void *buf,
>+static int ifpga_mgr_flash(struct opae_manager *mgr, int id, const char *buf,
> u32 size, u64 *status)
> {
> struct ifpga_fme_hw *fme = mgr->data;
>@@ -324,7 +324,7 @@ struct opae_adapter_ops ifpga_adapter_ops = {
> * - 0: Success, partial reconfiguration finished.
> * - <0: Error code returned in partial reconfiguration.
> **/
>-int ifpga_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
>+int ifpga_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, u32 size,
> u64 *status)
> {
> if (!is_valid_port_id(hw, port_id))
>diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_api.h b/drivers/raw/ifpga_rawdev/base/ifpga_api.h
>index 4a247698c..051ab8276 100644
>--- a/drivers/raw/ifpga_rawdev/base/ifpga_api.h
>+++ b/drivers/raw/ifpga_rawdev/base/ifpga_api.h
>@@ -23,7 +23,7 @@ int ifpga_set_irq(struct ifpga_hw *hw, u32 fiu_id, u32 port_id,
> u32 feature_id, void *irq_set);
>
> /* FME APIs */
>-int ifpga_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
>+int ifpga_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, u32 size,
> u64 *status);
>
> #endif /* _IFPGA_API_H_ */
>diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h
>index bb9fcc289..e243d4273 100644
>--- a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h
>+++ b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h
>@@ -149,7 +149,7 @@ static inline int fpga_port_reset(struct ifpga_port_hw *port)
> return ret;
> }
>
>-int do_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
>+int do_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, u32 size,
> u64 *status);
>
> int fme_get_prop(struct ifpga_fme_hw *fme, struct feature_prop *prop);
>diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c b/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c
>index efa72660f..9997942d2 100644
>--- a/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c
>+++ b/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c
>@@ -223,8 +223,8 @@ static int fpga_pr_buf_load(struct ifpga_fme_hw *fme_dev,
> return 0;
> }
>
>-static int fme_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
>- u64 *status)
>+static int fme_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer,
>+ u32 size, u64 *status)
> {
> struct feature_fme_header *fme_hdr;
> struct feature_fme_capability fme_capability;
>@@ -269,7 +269,7 @@ static int fme_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
> /* Disable Port before PR */
> fpga_port_disable(port);
>
>- ret = fpga_pr_buf_load(fme, &info, (void *)buffer, size);
>+ ret = fpga_pr_buf_load(fme, &info, buffer, size);
>
> *status = info.pr_err;
>
>@@ -280,27 +280,32 @@ static int fme_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size,
> return ret;
> }
>
>-int do_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size, u64 *status)
>+int do_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer,
>+ u32 size, u64 *status)
> {
>- struct bts_header *bts_hdr;
>- void *buf;
>+ const struct bts_header *bts_hdr;
>+ const char *buf;
> struct ifpga_port_hw *port;
> int ret;
>+ u32 header_size;
>
> if (!buffer || size == 0) {
> dev_err(hw, "invalid parameter\n");
> return -EINVAL;
> }
>
>- bts_hdr = (struct bts_header *)buffer;
>+ bts_hdr = (const struct bts_header *)buffer;
>
> if (is_valid_bts(bts_hdr)) {
> dev_info(hw, "this is a valid bitsteam..\n");
>- size -= (sizeof(struct bts_header) +
>- bts_hdr->metadata_len);
>- buf = (u8 *)buffer + sizeof(struct bts_header) +
>- bts_hdr->metadata_len;
>+ header_size = sizeof(struct bts_header) +
>+ bts_hdr->metadata_len;
>+ if (size < header_size)
>+ return -EINVAL;
>+ size -= header_size;
>+ buf = buffer + header_size;
> } else {
>+ dev_err(hw, "this is an invalid bitstream..\n");
> return -EINVAL;
> }
>
>diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c
>index 0e117d05e..8964e7984 100644
>--- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c
>+++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c
>@@ -244,8 +244,8 @@ opae_manager_alloc(const char *name, struct opae_manager_ops *ops,
> *
> * Return: 0 on success, otherwise error code.
> */
>-int opae_manager_flash(struct opae_manager *mgr, int id, void *buf, u32 size,
>- u64 *status)
>+int opae_manager_flash(struct opae_manager *mgr, int id, const char *buf,
>+ u32 size, u64 *status)
> {
> if (!mgr)
> return -EINVAL;
>diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h
>index 383e751cb..63405a471 100644
>--- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h
>+++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h
>@@ -44,7 +44,7 @@ struct opae_manager {
>
> /* FIXME: add more management ops, e.g power/thermal and etc */
> struct opae_manager_ops {
>- int (*flash)(struct opae_manager *mgr, int id, void *buffer,
>+ int (*flash)(struct opae_manager *mgr, int id, const char *buffer,
> u32 size, u64 *status);
> int (*get_eth_group_region_info)(struct opae_manager *mgr,
> struct opae_eth_group_region_info *info);
>@@ -74,7 +74,7 @@ struct opae_manager *
> opae_manager_alloc(const char *name, struct opae_manager_ops *ops,
> struct opae_manager_networking_ops *network_ops, void *data);
> #define opae_manager_free(mgr) opae_free(mgr)
>-int opae_manager_flash(struct opae_manager *mgr, int acc_id, void *buf,
>+int opae_manager_flash(struct opae_manager *mgr, int acc_id, const char *buf,
> u32 size, u64 *status);
> int opae_manager_get_eth_group_region_info(struct opae_manager *mgr,
> u8 group_id, struct opae_eth_group_region_info *info);
>diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>index 41be1a205..01aa917de 100644
>--- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>+++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>@@ -225,7 +225,7 @@ ifpga_rawdev_reset(struct rte_rawdev *dev)
> }
>
> static int
>-fpga_pr(struct rte_rawdev *raw_dev, u32 port_id, u64 *buffer, u32 size,
>+fpga_pr(struct rte_rawdev *raw_dev, u32 port_id, const char *buffer, u32 size,
> u64 *status)
> {
>
>@@ -296,6 +296,11 @@ rte_fpga_do_pr(struct rte_rawdev *rawdev, int port_id,
> goto close_fd;
> }
> buffer_size = file_stat.st_size;
>+ if (buffer_size <= 0) {
>+ ret = -EINVAL;
>+ goto close_fd;
>+ }
>+
> IFPGA_RAWDEV_PMD_INFO("bitstream file size: %zu\n", buffer_size);
> buffer = rte_malloc(NULL, buffer_size, 0);
> if (!buffer) {
>--
>2.17.1
>
next prev parent reply other threads:[~2019-06-04 7:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-04 13:51 [dpdk-dev] [PATCH v1 1/2] raw/ifpga: fix use of untrusted scalar value Zhang, Tianfei
2019-06-04 7:29 ` Ye Xiaolong [this message]
2019-06-04 8:03 ` Zhang, Tianfei
2019-06-04 13:51 ` [dpdk-dev] [PATCH v1 2/2] raw/ifpga: fix logically dead code Zhang, Tianfei
2019-06-04 7:26 ` Ye Xiaolong
2019-06-04 9:06 ` [dpdk-dev] [dpdk-stable] " Mcnamara, John
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190604072956.GC94383@intel.com \
--to=xiaolong.ye@intel.com \
--cc=Zhang@dpdk.org \
--cc=dev@dpdk.org \
--cc=rosen.xu@intel.com \
--cc=stable@dpdk.org \
--cc=tianfei.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.