| Message ID | 20230908061950.20287-2-hbh25y@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9b90aca97f6d5255ca41e716720d138b878cd034 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix possible OOB write when using rule_buf | expand |
On Fri, Sep 08, 2023 at 02:19:48PM +0800, Hangyu Hua wrote: > rule_locs is allocated in ethtool_get_rxnfc and the size is determined by > rule_cnt from user space. So rule_cnt needs to be check before using > rule_locs to avoid OOB writing or NULL pointer dereference. > > Fixes: c5d511c49587 ("net: bcmasp: Add support for wake on net filters") > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c index d63d321f3e7b..e45e0ae932cc 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c @@ -528,13 +528,16 @@ void bcmasp_netfilt_suspend(struct bcmasp_intf *intf) ASP_RX_FILTER_BLK_CTRL); } -void bcmasp_netfilt_get_all_active(struct bcmasp_intf *intf, u32 *rule_locs, - u32 *rule_cnt) +int bcmasp_netfilt_get_all_active(struct bcmasp_intf *intf, u32 *rule_locs, + u32 *rule_cnt) { struct bcmasp_priv *priv = intf->parent; int j = 0, i; for (i = 0; i < NUM_NET_FILTERS; i++) { + if (j == *rule_cnt) + return -EMSGSIZE; + if (!priv->net_filters[i].claimed || priv->net_filters[i].port != intf->port) continue; @@ -548,6 +551,8 @@ void bcmasp_netfilt_get_all_active(struct bcmasp_intf *intf, u32 *rule_locs, } *rule_cnt = j; + + return 0; } int bcmasp_netfilt_get_active(struct bcmasp_intf *intf) diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.h b/drivers/net/ethernet/broadcom/asp2/bcmasp.h index 5b512f7f5e94..ec90add6b03e 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp.h +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.h @@ -577,8 +577,8 @@ void bcmasp_netfilt_release(struct bcmasp_intf *intf, int bcmasp_netfilt_get_active(struct bcmasp_intf *intf); -void bcmasp_netfilt_get_all_active(struct bcmasp_intf *intf, u32 *rule_locs, - u32 *rule_cnt); +int bcmasp_netfilt_get_all_active(struct bcmasp_intf *intf, u32 *rule_locs, + u32 *rule_cnt); void bcmasp_netfilt_suspend(struct bcmasp_intf *intf); diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c index c4f1604d5ab3..ce6a3d56fb23 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c @@ -335,7 +335,7 @@ static int bcmasp_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd, err = bcmasp_flow_get(intf, cmd); break; case ETHTOOL_GRXCLSRLALL: - bcmasp_netfilt_get_all_active(intf, rule_locs, &cmd->rule_cnt); + err = bcmasp_netfilt_get_all_active(intf, rule_locs, &cmd->rule_cnt); cmd->data = NUM_NET_FILTERS; break; default:
rule_locs is allocated in ethtool_get_rxnfc and the size is determined by rule_cnt from user space. So rule_cnt needs to be check before using rule_locs to avoid OOB writing or NULL pointer dereference. Fixes: c5d511c49587 ("net: bcmasp: Add support for wake on net filters") Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- v2: use -EMSGSIZE instead of truncating the list sliently. drivers/net/ethernet/broadcom/asp2/bcmasp.c | 9 +++++++-- drivers/net/ethernet/broadcom/asp2/bcmasp.h | 4 ++-- drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-)