[dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length

DPDK patches and discussions
 help / color / mirror / Atom feed

* [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length
@ 2021-05-18 15:10 Ivan Malov
  2021-05-18 15:10 ` [dpdk-dev] [PATCH 2/2] common/sfc_efx/base: add missing MCDI response length checks Ivan Malov
  2021-05-18 16:41 ` [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ferruh Yigit
  0 siblings, 2 replies; 3+ messages in thread
From: Ivan Malov @ 2021-05-18 15:10 UTC (permalink / raw)
  To: dev; +Cc: Andy Moreton, stable, Andrew Rybchenko, Ferruh Yigit

From: Andy Moreton <amoreton@xilinx.com>

MCDI helper routines in libefx include length checks for response
messages, to ensure that short replies and optional fields are
handled correctly.

If the MCDI response message from the firmware is larger than the
caller's buffer then the response length reported to the caller
should be limited to the buffer size. Otherwise length checks in
the caller may allow reading past the end of the buffer.

Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
Cc: stable@dpdk.org

Signed-off-by: Andy Moreton <amoreton@xilinx.com>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/efx_mcdi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/common/sfc_efx/base/efx_mcdi.c b/drivers/common/sfc_efx/base/efx_mcdi.c
index ff676f8a0..f4e1384d0 100644
--- a/drivers/common/sfc_efx/base/efx_mcdi.c
+++ b/drivers/common/sfc_efx/base/efx_mcdi.c
@@ -516,6 +516,9 @@ efx_mcdi_finish_response(
 	bytes = MIN(emrp->emr_out_length_used, emrp->emr_out_length);
 	efx_mcdi_read_response(enp, emrp->emr_out_buf, resp_off, bytes);
 
+	/* Report bytes copied to caller (response message may be larger) */
+	emrp->emr_out_length_used = bytes;
+
 #if EFSYS_OPT_MCDI_LOGGING
 	if (emtp->emt_logger != NULL) {
 		emtp->emt_logger(emtp->emt_context,
-- 
2.20.1


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

* [dpdk-dev] [PATCH 2/2] common/sfc_efx/base: add missing MCDI response length checks
  2021-05-18 15:10 [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ivan Malov
@ 2021-05-18 15:10 ` Ivan Malov
  2021-05-18 16:41 ` [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ferruh Yigit
  1 sibling, 0 replies; 3+ messages in thread
From: Ivan Malov @ 2021-05-18 15:10 UTC (permalink / raw)
  To: dev; +Cc: Andy Moreton, stable, Andrew Rybchenko, Ferruh Yigit

From: Andy Moreton <amoreton@xilinx.com>

Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
Fixes: e7cd430c864f ("net/sfc/base: import SFN7xxx family support")
Fixes: 94190e3543bf ("net/sfc/base: import SFN8xxx family support")
Fixes: 34285fd0891d ("common/sfc_efx/base: add match spec validate API")
Fixes: e61baa82e64b ("common/sfc_efx/base: add MAE action set provisioning APIs")
Fixes: b4fac34715f2 ("common/sfc_efx/base: add MAE action rule provisioning APIs")
Fixes: ed15d7f8e064 ("common/sfc_efx/base: validate and compare outer match specs")
Fixes: 7a673e1a4a05 ("common/sfc_efx/base: support outer rule provisioning")
Cc: stable@dpdk.org

Signed-off-by: Andy Moreton <amoreton@xilinx.com>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_filter.c | 11 ++++-
 drivers/common/sfc_efx/base/ef10_nic.c    | 10 ++++-
 drivers/common/sfc_efx/base/efx_mae.c     | 52 +++++++++++++++++++----
 drivers/common/sfc_efx/base/efx_mcdi.c    |  7 +++
 4 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/drivers/common/sfc_efx/base/ef10_filter.c b/drivers/common/sfc_efx/base/ef10_filter.c
index 0c99d4b74..ac6006c9b 100644
--- a/drivers/common/sfc_efx/base/ef10_filter.c
+++ b/drivers/common/sfc_efx/base/ef10_filter.c
@@ -1225,20 +1225,25 @@ efx_mcdi_get_parser_disp_info(
 		goto fail1;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_GET_PARSER_DISP_INFO_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail2;
+	}
+
 	matches_count = MCDI_OUT_DWORD(req,
 	    GET_PARSER_DISP_INFO_OUT_NUM_SUPPORTED_MATCHES);
 
 	if (req.emr_out_length_used <
 	    MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(matches_count)) {
 		rc = EMSGSIZE;
-		goto fail2;
+		goto fail3;
 	}
 
 	*list_lengthp = matches_count;
 
 	if (buffer_length < matches_count) {
 		rc = ENOSPC;
-		goto fail3;
+		goto fail4;
 	}
 
 	/*
@@ -1258,6 +1263,8 @@ efx_mcdi_get_parser_disp_info(
 
 	return (0);
 
+fail4:
+	EFSYS_PROBE(fail4);
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index 531365e42..eda0ad306 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -491,11 +491,17 @@ efx_mcdi_get_rxdp_config(
 	req.emr_out_length = MC_CMD_GET_RXDP_CONFIG_OUT_LEN;
 
 	efx_mcdi_execute(enp, &req);
+
 	if (req.emr_rc != 0) {
 		rc = req.emr_rc;
 		goto fail1;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_GET_RXDP_CONFIG_OUT_LEN) {
+		rc = EMSGSIZE;
+		goto fail2;
+	}
+
 	if (MCDI_OUT_DWORD_FIELD(req, GET_RXDP_CONFIG_OUT_DATA,
 				    GET_RXDP_CONFIG_OUT_PAD_HOST_DMA) == 0) {
 		/* RX DMA end padding is disabled */
@@ -514,7 +520,7 @@ efx_mcdi_get_rxdp_config(
 			break;
 		default:
 			rc = ENOTSUP;
-			goto fail2;
+			goto fail3;
 		}
 	}
 
@@ -522,6 +528,8 @@ efx_mcdi_get_rxdp_config(
 
 	return (0);
 
+fail3:
+	EFSYS_PROBE(fail3);
 fail2:
 	EFSYS_PROBE(fail2);
 fail1:
diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
index 80fe155d0..c1784211e 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -109,17 +109,22 @@ efx_mae_get_outer_rule_caps(
 		goto fail2;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_MAE_GET_OR_CAPS_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail3;
+	}
+
 	mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_OR_CAPS_OUT_COUNT);
 
 	if (req.emr_out_length_used <
 	    MC_CMD_MAE_GET_OR_CAPS_OUT_LEN(mcdi_field_ncaps)) {
 		rc = EMSGSIZE;
-		goto fail3;
+		goto fail4;
 	}
 
 	if (mcdi_field_ncaps > field_ncaps) {
 		rc = EMSGSIZE;
-		goto fail4;
+		goto fail5;
 	}
 
 	for (i = 0; i < mcdi_field_ncaps; ++i) {
@@ -147,6 +152,8 @@ efx_mae_get_outer_rule_caps(
 
 	return (0);
 
+fail5:
+	EFSYS_PROBE(fail5);
 fail4:
 	EFSYS_PROBE(fail4);
 fail3:
@@ -191,17 +198,22 @@ efx_mae_get_action_rule_caps(
 		goto fail2;
 	}
 
-	mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_OR_CAPS_OUT_COUNT);
+	if (req.emr_out_length_used < MC_CMD_MAE_GET_AR_CAPS_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail3;
+	}
+
+	mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_AR_CAPS_OUT_COUNT);
 
 	if (req.emr_out_length_used <
 	    MC_CMD_MAE_GET_AR_CAPS_OUT_LEN(mcdi_field_ncaps)) {
 		rc = EMSGSIZE;
-		goto fail3;
+		goto fail4;
 	}
 
 	if (mcdi_field_ncaps > field_ncaps) {
 		rc = EMSGSIZE;
-		goto fail4;
+		goto fail5;
 	}
 
 	for (i = 0; i < mcdi_field_ncaps; ++i) {
@@ -229,6 +241,8 @@ efx_mae_get_action_rule_caps(
 
 	return (0);
 
+fail5:
+	EFSYS_PROBE(fail5);
 fail4:
 	EFSYS_PROBE(fail4);
 fail3:
@@ -1773,15 +1787,22 @@ efx_mae_outer_rule_remove(
 		goto fail2;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_MAE_OUTER_RULE_REMOVE_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail3;
+	}
+
 	if (MCDI_OUT_DWORD(req, MAE_OUTER_RULE_REMOVE_OUT_REMOVED_OR_ID) !=
 	    or_idp->id) {
 		/* Firmware failed to remove the outer rule. */
 		rc = EAGAIN;
-		goto fail3;
+		goto fail4;
 	}
 
 	return (0);
 
+fail4:
+	EFSYS_PROBE(fail4);
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
@@ -2176,15 +2197,22 @@ efx_mae_action_set_free(
 		goto fail2;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_MAE_ACTION_SET_FREE_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail3;
+	}
+
 	if (MCDI_OUT_DWORD(req, MAE_ACTION_SET_FREE_OUT_FREED_AS_ID) !=
 	    aset_idp->id) {
 		/* Firmware failed to free the action set. */
 		rc = EAGAIN;
-		goto fail3;
+		goto fail4;
 	}
 
 	return (0);
 
+fail4:
+	EFSYS_PROBE(fail4);
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
@@ -2326,15 +2354,23 @@ efx_mae_action_rule_remove(
 		goto fail2;
 	}
 
+	if (req.emr_out_length_used <
+	    MC_CMD_MAE_ACTION_RULE_DELETE_OUT_LENMIN) {
+		rc = EMSGSIZE;
+		goto fail3;
+	}
+
 	if (MCDI_OUT_DWORD(req, MAE_ACTION_RULE_DELETE_OUT_DELETED_AR_ID) !=
 	    ar_idp->id) {
 		/* Firmware failed to delete the action rule. */
 		rc = EAGAIN;
-		goto fail3;
+		goto fail4;
 	}
 
 	return (0);
 
+fail4:
+	EFSYS_PROBE(fail4);
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
diff --git a/drivers/common/sfc_efx/base/efx_mcdi.c b/drivers/common/sfc_efx/base/efx_mcdi.c
index f4e1384d0..f226ffd92 100644
--- a/drivers/common/sfc_efx/base/efx_mcdi.c
+++ b/drivers/common/sfc_efx/base/efx_mcdi.c
@@ -2294,6 +2294,11 @@ efx_mcdi_get_workarounds(
 		goto fail1;
 	}
 
+	if (req.emr_out_length_used < MC_CMD_GET_WORKAROUNDS_OUT_LEN) {
+		rc = EMSGSIZE;
+		goto fail2;
+	}
+
 	if (implementedp != NULL) {
 		*implementedp =
 		    MCDI_OUT_DWORD(req, GET_WORKAROUNDS_OUT_IMPLEMENTED);
@@ -2305,6 +2310,8 @@ efx_mcdi_get_workarounds(
 
 	return (0);
 
+fail2:
+	EFSYS_PROBE(fail2);
 fail1:
 	EFSYS_PROBE1(fail1, efx_rc_t, rc);
 
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length
  2021-05-18 15:10 [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ivan Malov
  2021-05-18 15:10 ` [dpdk-dev] [PATCH 2/2] common/sfc_efx/base: add missing MCDI response length checks Ivan Malov
@ 2021-05-18 16:41 ` Ferruh Yigit
  1 sibling, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2021-05-18 16:41 UTC (permalink / raw)
  To: Ivan Malov, dev; +Cc: Andy Moreton, stable, Andrew Rybchenko

On 5/18/2021 4:10 PM, Ivan Malov wrote:
> From: Andy Moreton <amoreton@xilinx.com>
> 
> MCDI helper routines in libefx include length checks for response
> messages, to ensure that short replies and optional fields are
> handled correctly.
> 
> If the MCDI response message from the firmware is larger than the
> caller's buffer then the response length reported to the caller
> should be limited to the buffer size. Otherwise length checks in
> the caller may allow reading past the end of the buffer.
> 
> Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andy Moreton <amoreton@xilinx.com>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Series applied to dpdk-next-net/main, thanks.

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

end of thread, other threads:[~2021-05-18 16:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18 15:10 [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ivan Malov
2021-05-18 15:10 ` [dpdk-dev] [PATCH 2/2] common/sfc_efx/base: add missing MCDI response length checks Ivan Malov
2021-05-18 16:41 ` [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length Ferruh Yigit

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