Skip to contentSkip to conversation

ipmi: Fix user refcount underflow in event delivery

PATCH In review Matt Fleming 2 participants 2 messages May 21

Subsystems (2)
Stable branch IPMI
lore
Share & tools
lore

Matt Fleming

author
#0 lore raw

PATCH 1/1 ipmi: Fix user refcount underflow in event delivery

From: Matt Fleming <[email protected]>

ipmi_alloc_recv_msg(user) takes the temporary user reference owned by the receive message, and ipmi_free_recv_msg() drops it again. If event delivery fails after allocating receive messages for earlier users, handle_read_event_rsp() rolls those messages back with ipmi_free_recv_msg().

That rollback path still drops user->refcount explicitly after freeing each message. The extra put can free a user that remains linked on intf->users, so later event delivery may dereference a freed user or trip refcount_t's addition-on-zero warning when ipmi_alloc_recv_msg() tries to acquire another reference.

Remove the stale explicit put and the now-dead user assignment. Keep the list_del() and ipmi_free_recv_msg() calls; they are the required rollback operations.

Fixes b52da4054ee0 ("ipmi: Rework user message limit handling")
Cc [email protected]
Signed-off-by Matt Fleming <[email protected]>
---
 drivers/char/ipmi/ipmi_msghandler.c | 2 --
 1 file changed, 2 deletions(-)
drivers/char/ipmi/ipmi_msghandler.c +0 -2
index 869ac87a4b6a..52561a880e54 100644
@@ -4477,10 +4477,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 			mutex_unlock(&intf->users_mutex);
 			list_for_each_entry_safe(recv_msg, recv_msg2, &msgs,
 						 link) {
-				user = recv_msg->user;
 				list_del(&recv_msg->link);
 				ipmi_free_recv_msg(recv_msg);
-				kref_put(&user->refcount, free_ipmi_user);
 			}
 			/*
 			 * We couldn't allocate memory for the
Signature · 1 line
2.43.0
maintainer
#1 lore raw
Quoted 14 lines · Matt Fleming

From: Matt Fleming <[email protected]>

ipmi_alloc_recv_msg(user) takes the temporary user reference owned by the receive message, and ipmi_free_recv_msg() drops it again. If event delivery fails after allocating receive messages for earlier users, handle_read_event_rsp() rolls those messages back with ipmi_free_recv_msg().

That rollback path still drops user->refcount explicitly after freeing each message. The extra put can free a user that remains linked on intf->users, so later event delivery may dereference a freed user or trip refcount_t's addition-on-zero warning when ipmi_alloc_recv_msg() tries to acquire another reference.

Quoted 3 lines

Remove the stale explicit put and the now-dead user assignment. Keep the list_del() and ipmi_free_recv_msg() calls; they are the required rollback operations.

Yes, this is correct. Queued in the ipmi next tree for next release.

Thanks,

-corey

Quoted 26 lines
Fixes b52da4054ee0 ("ipmi: Rework user message limit handling")
Cc [email protected]
Signed-off-by Matt Fleming <[email protected]>
---
 drivers/char/ipmi/ipmi_msghandler.c | 2 --
 1 file changed, 2 deletions(-)
drivers/char/ipmi/ipmi_msghandler.c +0 -2
index 869ac87a4b6a..52561a880e54 100644
@@ -4477,10 +4477,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 			mutex_unlock(&intf->users_mutex);
 			list_for_each_entry_safe(recv_msg, recv_msg2, &msgs,
 						 link) {
-				user = recv_msg->user;
 				list_del(&recv_msg->link);
 				ipmi_free_recv_msg(recv_msg);
-				kref_put(&user->refcount, free_ipmi_user);
 			}
 			/*
 			 * We couldn't allocate memory for the
Signature · 1 line
2.43.0