On Fri, Nov 03, 2023 at 11:23:16AM -0700, Justin Chen wrote:

On 11/3/23 5:57 AM, Greg KH wrote:

quoted

On Fri, Nov 03, 2023 at 05:57:48PM +0530, Yuran Pereira wrote:

quoted

The functions `bcmasp_netfilt_rd` and `bcmasp_netfilt_wr` both call
`bcmasp_netfilt_get_reg_offset` which, when it fails, returns `-EINVAL`.
This could lead to an out-of-bounds read or write when `rx_filter_core_rl`
or `rx_filter_core_wl` is called.

This patch adds a check in both functions to return immediately if
`bcmasp_netfilt_get_reg_offset` fails. This prevents potential out-of-bounds read
or writes, and ensures that no undefined or buggy behavior would originate from
the failure of `bcmasp_netfilt_get_reg_offset`.

Addresses-Coverity-IDs: 1544536 ("Out-of-bounds access")
Signed-off-by: Yuran Pereira <redacted>
---
  drivers/net/ethernet/broadcom/asp2/bcmasp.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
index 29b04a274d07..8b90b761bdec 100644
--- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c
+++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c

@@ -227,6 +227,8 @@ static void bcmasp_netfilt_wr(struct bcmasp_priv *priv,
  	reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
  						   offset);
+	if (reg_offset < 0)
+		return;
  	rx_filter_core_wl(priv, val, reg_offset);
  }

@@ -244,6 +246,8 @@ static u32 bcmasp_netfilt_rd(struct bcmasp_priv *priv,
  	reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
  						   offset);
+	if (reg_offset < 0)
+		return 0;

Shouldn't you return an error here?

thanks

greg k-h

As long as offset is less than MAX_WAKE_FILTER_SIZE we don't need to worry
about error checking. This is already checked before we call
netfilt_get_reg_offset() in both cases. Instead of returning -EINVAL in
neffilt_get_reg_offset() lets return 0. This will silence the coverity
check. In practice we will never hit this logic.

Then don't change it, coverity is incorrect here.

greg k-h