From: Yunjian Wang <wangyunjian@huawei.com> This patch fixes (dereference after null check) coverity issue. The intr_handle may be a null pointer which led to this issue. Coverity issue: 357695, 357751 Fixes: 05c4105738d8 ("trace: add interrupt tracepoints") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- lib/librte_eal/freebsd/eal_interrupts.c | 6 ++++-- lib/librte_eal/linux/eal_interrupts.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/librte_eal/freebsd/eal_interrupts.c b/lib/librte_eal/freebsd/eal_interrupts.c index 6d53d33c8..028ab457a 100644 --- a/lib/librte_eal/freebsd/eal_interrupts.c +++ b/lib/librte_eal/freebsd/eal_interrupts.c @@ -380,7 +380,8 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) } out: - rte_eal_trace_intr_enable(intr_handle, rc); + if (intr_handle) + rte_eal_trace_intr_enable(intr_handle, rc); return rc; } @@ -418,7 +419,8 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) break; } out: - rte_eal_trace_intr_disable(intr_handle, rc); + if (intr_handle) + rte_eal_trace_intr_disable(intr_handle, rc); return rc; } diff --git a/lib/librte_eal/linux/eal_interrupts.c b/lib/librte_eal/linux/eal_interrupts.c index 13db5c4e8..e46443873 100644 --- a/lib/librte_eal/linux/eal_interrupts.c +++ b/lib/librte_eal/linux/eal_interrupts.c @@ -725,7 +725,8 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) break; } out: - rte_eal_trace_intr_enable(intr_handle, rc); + if (intr_handle) + rte_eal_trace_intr_enable(intr_handle, rc); return rc; } @@ -852,7 +853,8 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) break; } out: - rte_eal_trace_intr_disable(intr_handle, rc); + if (intr_handle) + rte_eal_trace_intr_disable(intr_handle, rc); return rc; } -- 2.23.0
On 9/19/2020 11:34 AM, wangyunjian wrote: > From: Yunjian Wang <wangyunjian@huawei.com> > > This patch fixes (dereference after null check) coverity issue. > The intr_handle may be a null pointer which led to this issue. > > Coverity issue: 357695, 357751 > Fixes: 05c4105738d8 ("trace: add interrupt tracepoints") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > lib/librte_eal/freebsd/eal_interrupts.c | 6 ++++-- > lib/librte_eal/linux/eal_interrupts.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_eal/freebsd/eal_interrupts.c b/lib/librte_eal/freebsd/eal_interrupts.c > index 6d53d33c8..028ab457a 100644 > --- a/lib/librte_eal/freebsd/eal_interrupts.c > +++ b/lib/librte_eal/freebsd/eal_interrupts.c > @@ -380,7 +380,8 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) > } > > out: > - rte_eal_trace_intr_enable(intr_handle, rc); > + if (intr_handle) > + rte_eal_trace_intr_enable(intr_handle, rc); > return rc; > } > > @@ -418,7 +419,8 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) > break; > } > out: > - rte_eal_trace_intr_disable(intr_handle, rc); > + if (intr_handle) > + rte_eal_trace_intr_disable(intr_handle, rc); > return rc; > } > > diff --git a/lib/librte_eal/linux/eal_interrupts.c b/lib/librte_eal/linux/eal_interrupts.c > index 13db5c4e8..e46443873 100644 > --- a/lib/librte_eal/linux/eal_interrupts.c > +++ b/lib/librte_eal/linux/eal_interrupts.c > @@ -725,7 +725,8 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) > break; > } > out: > - rte_eal_trace_intr_enable(intr_handle, rc); > + if (intr_handle) > + rte_eal_trace_intr_enable(intr_handle, rc); > return rc; It looks like whole function requires 'intr_handle' to be not NULL, so what do you think add following at the very beginning of the function and remove other 'intr_handle' NULL checks from function: if (intr_handle == NULL) return -1; > } > > @@ -852,7 +853,8 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) > break; > } > out: > - rte_eal_trace_intr_disable(intr_handle, rc); > + if (intr_handle) > + rte_eal_trace_intr_disable(intr_handle, rc); > return rc; > } > >
> -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] > Sent: Thursday, October 15, 2020 1:03 AM > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org > Cc: david.marchand@redhat.com; jerinj@marvell.com; hkalra@marvell.com; > Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>; > stable@dpdk.org > Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] eal: fix dereference before null > check > > On 9/19/2020 11:34 AM, wangyunjian wrote: > > From: Yunjian Wang <wangyunjian@huawei.com> > > > > This patch fixes (dereference after null check) coverity issue. > > The intr_handle may be a null pointer which led to this issue. > > > > Coverity issue: 357695, 357751 > > Fixes: 05c4105738d8 ("trace: add interrupt tracepoints") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > lib/librte_eal/freebsd/eal_interrupts.c | 6 ++++-- > > lib/librte_eal/linux/eal_interrupts.c | 6 ++++-- > > 2 files changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_eal/freebsd/eal_interrupts.c > > b/lib/librte_eal/freebsd/eal_interrupts.c > > index 6d53d33c8..028ab457a 100644 > > --- a/lib/librte_eal/freebsd/eal_interrupts.c > > +++ b/lib/librte_eal/freebsd/eal_interrupts.c > > @@ -380,7 +380,8 @@ rte_intr_enable(const struct rte_intr_handle > *intr_handle) > > } > > > > out: > > - rte_eal_trace_intr_enable(intr_handle, rc); > > + if (intr_handle) > > + rte_eal_trace_intr_enable(intr_handle, rc); > > return rc; > > } > > > > @@ -418,7 +419,8 @@ rte_intr_disable(const struct rte_intr_handle > *intr_handle) > > break; > > } > > out: > > - rte_eal_trace_intr_disable(intr_handle, rc); > > + if (intr_handle) > > + rte_eal_trace_intr_disable(intr_handle, rc); > > return rc; > > } > > > > diff --git a/lib/librte_eal/linux/eal_interrupts.c > > b/lib/librte_eal/linux/eal_interrupts.c > > index 13db5c4e8..e46443873 100644 > > --- a/lib/librte_eal/linux/eal_interrupts.c > > +++ b/lib/librte_eal/linux/eal_interrupts.c > > @@ -725,7 +725,8 @@ rte_intr_enable(const struct rte_intr_handle > *intr_handle) > > break; > > } > > out: > > - rte_eal_trace_intr_enable(intr_handle, rc); > > + if (intr_handle) > > + rte_eal_trace_intr_enable(intr_handle, rc); > > return rc; > > It looks like whole function requires 'intr_handle' to be not NULL, so what do > you think add following at the very beginning of the function and remove other > 'intr_handle' NULL checks from function: > > if (intr_handle == NULL) > return -1; Agree, I will add them in next version. Thanks, Yunjian > > > } > > > > @@ -852,7 +853,8 @@ rte_intr_disable(const struct rte_intr_handle > *intr_handle) > > break; > > } > > out: > > - rte_eal_trace_intr_disable(intr_handle, rc); > > + if (intr_handle) > > + rte_eal_trace_intr_disable(intr_handle, rc); > > return rc; > > } > > > >
From: Yunjian Wang <wangyunjian@huawei.com> This patch fixes (dereference after null check) coverity issue. For this reason, we should add null check at the beginning of the function and return error directly if the 'intr_handle' is null. Coverity issue: 357695, 357751 Fixes: 05c4105738d8 ("trace: add interrupt tracepoints") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- v2: fix code styles suggested by Ferruh Yigit --- lib/librte_eal/freebsd/eal_interrupts.c | 16 ++++++++++------ lib/librte_eal/linux/eal_interrupts.c | 16 ++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/librte_eal/freebsd/eal_interrupts.c b/lib/librte_eal/freebsd/eal_interrupts.c index 6d53d33c8..211fd4f8d 100644 --- a/lib/librte_eal/freebsd/eal_interrupts.c +++ b/lib/librte_eal/freebsd/eal_interrupts.c @@ -350,13 +350,15 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) { int rc = 0; - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { + if (intr_handle == NULL) + return -1; + + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { rc = 0; goto out; } - if (!intr_handle || intr_handle->fd < 0 || - intr_handle->uio_cfg_fd < 0) { + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { rc = -1; goto out; } @@ -389,13 +391,15 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) { int rc = 0; - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { + if (intr_handle == NULL) + return -1; + + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { rc = 0; goto out; } - if (!intr_handle || intr_handle->fd < 0 || - intr_handle->uio_cfg_fd < 0) { + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { rc = -1; goto out; } diff --git a/lib/librte_eal/linux/eal_interrupts.c b/lib/librte_eal/linux/eal_interrupts.c index 13db5c4e8..f1bd0356c 100644 --- a/lib/librte_eal/linux/eal_interrupts.c +++ b/lib/librte_eal/linux/eal_interrupts.c @@ -667,13 +667,15 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) { int rc = 0; - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { + if (intr_handle == NULL) + return -1; + + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { rc = 0; goto out; } - if (!intr_handle || intr_handle->fd < 0 || - intr_handle->uio_cfg_fd < 0) { + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { rc = -1; goto out; } @@ -794,13 +796,15 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) { int rc = 0; - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { + if (intr_handle == NULL) + return -1; + + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { rc = 0; goto out; } - if (!intr_handle || intr_handle->fd < 0 || - intr_handle->uio_cfg_fd < 0) { + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { rc = -1; goto out; } -- 2.23.0
On Thu, Oct 15, 2020 at 10:43 AM wangyunjian <wangyunjian@huawei.com> wrote:
>
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> This patch fixes (dereference after null check) coverity issue.
> For this reason, we should add null check at the beginning of the
> function and return error directly if the 'intr_handle' is null.
>
> Coverity issue: 357695, 357751
> Fixes: 05c4105738d8 ("trace: add interrupt tracepoints")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
Review, please.
--
David Marchand
On Thu, Oct 15, 2020 at 04:42:30PM +0800, wangyunjian wrote: > External Email > > ---------------------------------------------------------------------- > From: Yunjian Wang <wangyunjian@huawei.com> > > This patch fixes (dereference after null check) coverity issue. > For this reason, we should add null check at the beginning of the > function and return error directly if the 'intr_handle' is null. > > Coverity issue: 357695, 357751 > Fixes: 05c4105738d8 ("trace: add interrupt tracepoints") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Thanks for fixing this. Reviewed-by: Harman Kalra <hkalra@marvell.com> > --- > v2: > fix code styles suggested by Ferruh Yigit > --- > lib/librte_eal/freebsd/eal_interrupts.c | 16 ++++++++++------ > lib/librte_eal/linux/eal_interrupts.c | 16 ++++++++++------ > 2 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/lib/librte_eal/freebsd/eal_interrupts.c b/lib/librte_eal/freebsd/eal_interrupts.c > index 6d53d33c8..211fd4f8d 100644 > --- a/lib/librte_eal/freebsd/eal_interrupts.c > +++ b/lib/librte_eal/freebsd/eal_interrupts.c > @@ -350,13 +350,15 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) > { > int rc = 0; > > - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { > + if (intr_handle == NULL) > + return -1; > + > + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { > rc = 0; > goto out; > } > > - if (!intr_handle || intr_handle->fd < 0 || > - intr_handle->uio_cfg_fd < 0) { > + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { > rc = -1; > goto out; > } > @@ -389,13 +391,15 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) > { > int rc = 0; > > - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { > + if (intr_handle == NULL) > + return -1; > + > + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { > rc = 0; > goto out; > } > > - if (!intr_handle || intr_handle->fd < 0 || > - intr_handle->uio_cfg_fd < 0) { > + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { > rc = -1; > goto out; > } > diff --git a/lib/librte_eal/linux/eal_interrupts.c b/lib/librte_eal/linux/eal_interrupts.c > index 13db5c4e8..f1bd0356c 100644 > --- a/lib/librte_eal/linux/eal_interrupts.c > +++ b/lib/librte_eal/linux/eal_interrupts.c > @@ -667,13 +667,15 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle) > { > int rc = 0; > > - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { > + if (intr_handle == NULL) > + return -1; > + > + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { > rc = 0; > goto out; > } > > - if (!intr_handle || intr_handle->fd < 0 || > - intr_handle->uio_cfg_fd < 0) { > + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { > rc = -1; > goto out; > } > @@ -794,13 +796,15 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle) > { > int rc = 0; > > - if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV) { > + if (intr_handle == NULL) > + return -1; > + > + if (intr_handle->type == RTE_INTR_HANDLE_VDEV) { > rc = 0; > goto out; > } > > - if (!intr_handle || intr_handle->fd < 0 || > - intr_handle->uio_cfg_fd < 0) { > + if (intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0) { > rc = -1; > goto out; > } > -- > 2.23.0 >
On Wed, Oct 28, 2020 at 10:19 PM Harman Kalra <hkalra@marvell.com> wrote:
> On Thu, Oct 15, 2020 at 04:42:30PM +0800, wangyunjian wrote:
> > This patch fixes (dereference after null check) coverity issue.
> > For this reason, we should add null check at the beginning of the
> > function and return error directly if the 'intr_handle' is null.
> >
> > Coverity issue: 357695, 357751
> > Fixes: 05c4105738d8 ("trace: add interrupt tracepoints")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> Reviewed-by: Harman Kalra <hkalra@marvell.com>
Applied, thanks.
--
David Marchand