[PATCH net-next] ibmvnic: Free rwi on reset success

[PATCH net-next] ibmvnic: Free rwi on reset success

[Nick Child]

Free the rwi structure in the event that the last rwi in the list
processed successfully. The logic in commit 4f408e1fa6e1 ("ibmvnic:
retry reset if there are no other resets") introduces an issue that
results in a 32 byte memory leak whenever the last rwi in the list
gets processed.

Fixes: 4f408e1fa6e1 ("ibmvnic: retry reset if there are no other resets")
Signed-off-by: Nick Child <nnac123@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 65dbfbec487a..9282381a438f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3007,19 +3007,19 @@ static void __ibmvnic_reset(struct work_struct *work)
 		rwi = get_next_rwi(adapter);
 
 		/*
-		 * If there is another reset queued, free the previous rwi
-		 * and process the new reset even if previous reset failed
-		 * (the previous reset could have failed because of a fail
-		 * over for instance, so process the fail over).
-		 *
 		 * If there are no resets queued and the previous reset failed,
 		 * the adapter would be in an undefined state. So retry the
 		 * previous reset as a hard reset.
+		 *
+		 * Else, free the previous rwi and, if there is another reset
+		 * queued, process the new reset even if previous reset failed
+		 * (the previous reset could have failed because of a fail
+		 * over for instance, so process the fail over).
 		 */
-		if (rwi)
-			kfree(tmprwi);
-		else if (rc)
+		if (!rwi && rc)
 			rwi = tmprwi;
+		else
+			kfree(tmprwi);
 
 		if (rwi && (rwi->reset_reason == VNIC_RESET_FAILOVER ||
 			    rwi->reset_reason == VNIC_RESET_MOBILITY || rc))
-- 
2.31.1

Re: [PATCH net-next] ibmvnic: Free rwi on reset success

[Jakub Kicinski]

On Mon, 17 Oct 2022 10:15:16 -0500 Nick Child wrote:
> Subject: [PATCH net-next] ibmvnic: Free rwi on reset success

Why net-next? it's a fix, right? it should go to Linus in the current
release cycle, i.e. the next -rc release.

Please make sure to CC the authors of the change under Fixes, and 
the maintainers of the driver. ./scripts/get_maintainer is your friend.

> Free the rwi structure in the event that the last rwi in the list
> processed successfully. The logic in commit 4f408e1fa6e1 ("ibmvnic:
> retry reset if there are no other resets") introduces an issue that
> results in a 32 byte memory leak whenever the last rwi in the list
> gets processed.
> 
> Fixes: 4f408e1fa6e1 ("ibmvnic: retry reset if there are no other resets")
> Signed-off-by: Nick Child <nnac123@linux.ibm.com>

Re: [PATCH net-next] ibmvnic: Free rwi on reset success

[Nick Child]

Hello Jakub,

Thanks for the review.

On 10/18/22 13:45, Jakub Kicinski wrote:
> On Mon, 17 Oct 2022 10:15:16 -0500 Nick Child wrote:
>> Subject: [PATCH net-next] ibmvnic: Free rwi on reset success
> 
> Why net-next? it's a fix, right? it should go to Linus in the current
> release cycle, i.e. the next -rc release.

Apologies, I must have misunderstood when to use net vs net-next.
The bug has been around since v5.14 so I did not want to clutter
net inbox with things not directly relevant to v6.1.
Would you like me to resend with the `net` tag?

> Please make sure to CC the authors of the change under Fixes, and
> the maintainers of the driver. ./scripts/get_maintainer is your friend.

The ibmvnic list of maintainers is due for an update. I have added the 
current team to the CC list. We will have a patch out soon to update 
MAINTAINERS.

Thanks again. Apologies for any confusion,
Nick Child

Re: [PATCH net-next] ibmvnic: Free rwi on reset success

[Jakub Kicinski]

On Tue, 18 Oct 2022 15:01:13 -0500 Nick Child wrote:
> On 10/18/22 13:45, Jakub Kicinski wrote:
> > On Mon, 17 Oct 2022 10:15:16 -0500 Nick Child wrote:  
> >> Subject: [PATCH net-next] ibmvnic: Free rwi on reset success  
> > 
> > Why net-next? it's a fix, right? it should go to Linus in the current
> > release cycle, i.e. the next -rc release.  
> 
> Apologies, I must have misunderstood when to use net vs net-next.
> The bug has been around since v5.14 so I did not want to clutter
> net inbox with things not directly relevant to v6.1.
> Would you like me to resend with the `net` tag?

Yes, anything that's a fix for code which is already present in net
should go to net. It will then make it to Linus within a week, and
Greg KH & co. can pick it up for stable trees (like 5.14) some time
later.

> > Please make sure to CC the authors of the change under Fixes, and
> > the maintainers of the driver. ./scripts/get_maintainer is your friend.  
> 
> The ibmvnic list of maintainers is due for an update. I have added the 
> current team to the CC list. We will have a patch out soon to update 
> MAINTAINERS.

I figured that may be the case, if it's not a problem I'd CC everyone
even if you know some of the addresses will bounce. Removes the need
for us to manually check why people aren't CCed.

BTW when you send the update to MAINTAINERS please target net as well,
MAINTAINERS updates are fast-tracked like fixes.