In efx_probe_filters, the channel->rps_flow_id is freed in a
efx_for_each_channel marco when success equals to 0.
However, after the following call chain:
ef100_net_open
|-> efx_probe_filters
|-> ef100_net_stop
|-> efx_remove_filters
The channel->rps_flow_id is freed again in the efx_for_each_channel of
efx_remove_filters, triggering a double-free bug.
---
Changelog:
v2: Correct the call-chain description in commit message and change
patch subject.
Fixes: a9dc3d5612ce ("sfc_ef100: RX filter table management and related gubbins")
Signed-off-by: Zhipeng Lu <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
Reviewed-by: Edward Cree <[email protected]>
---
drivers/net/ethernet/sfc/rx_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index d2f35ee15eff..fac227d372db 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -823,8 +823,10 @@ int efx_probe_filters(struct efx_nic *efx)
}
if (!success) {
- efx_for_each_channel(channel, efx)
+ efx_for_each_channel(channel, efx) {
kfree(channel->rps_flow_id);
+ channel->rps_flow_id = NULL;
+ }
efx->type->filter_table_remove(efx);
rc = -ENOMEM;
goto out_unlock;
--
2.34.1
On Fri, Dec 22, 2023 at 11:49:52PM +0800, Zhipeng Lu wrote:
> In efx_probe_filters, the channel->rps_flow_id is freed in a
> efx_for_each_channel marco when success equals to 0.
> However, after the following call chain:
>
> ef100_net_open
> |-> efx_probe_filters
> |-> ef100_net_stop
> |-> efx_remove_filters
>
> The channel->rps_flow_id is freed again in the efx_for_each_channel of
> efx_remove_filters, triggering a double-free bug.
> ---
Everything below the line above (---) will be omitted from the commit
message when the patch is applied.
> Changelog:
>
> v2: Correct the call-chain description in commit message and change
> patch subject.
>
> Fixes: a9dc3d5612ce ("sfc_ef100: RX filter table management and related gubbins")
> Signed-off-by: Zhipeng Lu <[email protected]>
> Reviewed-by: Simon Horman <[email protected]>
> Reviewed-by: Edward Cree <[email protected]>
Hi Zhipeng Lu,
I think that your Signed-off-by should go last when you post a patch.
And the Changelog should go below the (first set of) scissors (---).
> ---
> drivers/net/ethernet/sfc/rx_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
With the above in mind, I think you want something like:
In efx_probe_filters, the channel->rps_flow_id is freed in a
efx_for_each_channel marco when success equals to 0.
However, after the following call chain:
ef100_net_open
|-> efx_probe_filters
|-> ef100_net_stop
|-> efx_remove_filters
The channel->rps_flow_id is freed again in the efx_for_each_channel of
efx_remove_filters, triggering a double-free bug.
Fixes: a9dc3d5612ce ("sfc_ef100: RX filter table management and related gubbins")
Reviewed-by: Simon Horman <[email protected]>
Reviewed-by: Edward Cree <[email protected]>
Signed-off-by: Zhipeng Lu <[email protected]>
---
Changelog:
v2: Correct the call-chain description in commit message and change
patch subject.
---
drivers/net/ethernet/sfc/rx_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--
pw-bot: changes-requested
> On Fri, Dec 22, 2023 at 11:49:52PM +0800, Zhipeng Lu wrote:
> > In efx_probe_filters, the channel->rps_flow_id is freed in a
> > efx_for_each_channel marco when success equals to 0.
> > However, after the following call chain:
> >
> > ef100_net_open
> > |-> efx_probe_filters
> > |-> ef100_net_stop
> > |-> efx_remove_filters
> >
> > The channel->rps_flow_id is freed again in the efx_for_each_channel of
> > efx_remove_filters, triggering a double-free bug.
> > ---
>
> Everything below the line above (---) will be omitted from the commit
> message when the patch is applied.
>
> > Changelog:
> >
> > v2: Correct the call-chain description in commit message and change
> > patch subject.
> >
> > Fixes: a9dc3d5612ce ("sfc_ef100: RX filter table management and related gubbins")
> > Signed-off-by: Zhipeng Lu <[email protected]>
> > Reviewed-by: Simon Horman <[email protected]>
> > Reviewed-by: Edward Cree <[email protected]>
>
> Hi Zhipeng Lu,
>
> I think that your Signed-off-by should go last when you post a patch.
>
> And the Changelog should go below the (first set of) scissors (---).
>
> > ---
> > drivers/net/ethernet/sfc/rx_common.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
>
> With the above in mind, I think you want something like:
>
> In efx_probe_filters, the channel->rps_flow_id is freed in a
> efx_for_each_channel marco when success equals to 0.
> However, after the following call chain:
>
> ef100_net_open
> |-> efx_probe_filters
> |-> ef100_net_stop
> |-> efx_remove_filters
>
> The channel->rps_flow_id is freed again in the efx_for_each_channel of
> efx_remove_filters, triggering a double-free bug.
>
> Fixes: a9dc3d5612ce ("sfc_ef100: RX filter table management and related gubbins")
> Reviewed-by: Simon Horman <[email protected]>
> Reviewed-by: Edward Cree <[email protected]>
> Signed-off-by: Zhipeng Lu <[email protected]>
> ---
> Changelog:
>
> v2: Correct the call-chain description in commit message and change
> patch subject.
> ---
> drivers/net/ethernet/sfc/rx_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --
> pw-bot: changes-requested
Thank you for your detailed revision and correction!
I'll send this patch again with your correction.