6.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Corey Minyard corey@minyard.net
commit 8fd8ea2869cfafb3b1d6f95ff49561b13a73438d upstream.
Dan Carpenter got a Smatch warning:
drivers/char/ipmi/ipmi_msghandler.c:5265 ipmi_free_recv_msg() warn: sleeping in atomic context
due to the recent rework of the IPMI driver's locking. I didn't realize vfree could block. But there is an easy solution to this, now that almost everything in the message handler runs in thread context.
I wanted to spend the time earlier to see if seq_lock could be converted from a spinlock to a mutex, but I wanted the previous changes to go in and soak before I did that. So I went ahead and did the analysis and converting should work. And solve this problem.
Reported-by: kernel test robot lkp@intel.com Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202503240244.LR7pOwyr-lkp@intel.com/ Fixes: 3be997d5a64a ("ipmi:msghandler: Remove srcu from the ipmi user structure") Cc: stable@vger.kernel.org # 6.16 Signed-off-by: Corey Minyard corey@minyard.net Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/char/ipmi/ipmi_msghandler.c | 63 ++++++++++++++---------------------- 1 file changed, 26 insertions(+), 37 deletions(-)
--- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -466,7 +466,7 @@ struct ipmi_smi { * interface to match them up with their responses. A routine * is called periodically to time the items in this list. */ - spinlock_t seq_lock; + struct mutex seq_lock; struct seq_table seq_table[IPMI_IPMB_NUM_SEQ]; int curr_seq;
@@ -1117,12 +1117,11 @@ static int intf_find_seq(struct ipmi_smi struct ipmi_recv_msg **recv_msg) { int rv = -ENODEV; - unsigned long flags;
if (seq >= IPMI_IPMB_NUM_SEQ) return -EINVAL;
- spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); if (intf->seq_table[seq].inuse) { struct ipmi_recv_msg *msg = intf->seq_table[seq].recv_msg;
@@ -1135,7 +1134,7 @@ static int intf_find_seq(struct ipmi_smi rv = 0; } } - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock);
return rv; } @@ -1146,14 +1145,13 @@ static int intf_start_seq_timer(struct i long msgid) { int rv = -ENODEV; - unsigned long flags; unsigned char seq; unsigned long seqid;
GET_SEQ_FROM_MSGID(msgid, seq, seqid);
- spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); /* * We do this verification because the user can be deleted * while a message is outstanding. @@ -1164,7 +1162,7 @@ static int intf_start_seq_timer(struct i ent->timeout = ent->orig_timeout; rv = 0; } - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock);
return rv; } @@ -1175,7 +1173,6 @@ static int intf_err_seq(struct ipmi_smi unsigned int err) { int rv = -ENODEV; - unsigned long flags; unsigned char seq; unsigned long seqid; struct ipmi_recv_msg *msg = NULL; @@ -1183,7 +1180,7 @@ static int intf_err_seq(struct ipmi_smi
GET_SEQ_FROM_MSGID(msgid, seq, seqid);
- spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); /* * We do this verification because the user can be deleted * while a message is outstanding. @@ -1197,7 +1194,7 @@ static int intf_err_seq(struct ipmi_smi msg = ent->recv_msg; rv = 0; } - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock);
if (msg) deliver_err_response(intf, msg, err); @@ -1210,7 +1207,6 @@ int ipmi_create_user(unsigned int void *handler_data, struct ipmi_user **user) { - unsigned long flags; struct ipmi_user *new_user = NULL; int rv = 0; struct ipmi_smi *intf; @@ -1278,9 +1274,9 @@ int ipmi_create_user(unsigned int new_user->gets_events = false;
mutex_lock(&intf->users_mutex); - spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); list_add(&new_user->link, &intf->users); - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock); mutex_unlock(&intf->users_mutex);
if (handler->ipmi_watchdog_pretimeout) @@ -1326,7 +1322,6 @@ static void _ipmi_destroy_user(struct ip { struct ipmi_smi *intf = user->intf; int i; - unsigned long flags; struct cmd_rcvr *rcvr; struct cmd_rcvr *rcvrs = NULL; struct ipmi_recv_msg *msg, *msg2; @@ -1347,7 +1342,7 @@ static void _ipmi_destroy_user(struct ip list_del(&user->link); atomic_dec(&intf->nr_users);
- spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) { if (intf->seq_table[i].inuse && (intf->seq_table[i].recv_msg->user == user)) { @@ -1356,7 +1351,7 @@ static void _ipmi_destroy_user(struct ip ipmi_free_recv_msg(intf->seq_table[i].recv_msg); } } - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock);
/* * Remove the user from the command receiver's table. First @@ -2026,10 +2021,7 @@ static int i_ipmi_req_ipmb(struct ipmi_s */ smi_msg->user_data = recv_msg; } else { - /* It's a command, so get a sequence for it. */ - unsigned long flags; - - spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock);
if (is_maintenance_mode_cmd(msg)) intf->ipmb_maintenance_mode_timeout = @@ -2087,7 +2079,7 @@ static int i_ipmi_req_ipmb(struct ipmi_s * to be correct. */ out_err: - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock); }
return rv; @@ -2205,10 +2197,7 @@ static int i_ipmi_req_lan(struct ipmi_sm */ smi_msg->user_data = recv_msg; } else { - /* It's a command, so get a sequence for it. */ - unsigned long flags; - - spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock);
/* * Create a sequence number with a 1 second @@ -2257,7 +2246,7 @@ static int i_ipmi_req_lan(struct ipmi_sm * to be correct. */ out_err: - spin_unlock_irqrestore(&intf->seq_lock, flags); + mutex_unlock(&intf->seq_lock); }
return rv; @@ -3562,7 +3551,7 @@ int ipmi_add_smi(struct module * atomic_set(&intf->nr_users, 0); intf->handlers = handlers; intf->send_info = send_info; - spin_lock_init(&intf->seq_lock); + mutex_init(&intf->seq_lock); for (j = 0; j < IPMI_IPMB_NUM_SEQ; j++) { intf->seq_table[j].inuse = 0; intf->seq_table[j].seqid = 0; @@ -4487,9 +4476,10 @@ static int handle_one_recv_msg(struct ip
if (msg->rsp_size < 2) { /* Message is too small to be correct. */ - dev_warn(intf->si_dev, - "BMC returned too small a message for netfn %x cmd %x, got %d bytes\n", - (msg->data[0] >> 2) | 1, msg->data[1], msg->rsp_size); + dev_warn_ratelimited(intf->si_dev, + "BMC returned too small a message for netfn %x cmd %x, got %d bytes\n", + (msg->data[0] >> 2) | 1, + msg->data[1], msg->rsp_size);
return_unspecified: /* Generate an error response for the message. */ @@ -4907,8 +4897,7 @@ smi_from_recv_msg(struct ipmi_smi *intf, static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, struct list_head *timeouts, unsigned long timeout_period, - int slot, unsigned long *flags, - bool *need_timer) + int slot, bool *need_timer) { struct ipmi_recv_msg *msg;
@@ -4960,7 +4949,7 @@ static void check_msg_timeout(struct ipm return; }
- spin_unlock_irqrestore(&intf->seq_lock, *flags); + mutex_unlock(&intf->seq_lock);
/* * Send the new message. We send with a zero @@ -4981,7 +4970,7 @@ static void check_msg_timeout(struct ipm } else ipmi_free_smi_msg(smi_msg);
- spin_lock_irqsave(&intf->seq_lock, *flags); + mutex_lock(&intf->seq_lock); } }
@@ -5008,7 +4997,7 @@ static bool ipmi_timeout_handler(struct * list. */ INIT_LIST_HEAD(&timeouts); - spin_lock_irqsave(&intf->seq_lock, flags); + mutex_lock(&intf->seq_lock); if (intf->ipmb_maintenance_mode_timeout) { if (intf->ipmb_maintenance_mode_timeout <= timeout_period) intf->ipmb_maintenance_mode_timeout = 0; @@ -5018,8 +5007,8 @@ static bool ipmi_timeout_handler(struct for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) check_msg_timeout(intf, &intf->seq_table[i], &timeouts, timeout_period, i, - &flags, &need_timer); - spin_unlock_irqrestore(&intf->seq_lock, flags); + &need_timer); + mutex_unlock(&intf->seq_lock);
list_for_each_entry_safe(msg, msg2, &timeouts, link) deliver_err_response(intf, msg, IPMI_TIMEOUT_COMPLETION_CODE);