* [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw
2019-03-27 9:33 [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 9:33 ` [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check Konstantin Ananyev
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
three simple fixes for ipsec-secgw, first two reported by Coverity.
Konstantin Ananyev (3):
examples/ipsec-secgw: fix invalid out-of-bound check
examples/ipsec_secgw: fix possible NULL dereference
examples/ipsec-secgw: fix typo in test script
examples/ipsec-secgw/ipsec-secgw.c | 2 +-
examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
examples/ipsec-secgw/test/run_test.sh | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check
2019-03-27 9:33 [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:52 ` Akhil Goyal
2019-03-27 9:33 ` [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference Konstantin Ananyev
` (2 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fixes: 7622291b641d ("examples/ipsec-secgw: allow to specify neighbour MAC address")
Coverity issue: 336791
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/ipsec-secgw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index b253eea2b..ffbd00b08 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1439,7 +1439,7 @@ print_ethaddr(const char *name, const struct ether_addr *eth_addr)
int
add_dst_ethaddr(uint16_t port, const struct ether_addr *addr)
{
- if (port > RTE_DIM(ethaddr_tbl))
+ if (port >= RTE_DIM(ethaddr_tbl))
return -EINVAL;
ethaddr_tbl[port].dst = ETHADDR_TO_UINT64(addr);
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check
2019-03-27 9:33 ` [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:52 ` Akhil Goyal
1 sibling, 0 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fixes: 7622291b641d ("examples/ipsec-secgw: allow to specify neighbour MAC address")
Coverity issue: 336791
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/ipsec-secgw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index b253eea2b..ffbd00b08 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1439,7 +1439,7 @@ print_ethaddr(const char *name, const struct ether_addr *eth_addr)
int
add_dst_ethaddr(uint16_t port, const struct ether_addr *addr)
{
- if (port > RTE_DIM(ethaddr_tbl))
+ if (port >= RTE_DIM(ethaddr_tbl))
return -EINVAL;
ethaddr_tbl[port].dst = ETHADDR_TO_UINT64(addr);
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check
2019-03-27 9:33 ` [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
@ 2019-03-27 12:52 ` Akhil Goyal
2019-03-27 12:52 ` Akhil Goyal
1 sibling, 1 reply; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:52 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fixes: 7622291b641d ("examples/ipsec-secgw: allow to specify neighbour MAC address")
> Coverity issue: 336791
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/ipsec-secgw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index b253eea2b..ffbd00b08 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1439,7 +1439,7 @@ print_ethaddr(const char *name, const struct ether_addr *eth_addr)
> int
> add_dst_ethaddr(uint16_t port, const struct ether_addr *addr)
> {
> - if (port > RTE_DIM(ethaddr_tbl))
> + if (port >= RTE_DIM(ethaddr_tbl))
> return -EINVAL;
>
> ethaddr_tbl[port].dst = ETHADDR_TO_UINT64(addr);
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check
2019-03-27 12:52 ` Akhil Goyal
@ 2019-03-27 12:52 ` Akhil Goyal
0 siblings, 0 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:52 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fixes: 7622291b641d ("examples/ipsec-secgw: allow to specify neighbour MAC address")
> Coverity issue: 336791
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/ipsec-secgw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index b253eea2b..ffbd00b08 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1439,7 +1439,7 @@ print_ethaddr(const char *name, const struct ether_addr *eth_addr)
> int
> add_dst_ethaddr(uint16_t port, const struct ether_addr *addr)
> {
> - if (port > RTE_DIM(ethaddr_tbl))
> + if (port >= RTE_DIM(ethaddr_tbl))
> return -EINVAL;
>
> ethaddr_tbl[port].dst = ETHADDR_TO_UINT64(addr);
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 9:33 [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 9:33 ` [dpdk-dev] [PATCH 1/3] examples/ipsec-secgw: fix invalid out-of-bound check Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:54 ` Akhil Goyal
2019-03-27 9:33 ` [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script Konstantin Ananyev
2019-03-29 14:21 ` [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Akhil Goyal
4 siblings, 2 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
Coverity issue: 336844
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index e403c461a..3f9cacb8f 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
pg = grp + i;
sa = pg->id.ptr;
- /* no valid SA found */
- if (sa == NULL)
- k = 0;
-
ips = &sa->ips;
- satp = rte_ipsec_sa_type(ips->sa);
/* no valid HW session for that SA, try to create one */
- if (ips->crypto.ses == NULL &&
- fill_ipsec_session(ips, ctx, sa) != 0)
+ if (sa == NULL || (ips->crypto.ses == NULL &&
+ fill_ipsec_session(ips, ctx, sa) != 0))
k = 0;
/* process packets inline */
@@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
sa->type ==
RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
+ satp = rte_ipsec_sa_type(ips->sa);
+
/*
* This is just to satisfy inbound_sa_check()
* and get_hop_for_offload_pkt().
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 9:33 ` [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:54 ` Akhil Goyal
1 sibling, 0 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
Coverity issue: 336844
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index e403c461a..3f9cacb8f 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
pg = grp + i;
sa = pg->id.ptr;
- /* no valid SA found */
- if (sa == NULL)
- k = 0;
-
ips = &sa->ips;
- satp = rte_ipsec_sa_type(ips->sa);
/* no valid HW session for that SA, try to create one */
- if (ips->crypto.ses == NULL &&
- fill_ipsec_session(ips, ctx, sa) != 0)
+ if (sa == NULL || (ips->crypto.ses == NULL &&
+ fill_ipsec_session(ips, ctx, sa) != 0))
k = 0;
/* process packets inline */
@@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
sa->type ==
RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
+ satp = rte_ipsec_sa_type(ips->sa);
+
/*
* This is just to satisfy inbound_sa_check()
* and get_hop_for_offload_pkt().
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 9:33 ` [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
@ 2019-03-27 12:54 ` Akhil Goyal
2019-03-27 12:54 ` Akhil Goyal
2019-03-27 13:19 ` Ananyev, Konstantin
1 sibling, 2 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:54 UTC (permalink / raw)
To: Konstantin Ananyev, dev
Hi Konstantin,
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> Coverity issue: 336844
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index e403c461a..3f9cacb8f 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> pg = grp + i;
> sa = pg->id.ptr;
>
> - /* no valid SA found */
> - if (sa == NULL)
> - k = 0;
> -
> ips = &sa->ips;
I think this is not fixing the NULL dereference properly. This line
would give fault if sa is null.
> - satp = rte_ipsec_sa_type(ips->sa);
>
> /* no valid HW session for that SA, try to create one */
> - if (ips->crypto.ses == NULL &&
> - fill_ipsec_session(ips, ctx, sa) != 0)
> + if (sa == NULL || (ips->crypto.ses == NULL &&
> + fill_ipsec_session(ips, ctx, sa) != 0))
> k = 0;
>
> /* process packets inline */
> @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> sa->type ==
> RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
>
> + satp = rte_ipsec_sa_type(ips->sa);
> +
> /*
> * This is just to satisfy inbound_sa_check()
> * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 12:54 ` Akhil Goyal
@ 2019-03-27 12:54 ` Akhil Goyal
2019-03-27 13:19 ` Ananyev, Konstantin
1 sibling, 0 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:54 UTC (permalink / raw)
To: Konstantin Ananyev, dev
Hi Konstantin,
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> Coverity issue: 336844
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index e403c461a..3f9cacb8f 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> pg = grp + i;
> sa = pg->id.ptr;
>
> - /* no valid SA found */
> - if (sa == NULL)
> - k = 0;
> -
> ips = &sa->ips;
I think this is not fixing the NULL dereference properly. This line
would give fault if sa is null.
> - satp = rte_ipsec_sa_type(ips->sa);
>
> /* no valid HW session for that SA, try to create one */
> - if (ips->crypto.ses == NULL &&
> - fill_ipsec_session(ips, ctx, sa) != 0)
> + if (sa == NULL || (ips->crypto.ses == NULL &&
> + fill_ipsec_session(ips, ctx, sa) != 0))
> k = 0;
>
> /* process packets inline */
> @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> sa->type ==
> RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
>
> + satp = rte_ipsec_sa_type(ips->sa);
> +
> /*
> * This is just to satisfy inbound_sa_check()
> * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 12:54 ` Akhil Goyal
2019-03-27 12:54 ` Akhil Goyal
@ 2019-03-27 13:19 ` Ananyev, Konstantin
2019-03-27 13:19 ` Ananyev, Konstantin
2019-03-27 14:37 ` Akhil Goyal
1 sibling, 2 replies; 20+ messages in thread
From: Ananyev, Konstantin @ 2019-03-27 13:19 UTC (permalink / raw)
To: Akhil Goyal, dev
Hi Akhil,
> > Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> > Coverity issue: 336844
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> > index e403c461a..3f9cacb8f 100644
> > --- a/examples/ipsec-secgw/ipsec_process.c
> > +++ b/examples/ipsec-secgw/ipsec_process.c
> > @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> > pg = grp + i;
> > sa = pg->id.ptr;
> >
> > - /* no valid SA found */
> > - if (sa == NULL)
> > - k = 0;
> > -
> > ips = &sa->ips;
> I think this is not fixing the NULL dereference properly. This line
> would give fault if sa is null.
I don't think it would - here we just get an address of ips,
we don't try to access it.
Konstantin
> > - satp = rte_ipsec_sa_type(ips->sa);
> >
> > /* no valid HW session for that SA, try to create one */
> > - if (ips->crypto.ses == NULL &&
> > - fill_ipsec_session(ips, ctx, sa) != 0)
> > + if (sa == NULL || (ips->crypto.ses == NULL &&
> > + fill_ipsec_session(ips, ctx, sa) != 0))
> > k = 0;
> >
> > /* process packets inline */
> > @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> > sa->type ==
> > RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
> >
> > + satp = rte_ipsec_sa_type(ips->sa);
> > +
> > /*
> > * This is just to satisfy inbound_sa_check()
> > * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 13:19 ` Ananyev, Konstantin
@ 2019-03-27 13:19 ` Ananyev, Konstantin
2019-03-27 14:37 ` Akhil Goyal
1 sibling, 0 replies; 20+ messages in thread
From: Ananyev, Konstantin @ 2019-03-27 13:19 UTC (permalink / raw)
To: Akhil Goyal, dev
Hi Akhil,
> > Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> > Coverity issue: 336844
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> > index e403c461a..3f9cacb8f 100644
> > --- a/examples/ipsec-secgw/ipsec_process.c
> > +++ b/examples/ipsec-secgw/ipsec_process.c
> > @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> > pg = grp + i;
> > sa = pg->id.ptr;
> >
> > - /* no valid SA found */
> > - if (sa == NULL)
> > - k = 0;
> > -
> > ips = &sa->ips;
> I think this is not fixing the NULL dereference properly. This line
> would give fault if sa is null.
I don't think it would - here we just get an address of ips,
we don't try to access it.
Konstantin
> > - satp = rte_ipsec_sa_type(ips->sa);
> >
> > /* no valid HW session for that SA, try to create one */
> > - if (ips->crypto.ses == NULL &&
> > - fill_ipsec_session(ips, ctx, sa) != 0)
> > + if (sa == NULL || (ips->crypto.ses == NULL &&
> > + fill_ipsec_session(ips, ctx, sa) != 0))
> > k = 0;
> >
> > /* process packets inline */
> > @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> > sa->type ==
> > RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
> >
> > + satp = rte_ipsec_sa_type(ips->sa);
> > +
> > /*
> > * This is just to satisfy inbound_sa_check()
> > * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 13:19 ` Ananyev, Konstantin
2019-03-27 13:19 ` Ananyev, Konstantin
@ 2019-03-27 14:37 ` Akhil Goyal
2019-03-27 14:37 ` Akhil Goyal
1 sibling, 1 reply; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 14:37 UTC (permalink / raw)
To: Ananyev, Konstantin, dev
On 3/27/2019 6:49 PM, Ananyev, Konstantin wrote:
> Hi Akhil,
>
>>> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
>>> Coverity issue: 336844
>>>
>>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>>> ---
>>> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
>>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
>>> index e403c461a..3f9cacb8f 100644
>>> --- a/examples/ipsec-secgw/ipsec_process.c
>>> +++ b/examples/ipsec-secgw/ipsec_process.c
>>> @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>>> pg = grp + i;
>>> sa = pg->id.ptr;
>>>
>>> - /* no valid SA found */
>>> - if (sa == NULL)
>>> - k = 0;
>>> -
>>> ips = &sa->ips;
>> I think this is not fixing the NULL dereference properly. This line
>> would give fault if sa is null.
> I don't think it would - here we just get an address of ips,
> we don't try to access it.
> Konstantin
ok got it.. I missed the '&'..
>
>>> - satp = rte_ipsec_sa_type(ips->sa);
>>>
>>> /* no valid HW session for that SA, try to create one */
>>> - if (ips->crypto.ses == NULL &&
>>> - fill_ipsec_session(ips, ctx, sa) != 0)
>>> + if (sa == NULL || (ips->crypto.ses == NULL &&
>>> + fill_ipsec_session(ips, ctx, sa) != 0))
>>> k = 0;
>>>
>>> /* process packets inline */
>>> @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>>> sa->type ==
>>> RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
>>>
>>> + satp = rte_ipsec_sa_type(ips->sa);
>>> +
>>> /*
>>> * This is just to satisfy inbound_sa_check()
>>> * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference
2019-03-27 14:37 ` Akhil Goyal
@ 2019-03-27 14:37 ` Akhil Goyal
0 siblings, 0 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 14:37 UTC (permalink / raw)
To: Ananyev, Konstantin, dev
On 3/27/2019 6:49 PM, Ananyev, Konstantin wrote:
> Hi Akhil,
>
>>> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
>>> Coverity issue: 336844
>>>
>>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>>> ---
>>> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
>>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
>>> index e403c461a..3f9cacb8f 100644
>>> --- a/examples/ipsec-secgw/ipsec_process.c
>>> +++ b/examples/ipsec-secgw/ipsec_process.c
>>> @@ -217,16 +217,11 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>>> pg = grp + i;
>>> sa = pg->id.ptr;
>>>
>>> - /* no valid SA found */
>>> - if (sa == NULL)
>>> - k = 0;
>>> -
>>> ips = &sa->ips;
>> I think this is not fixing the NULL dereference properly. This line
>> would give fault if sa is null.
> I don't think it would - here we just get an address of ips,
> we don't try to access it.
> Konstantin
ok got it.. I missed the '&'..
>
>>> - satp = rte_ipsec_sa_type(ips->sa);
>>>
>>> /* no valid HW session for that SA, try to create one */
>>> - if (ips->crypto.ses == NULL &&
>>> - fill_ipsec_session(ips, ctx, sa) != 0)
>>> + if (sa == NULL || (ips->crypto.ses == NULL &&
>>> + fill_ipsec_session(ips, ctx, sa) != 0))
>>> k = 0;
>>>
>>> /* process packets inline */
>>> @@ -234,6 +229,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>>> sa->type ==
>>> RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
>>>
>>> + satp = rte_ipsec_sa_type(ips->sa);
>>> +
>>> /*
>>> * This is just to satisfy inbound_sa_check()
>>> * and get_hop_for_offload_pkt().
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script
2019-03-27 9:33 [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Konstantin Ananyev
` (2 preceding siblings ...)
2019-03-27 9:33 ` [dpdk-dev] [PATCH 2/3] examples/ipsec_secgw: fix possible NULL dereference Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:55 ` Akhil Goyal
2019-03-29 14:21 ` [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Akhil Goyal
4 siblings, 2 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fix typo in run_test.sh
Fixes: 929784452094 ("examples/ipsec-secgw: add scripts for functional test")
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/test/run_test.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ipsec-secgw/test/run_test.sh b/examples/ipsec-secgw/test/run_test.sh
index d8c4b6625..3a1a7d4b4 100644
--- a/examples/ipsec-secgw/test/run_test.sh
+++ b/examples/ipsec-secgw/test/run_test.sh
@@ -66,7 +66,7 @@ while [[ ${st} -eq 0 ]]; do
fi
done
-if [[ ${run4} -eq 0 && {run6} -eq 0 ]]; then
+if [[ ${run4} -eq 0 && ${run6} -eq 0 ]]; then
exit 127
fi
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script
2019-03-27 9:33 ` [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script Konstantin Ananyev
@ 2019-03-27 9:33 ` Konstantin Ananyev
2019-03-27 12:55 ` Akhil Goyal
1 sibling, 0 replies; 20+ messages in thread
From: Konstantin Ananyev @ 2019-03-27 9:33 UTC (permalink / raw)
To: dev; +Cc: akhil.goyal, Konstantin Ananyev
Fix typo in run_test.sh
Fixes: 929784452094 ("examples/ipsec-secgw: add scripts for functional test")
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/test/run_test.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ipsec-secgw/test/run_test.sh b/examples/ipsec-secgw/test/run_test.sh
index d8c4b6625..3a1a7d4b4 100644
--- a/examples/ipsec-secgw/test/run_test.sh
+++ b/examples/ipsec-secgw/test/run_test.sh
@@ -66,7 +66,7 @@ while [[ ${st} -eq 0 ]]; do
fi
done
-if [[ ${run4} -eq 0 && {run6} -eq 0 ]]; then
+if [[ ${run4} -eq 0 && ${run6} -eq 0 ]]; then
exit 127
fi
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script
2019-03-27 9:33 ` [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script Konstantin Ananyev
2019-03-27 9:33 ` Konstantin Ananyev
@ 2019-03-27 12:55 ` Akhil Goyal
2019-03-27 12:55 ` Akhil Goyal
1 sibling, 1 reply; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:55 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fix typo in run_test.sh
> Fixes: 929784452094 ("examples/ipsec-secgw: add scripts for functional test")
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/test/run_test.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/test/run_test.sh b/examples/ipsec-secgw/test/run_test.sh
> index d8c4b6625..3a1a7d4b4 100644
> --- a/examples/ipsec-secgw/test/run_test.sh
> +++ b/examples/ipsec-secgw/test/run_test.sh
> @@ -66,7 +66,7 @@ while [[ ${st} -eq 0 ]]; do
> fi
> done
>
> -if [[ ${run4} -eq 0 && {run6} -eq 0 ]]; then
> +if [[ ${run4} -eq 0 && ${run6} -eq 0 ]]; then
> exit 127
> fi
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script
2019-03-27 12:55 ` Akhil Goyal
@ 2019-03-27 12:55 ` Akhil Goyal
0 siblings, 0 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-27 12:55 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> Fix typo in run_test.sh
> Fixes: 929784452094 ("examples/ipsec-secgw: add scripts for functional test")
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> examples/ipsec-secgw/test/run_test.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/test/run_test.sh b/examples/ipsec-secgw/test/run_test.sh
> index d8c4b6625..3a1a7d4b4 100644
> --- a/examples/ipsec-secgw/test/run_test.sh
> +++ b/examples/ipsec-secgw/test/run_test.sh
> @@ -66,7 +66,7 @@ while [[ ${st} -eq 0 ]]; do
> fi
> done
>
> -if [[ ${run4} -eq 0 && {run6} -eq 0 ]]; then
> +if [[ ${run4} -eq 0 && ${run6} -eq 0 ]]; then
> exit 127
> fi
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw
2019-03-27 9:33 [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Konstantin Ananyev
` (3 preceding siblings ...)
2019-03-27 9:33 ` [dpdk-dev] [PATCH 3/3] examples/ipsec-secgw: fix typo in test script Konstantin Ananyev
@ 2019-03-29 14:21 ` Akhil Goyal
2019-03-29 14:21 ` Akhil Goyal
4 siblings, 1 reply; 20+ messages in thread
From: Akhil Goyal @ 2019-03-29 14:21 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> three simple fixes for ipsec-secgw, first two reported by Coverity.
>
> Konstantin Ananyev (3):
> examples/ipsec-secgw: fix invalid out-of-bound check
> examples/ipsec_secgw: fix possible NULL dereference
> examples/ipsec-secgw: fix typo in test script
>
> examples/ipsec-secgw/ipsec-secgw.c | 2 +-
> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> examples/ipsec-secgw/test/run_test.sh | 2 +-
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
Applied to dpdk-next-crypto
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw
2019-03-29 14:21 ` [dpdk-dev] [PATCH 0/3] few trivial fixes for ipsec-secgw Akhil Goyal
@ 2019-03-29 14:21 ` Akhil Goyal
0 siblings, 0 replies; 20+ messages in thread
From: Akhil Goyal @ 2019-03-29 14:21 UTC (permalink / raw)
To: Konstantin Ananyev, dev
On 3/27/2019 3:03 PM, Konstantin Ananyev wrote:
> three simple fixes for ipsec-secgw, first two reported by Coverity.
>
> Konstantin Ananyev (3):
> examples/ipsec-secgw: fix invalid out-of-bound check
> examples/ipsec_secgw: fix possible NULL dereference
> examples/ipsec-secgw: fix typo in test script
>
> examples/ipsec-secgw/ipsec-secgw.c | 2 +-
> examples/ipsec-secgw/ipsec_process.c | 11 ++++-------
> examples/ipsec-secgw/test/run_test.sh | 2 +-
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
Applied to dpdk-next-crypto
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread