summaryrefslogtreecommitdiffstats
path: root/drivers/char/ipmi/ipmi_msghandler.c
Commit message (Collapse)AuthorAgeFilesLines
* ipmi: Remove usage of the deprecated ida_simple_xx() APIChristophe JAILLET2023-12-191-2/+2
| | | | | | | | | | | ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). This is less verbose. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Message-Id: <b1a7a75263400742e5fda6bd7ba426772dc8ef11.1702961986.git.christophe.jaillet@wanadoo.fr> Signed-off-by: Corey Minyard <minyard@acm.org>
* ipmi: refactor deprecated strncpyJustin Stitt2023-09-131-8/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | `strncpy` is deprecated for use on NUL-terminated destination strings [1]. In this case, strncpy is being used specifically for its NUL-padding behavior (and has been commented as such). Moreover, the destination string is not required to be NUL-terminated [2]. We can use a more robust and less ambiguous interface in `memcpy_and_pad` which makes the code more readable and even eliminates the need for that comment. Let's also use `strnlen` instead of `strlen()` with an upper-bounds check as this is intrinsically a part of `strnlen`. Also included in this patch is a simple 1:1 change of `strncpy` to `strscpy` for ipmi_ssif.c. If NUL-padding is wanted here as well then we should opt again for `strscpy_pad`. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://lore.kernel.org/all/ZQEADYBl0uZ1nX60@mail.minyard.net/ [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com> Message-Id: <20230913-strncpy-drivers-char-ipmi-ipmi-v2-1-e3bc0f6e599f@google.com> Signed-off-by: Corey Minyard <minyard@acm.org>
* ipmi: fix use after free in _ipmi_destroy_user()Dan Carpenter2022-11-151-1/+3
| | | | | | | | | | | The intf_free() function frees the "intf" pointer so we cannot dereference it again on the next line. Fixes: cbb79863fc31 ("ipmi: Don't allow device module unload when in use") Signed-off-by: Dan Carpenter <error27@gmail.com> Message-Id: <Y3M8xa1drZv4CToE@kili> Cc: <stable@vger.kernel.org> # 5.5+ Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Fix some kernel-doc warningsBo Liu2022-10-251-1/+1
| | | | | | | | | The current code provokes some kernel-doc warnings: drivers/char/ipmi/ipmi_msghandler.c:618: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst Signed-off-by: Bo Liu <liubo03@inspur.com> Message-Id: <20221025060436.4372-1-liubo03@inspur.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: fix memleak when unload ipmi driverZhang Yuchen2022-10-171-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | After the IPMI disconnect problem, the memory kept rising and we tried to unload the driver to free the memory. However, only part of the free memory is recovered after the driver is uninstalled. Using ebpf to hook free functions, we find that neither ipmi_user nor ipmi_smi_msg is free, only ipmi_recv_msg is free. We find that the deliver_smi_err_response call in clean_smi_msgs does the destroy processing on each message from the xmit_msg queue without checking the return value and free ipmi_smi_msg. deliver_smi_err_response is called only at this location. Adding the free handling has no effect. To verify, try using ebpf to trace the free function. $ bpftrace -e 'kretprobe:ipmi_alloc_recv_msg {printf("alloc rcv %p\n",retval);} kprobe:free_recv_msg {printf("free recv %p\n", arg0)} kretprobe:ipmi_alloc_smi_msg {printf("alloc smi %p\n", retval);} kprobe:free_smi_msg {printf("free smi %p\n",arg0)}' Signed-off-by: Zhang Yuchen <zhangyuchen.lcr@bytedance.com> Message-Id: <20221007092617.87597-4-zhangyuchen.lcr@bytedance.com> [Fixed the comment above handle_one_recv_msg().] Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Remove unused struct watcher_entryYuan Can2022-09-281-6/+0
| | | | | | | | | After commit e86ee2d44b44("ipmi: Rework locking and shutdown for hot remove"), no one use struct watcher_entry, so remove it. Signed-off-by: Yuan Can <yuancan@huawei.com> Message-Id: <20220927133814.98929-1-yuancan@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Fix comment typoJason Wang2022-07-181-1/+1
| | | | | | | | The double `the' is duplicated in line 4360, remove one. Signed-off-by: Jason Wang <wangborong@cdjrlc.com> Message-Id: <20220715054156.6342-1-wangborong@cdjrlc.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: remove unnecessary type castingsYu Zhe2022-05-121-3/+3
| | | | | | | | remove unnecessary void* type castings. Signed-off-by: Yu Zhe <yuzhe@nfschina.com> Message-Id: <20220421150941.7659-1-yuzhe@nfschina.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Make two logs uniqueCorey Minyard2022-05-121-2/+2
| | | | | | | There were two identical logs in two different places, so you couldn't tell which one was being logged. Make them unique. Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Convert pr_debug() to dev_dbg()Corey Minyard2022-05-121-4/+7
| | | | | | A device is available at all debug points, use the right interface. Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Fix pr_fmt to avoid compilation issuesCorey Minyard2022-05-121-2/+2
| | | | | | | | The was it was wouldn't work in some situations, simplify it. What was there was unnecessary complexity. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Add a sysfs count of total outstanding messages for an interfaceCorey Minyard2022-05-121-0/+29
| | | | | | | | | | | | | Go through each user and add its message count to a total and print the total. It would be nice to have a per-user file, but there's no user sysfs entity at this point to hang it off of. Probably not worth the effort. Based on work by Chen Guanqiao <chen.chenchacha@foxmail.com> Cc: Chen Guanqiao <chen.chenchacha@foxmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Add a sysfs interface to view the number of usersCorey Minyard2022-05-121-0/+21
| | | | | | | | | A count of users is kept for each interface, allow it to be viewed. Based on work by Chen Guanqiao <chen.chenchacha@foxmail.com> Cc: Chen Guanqiao <chen.chenchacha@foxmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Limit the number of message a user may have outstandingCorey Minyard2022-05-121-0/+21
| | | | | | | | | This way a rogue application can't use up a bunch of memory. Based on work by Chen Guanqiao <chen.chenchacha@foxmail.com> Cc: Chen Guanqiao <chen.chenchacha@foxmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Add a limit on the number of users that may use IPMICorey Minyard2022-05-121-0/+15
| | | | | | | | | | Each user uses memory, we need limits to avoid a rogue program from running the system out of memory. Based on work by Chen Guanqiao <chen.chenchacha@foxmail.com> Cc: Chen Guanqiao <chen.chenchacha@foxmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi:ipmi_ipmb: Fix null-ptr-deref in ipmi_unregister_smi()Corey Minyard2022-04-291-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | KASAN report null-ptr-deref as follows: KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 RIP: 0010:ipmi_unregister_smi+0x7d/0xd50 drivers/char/ipmi/ipmi_msghandler.c:3680 Call Trace: ipmi_ipmb_remove+0x138/0x1a0 drivers/char/ipmi/ipmi_ipmb.c:443 ipmi_ipmb_probe+0x409/0xda1 drivers/char/ipmi/ipmi_ipmb.c:548 i2c_device_probe+0x959/0xac0 drivers/i2c/i2c-core-base.c:563 really_probe+0x3f3/0xa70 drivers/base/dd.c:541 In ipmi_ipmb_probe(), 'iidev->intf' is not set before ipmi_register_smi() success. And in the error handling case, ipmi_ipmb_remove() is called to release resources, ipmi_unregister_smi() is called without check 'iidev->intf', this will cause KASAN null-ptr-deref issue. General kernel style is to allow NULL to be passed into unregister calls, so fix it that way. This allows a NULL check to be removed in other code. Fixes: 57c9e3c9a374 ("ipmi:ipmi_ipmb: Unregister the SMI on remove") Reported-by: Hulk Robot <hulkci@huawei.com> Cc: stable@vger.kernel.org # v5.17+ Cc: Wei Yongjun <weiyongjun1@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: When handling send message responses, don't process the messageCorey Minyard2022-04-291-0/+2
| | | | | | | | | | | | | | | A chunk was dropped when the code handling send messages was rewritten. Those messages shouldn't be processed normally, they are just an indication that the message was successfully sent and the timers should be started for the real response that should be coming later. Add back in the missing chunk to just discard the message and go on. Fixes: 059747c245f0 ("ipmi: Add support for IPMB direct messages") Reported-by: Joe Wiese <jwiese@rackspace.com> Cc: stable@vger.kernel.org # v5.16+ Signed-off-by: Corey Minyard <cminyard@mvista.com> Tested-by: Joe Wiese <jwiese@rackspace.com>
* ipmi: Fix UAF when uninstall ipmi_si and ipmi_msghandler moduleWu Bo2021-12-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Hi, When testing install and uninstall of ipmi_si.ko and ipmi_msghandler.ko, the system crashed. The log as follows: [ 141.087026] BUG: unable to handle kernel paging request at ffffffffc09b3a5a [ 141.087241] PGD 8fe4c0d067 P4D 8fe4c0d067 PUD 8fe4c0f067 PMD 103ad89067 PTE 0 [ 141.087464] Oops: 0010 [#1] SMP NOPTI [ 141.087580] CPU: 67 PID: 668 Comm: kworker/67:1 Kdump: loaded Not tainted 4.18.0.x86_64 #47 [ 141.088009] Workqueue: events 0xffffffffc09b3a40 [ 141.088009] RIP: 0010:0xffffffffc09b3a5a [ 141.088009] Code: Bad RIP value. [ 141.088009] RSP: 0018:ffffb9094e2c3e88 EFLAGS: 00010246 [ 141.088009] RAX: 0000000000000000 RBX: ffff9abfdb1f04a0 RCX: 0000000000000000 [ 141.088009] RDX: 0000000000000000 RSI: 0000000000000246 RDI: 0000000000000246 [ 141.088009] RBP: 0000000000000000 R08: ffff9abfffee3cb8 R09: 00000000000002e1 [ 141.088009] R10: ffffb9094cb73d90 R11: 00000000000f4240 R12: ffff9abfffee8700 [ 141.088009] R13: 0000000000000000 R14: ffff9abfdb1f04a0 R15: ffff9abfdb1f04a8 [ 141.088009] FS: 0000000000000000(0000) GS:ffff9abfffec0000(0000) knlGS:0000000000000000 [ 141.088009] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.088009] CR2: ffffffffc09b3a30 CR3: 0000008fe4c0a001 CR4: 00000000007606e0 [ 141.088009] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.088009] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 141.088009] PKRU: 55555554 [ 141.088009] Call Trace: [ 141.088009] ? process_one_work+0x195/0x390 [ 141.088009] ? worker_thread+0x30/0x390 [ 141.088009] ? process_one_work+0x390/0x390 [ 141.088009] ? kthread+0x10d/0x130 [ 141.088009] ? kthread_flush_work_fn+0x10/0x10 [ 141.088009] ? ret_from_fork+0x35/0x40] BUG: unable to handle kernel paging request at ffffffffc0b28a5a [ 200.223240] PGD 97fe00d067 P4D 97fe00d067 PUD 97fe00f067 PMD a580cbf067 PTE 0 [ 200.223464] Oops: 0010 [#1] SMP NOPTI [ 200.223579] CPU: 63 PID: 664 Comm: kworker/63:1 Kdump: loaded Not tainted 4.18.0.x86_64 #46 [ 200.224008] Workqueue: events 0xffffffffc0b28a40 [ 200.224008] RIP: 0010:0xffffffffc0b28a5a [ 200.224008] Code: Bad RIP value. [ 200.224008] RSP: 0018:ffffbf3c8e2a3e88 EFLAGS: 00010246 [ 200.224008] RAX: 0000000000000000 RBX: ffffa0799ad6bca0 RCX: 0000000000000000 [ 200.224008] RDX: 0000000000000000 RSI: 0000000000000246 RDI: 0000000000000246 [ 200.224008] RBP: 0000000000000000 R08: ffff9fe43fde3cb8 R09: 00000000000000d5 [ 200.224008] R10: ffffbf3c8cb53d90 R11: 00000000000f4240 R12: ffff9fe43fde8700 [ 200.224008] R13: 0000000000000000 R14: ffffa0799ad6bca0 R15: ffffa0799ad6bca8 [ 200.224008] FS: 0000000000000000(0000) GS:ffff9fe43fdc0000(0000) knlGS:0000000000000000 [ 200.224008] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 200.224008] CR2: ffffffffc0b28a30 CR3: 00000097fe00a002 CR4: 00000000007606e0 [ 200.224008] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 200.224008] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 200.224008] PKRU: 55555554 [ 200.224008] Call Trace: [ 200.224008] ? process_one_work+0x195/0x390 [ 200.224008] ? worker_thread+0x30/0x390 [ 200.224008] ? process_one_work+0x390/0x390 [ 200.224008] ? kthread+0x10d/0x130 [ 200.224008] ? kthread_flush_work_fn+0x10/0x10 [ 200.224008] ? ret_from_fork+0x35/0x40 [ 200.224008] kernel fault(0x1) notification starting on CPU 63 [ 200.224008] kernel fault(0x1) notification finished on CPU 63 [ 200.224008] CR2: ffffffffc0b28a5a [ 200.224008] ---[ end trace c82a412d93f57412 ]--- The reason is as follows: T1: rmmod ipmi_si. ->ipmi_unregister_smi() -> ipmi_bmc_unregister() -> __ipmi_bmc_unregister() -> kref_put(&bmc->usecount, cleanup_bmc_device); -> schedule_work(&bmc->remove_work); T2: rmmod ipmi_msghandler. ipmi_msghander module uninstalled, and the module space will be freed. T3: bmc->remove_work doing cleanup the bmc resource. -> cleanup_bmc_work() -> platform_device_unregister(&bmc->pdev); -> platform_device_del(pdev); -> device_del(&pdev->dev); -> kobject_uevent(&dev->kobj, KOBJ_REMOVE); -> kobject_uevent_env() -> dev_uevent() -> if (dev->type && dev->type->name) 'dev->type'(bmc_device_type) pointer space has freed when uninstall ipmi_msghander module, 'dev->type->name' cause the system crash. drivers/char/ipmi/ipmi_msghandler.c: 2820 static const struct device_type bmc_device_type = { 2821 .groups = bmc_dev_attr_groups, 2822 }; Steps to reproduce: Add a time delay in cleanup_bmc_work() function, and uninstall ipmi_si and ipmi_msghandler module. 2910 static void cleanup_bmc_work(struct work_struct *work) 2911 { 2912 struct bmc_device *bmc = container_of(work, struct bmc_device, 2913 remove_work); 2914 int id = bmc->pdev.id; /* Unregister overwrites id */ 2915 2916 msleep(3000); <--- 2917 platform_device_unregister(&bmc->pdev); 2918 ida_simple_remove(&ipmi_bmc_ida, id); 2919 } Use 'remove_work_wq' instead of 'system_wq' to solve this issues. Fixes: b2cfd8ab4add ("ipmi: Rework device id and guid handling to catch changing BMCs") Signed-off-by: Wu Bo <wubo40@huawei.com> Message-Id: <1640070034-56671-1-git-send-email-wubo40@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: fix initialization when workqueue allocation failsThadeu Lima de Souza Cascardo2021-12-171-6/+9
| | | | | | | | | | | | | | | | If the workqueue allocation fails, the driver is marked as not initialized, and timer and panic_notifier will be left registered. Instead of removing those when workqueue allocation fails, do the workqueue initialization before doing it, and cleanup srcu_struct if it fails. Fixes: 1d49eb91e86e ("ipmi: Move remove_work to dedicated workqueue") Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Cc: Corey Minyard <cminyard@mvista.com> Cc: Ioanna Alifieraki <ioanna-maria.alifieraki@canonical.com> Cc: stable@vger.kernel.org Message-Id: <20211217154410.1228673-2-cascardo@canonical.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: bail out if init_srcu_struct failsThadeu Lima de Souza Cascardo2021-12-171-1/+3
| | | | | | | | | | | | | In case, init_srcu_struct fails (because of memory allocation failure), we might proceed with the driver initialization despite srcu_struct not being entirely initialized. Fixes: 913a89f009d9 ("ipmi: Don't initialize anything in the core until something uses it") Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Cc: Corey Minyard <cminyard@mvista.com> Cc: stable@vger.kernel.org Message-Id: <20211217154410.1228673-1-cascardo@canonical.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi:ipmb: Fix unknown command responseCorey Minyard2021-11-251-3/+5
| | | | | | | More missed changes, the response back to another system sending a command that had no user to handle it wasn't formatted properly. Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: fix IPMI_SMI_MSG_TYPE_IPMB_DIRECT response length checkingCorey Minyard2021-11-251-4/+15
| | | | | | | | | | | | | | A couple of issues: The tested data sizes are wrong; during the design that changed and this got missed. The formatting of the reponse couldn't use the normal one, it has to be an IPMB formatted response. Reported-by: Jakub Kicinski <kuba@kernel.org> Fixes: 059747c245f0 ("ipmi: Add support for IPMB direct messages") Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: fix oob access due to uninit smi_msg typeJakub Kicinski2021-11-251-0/+1
| | | | | | | | | | | | We're hitting OOB accesses in handle_ipmb_direct_rcv_rsp() (memcpy of size -1) after user space generates a message. Looks like the message is incorrectly assumed to be of the new IPMB type, because type is never set and message is allocated with kmalloc() not kzalloc(). Fixes: 059747c245f0 ("ipmi: Add support for IPMB direct messages") Signed-off-by: Jakub Kicinski <kuba@kernel.org> Message-Id: <20211124210323.1950976-1-kuba@kernel.org> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: msghandler: Make symbol 'remove_work_wq' staticWei Yongjun2021-11-231-1/+1
| | | | | | | | | | | | | | | | The sparse tool complains as follows: drivers/char/ipmi/ipmi_msghandler.c:194:25: warning: symbol 'remove_work_wq' was not declared. Should it be static? This symbol is not used outside of ipmi_msghandler.c, so marks it static. Fixes: 1d49eb91e86e ("ipmi: Move remove_work to dedicated workqueue") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Message-Id: <20211123083618.2366808-1-weiyongjun1@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Move remove_work to dedicated workqueueIoanna Alifieraki2021-11-151-1/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | Currently when removing an ipmi_user the removal is deferred as a work on the system's workqueue. Although this guarantees the free operation will occur in non atomic context, it can race with the ipmi_msghandler module removal (see [1]) . In case a remove_user work is scheduled for removal and shortly after ipmi_msghandler module is removed we can end up in a situation where the module is removed fist and when the work is executed the system crashes with : BUG: unable to handle page fault for address: ffffffffc05c3450 PF: supervisor instruction fetch in kernel mode PF: error_code(0x0010) - not-present page because the pages of the module are gone. In cleanup_ipmi() there is no easy way to detect if there are any pending works to flush them before removing the module. This patch creates a separate workqueue and schedules the remove_work works on it. When removing the module the workqueue is drained when destroyed to avoid the race. [1] https://bugs.launchpad.net/bugs/1950666 Cc: stable@vger.kernel.org # 5.1 Fixes: 3b9a907223d7 (ipmi: fix sleep-in-atomic in free_user at cleanup SRCU user->release_barrier) Signed-off-by: Ioanna Alifieraki <ioanna-maria.alifieraki@canonical.com> Message-Id: <20211115131645.25116-1-ioanna-maria.alifieraki@canonical.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* char: ipmi: replace snprintf in show functions with sysfs_emitYe Guojin2021-10-211-10/+10
| | | | | | | | | | | | | | coccicheck complains about the use of snprintf() in sysfs show functions: WARNING use scnprintf or sprintf Use sysfs_emit instead of scnprintf, snprintf or sprintf makes more sense. Reported-by: Zeal Robot <zealci@zte.com.cn> Signed-off-by: Ye Guojin <ye.guojin@zte.com.cn> Message-Id: <20211021110608.1060260-1-ye.guojin@zte.com.cn> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Add support for IPMB direct messagesCorey Minyard2021-10-051-33/+255
| | | | | | | | | | | | | | | | | | | | | An application has come up that has a device sitting right on the IPMB that would like to communicate with the BMC on the IPMB using normal IPMI commands. Sending these commands and handling the responses is easy enough, no modifications are needed to the IPMI infrastructure. But if this is an application that also needs to receive IPMB commands and respond, some way is needed to handle these incoming commands and send the responses. Currently, the IPMI message handler only sends commands to the interface and only receives responses from interface. This change extends the interface to receive commands/responses and send commands/responses. These are formatted differently in support of receiving/sending IPMB messages directly. Signed-off-by: Corey Minyard <minyard@acm.org> Tested-by: Andrew Manley <andrew.manley@sealingtech.com> Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
* ipmi: Export ipmb_checksum()Corey Minyard2021-10-051-1/+2
| | | | | | | | It will be needed by the upcoming ipmb direct addressing. Signed-off-by: Corey Minyard <minyard@acm.org> Tested-by: Andrew Manley <andrew.manley@sealingtech.com> Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
* ipmi: Check error code before processing BMC responseCorey Minyard2021-10-051-1/+8
| | | | | | In case an error did occur, print out useful information. Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Disable some operations during a panicCorey Minyard2021-10-051-3/+7
| | | | | | | Don't do kfree or other risky things when oops_in_progress is set. It's easy enough to avoid doing them Signed-off-by: Corey Minyard <cminyard@mvista.com>
* kernel.h: split out panic and oops helpersAndy Shevchenko2021-07-011-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | kernel.h is being used as a dump for all kinds of stuff for a long time. Here is the attempt to start cleaning it up by splitting out panic and oops helpers. There are several purposes of doing this: - dropping dependency in bug.h - dropping a loop by moving out panic_notifier.h - unload kernel.h from something which has its own domain At the same time convert users tree-wide to use new headers, although for the time being include new header back to kernel.h to avoid twisted indirected includes for existing users. [akpm@linux-foundation.org: thread_info.h needs limits.h] [andriy.shevchenko@linux.intel.com: ia64 fix] Link: https://lkml.kernel.org/r/20210520130557.55277-1-andriy.shevchenko@linux.intel.com Link: https://lkml.kernel.org/r/20210511074137.33666-1-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Co-developed-by: Andrew Morton <akpm@linux-foundation.org> Acked-by: Mike Rapoport <rppt@linux.ibm.com> Acked-by: Corey Minyard <cminyard@mvista.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Kees Cook <keescook@chromium.org> Acked-by: Wei Liu <wei.liu@kernel.org> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Acked-by: Sebastian Reichel <sre@kernel.org> Acked-by: Luis Chamberlain <mcgrof@kernel.org> Acked-by: Stephen Boyd <sboyd@kernel.org> Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Acked-by: Helge Deller <deller@gmx.de> # parisc Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* ipmi_si: Join string literals backAndy Shevchenko2021-04-021-2/+1
| | | | | | | | | | | For easy grepping on debug purposes join string literals back in the messages. No functional change. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20210402174334.13466-11-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi_si: Introduce ipmi_panic_event_str[] arrayAndy Shevchenko2021-04-021-32/+19
| | | | | | | | | | Instead of repeating twice the constant literals, introduce ipmi_panic_event_str[] array. It allows to simplify the code with help of match_string() API. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20210402174334.13466-6-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Refine retry conditions for getting device idTerry Duncan2021-03-101-4/+2
| | | | | | | | | | | Rarely but still failures are observed while getting BMC device ID so this commit changes the condition to retry to get device id when cc is not IPMI_CC_NO_ERROR. Signed-off-by: Terry Duncan <terry.s.duncan@intel.com> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Message-Id: <20210225045027.9344-1-jae.hyun.yoo@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: msghandler: Suppress suspicious RCU usage warningQinglang Miao2020-11-191-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | while running ipmi, ipmi_smi_watcher_register() caused a suspicious RCU usage warning. ----- ============================= WARNING: suspicious RCU usage 5.10.0-rc3+ #1 Not tainted ----------------------------- drivers/char/ipmi/ipmi_msghandler.c:750 RCU-list traversed in non-reader section!! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 2 locks held by syz-executor.0/4254: stack backtrace: CPU: 0 PID: 4254 Comm: syz-executor.0 Not tainted 5.10.0-rc3+ #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/ 01/2014 Call Trace: dump_stack+0x19d/0x200 ipmi_smi_watcher_register+0x2d3/0x340 [ipmi_msghandler] acpi_ipmi_init+0xb1/0x1000 [acpi_ipmi] do_one_initcall+0x149/0x7e0 do_init_module+0x1ef/0x700 load_module+0x3467/0x4140 __do_sys_finit_module+0x10d/0x1a0 do_syscall_64+0x34/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x468ded ----- It is safe because smi_watchers_mutex is locked and srcu_read_lock has been used, so simply pass lockdep_is_held() to the list_for_each_entry_rcu() to suppress this warning. Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> Message-Id: <20201119070839.381-1-miaoqinglang@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: msghandler: Fix a signedness bugDan Carpenter2020-09-181-1/+1
| | | | | | | | | | | | | | | The type for the completion codes should be unsigned char instead of char. If it is declared as a normal char then the conditions in __get_device_id() are impossible because the IPMI_DEVICE_IN_FW_UPDATE_ERR error codes are higher than 127. drivers/char/ipmi/ipmi_msghandler.c:2449 __get_device_id() warn: impossible condition '(bmc->cc == 209) => ((-128)-127 == 209)' Fixes: f8910ffa81b0 ("ipmi:msghandler: retry to get device id on an error") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Message-Id: <20200918142756.GB909725@mwanda> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: add retry in try_get_dev_id()Xianting Tian2020-09-161-2/+0
| | | | | | | | | | | | | Use a retry machanism to give the BMC more opportunities to correctly respond when we receive specific completion codes. This is similar to what is done in __get_device_id(). Signed-off-by: Xianting Tian <tian.xianting@h3c.com> Message-Id: <20200916062129.26129-1-tian.xianting@h3c.com> [Moved GET_DEVICE_ID_MAX_RETRY to include/linux/ipmi.h, reworded some text.] Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi:msghandler: retry to get device id on an errorXianting Tian2020-09-151-4/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We fail to get the BMCS's device id with low probability when loading the ipmi driver and it causes BMC device registration failed. When this issue occurs we got below kernel prints: [Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: IPMI message handler: device id demangle failed: -22 [Wed Sep 9 19:52:03 2020] IPMI BT: using default values [Wed Sep 9 19:52:03 2020] IPMI BT: req2rsp=5 secs retries=2 [Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: Unable to get the device id: -5 [Wed Sep 9 19:52:04 2020] ipmi_si IPI0001:00: Unable to register device: error -5 When this issue happens, we want to manually unload the driver and try to load it again, but it can't be unloaded by 'rmmod' as it is already 'in use'. We add a print in handle_one_recv_msg(), when this issue happens, the msg we received is "Recv: 1c 01 d5", which means the data_len is 1, data[0] is 0xd5 (completion code), which means "bmc cannot execute command. Command, or request parameter(s), not supported in present state". Debug code: static int handle_one_recv_msg(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { printk("Recv: %*ph\n", msg->rsp_size, msg->rsp); ... ... } Then in ipmi_demangle_device_id(), it returned '-EINVAL' as 'data_len < 7' and 'data[0] != 0'. We created this patch to retry the get device id when this error happens. We reproduced this issue again and the retry succeed on the first retry, we finally got the correct msg and then all is ok: Recv: 1c 01 00 01 81 05 84 02 af db 07 00 01 00 b9 00 10 00 So use a retry machanism in this patch to give bmc more opportunity to correctly response kernel when we received specific completion codes. Signed-off-by: Xianting Tian <tian.xianting@h3c.com> Message-Id: <20200915071817.4484-1-tian.xianting@h3c.com> [Cleaned up the verbage a bit in the header and prints.] Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Reset response handler when failing to send the commandMarkus Boehme2020-09-151-1/+3
| | | | | | | | | When failing to send a command we don't expect a response. Clear the `null_user_handler` like is done in the success path. Signed-off-by: Markus Boehme <markubo@amazon.com> Message-Id: <1599495937-10654-1-git-send-email-markubo@amazon.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: add a newline when printing parameter 'panic_op' by sysfsXiongfeng Wang2020-09-031-4/+4
| | | | | | | | | | | | When I cat ipmi_msghandler parameter 'panic_op' by sysfs, it displays as follows. It's better to add a newline for easy reading. root@(none):/# cat /sys/module/ipmi_msghandler/parameters/panic_op noneroot@(none):/# Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> Message-Id: <1599130873-2402-1-git-send-email-wangxiongfeng2@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* char: ipmi: convert tasklets to use new tasklet_setup() APIAllen Pais2020-08-181-7/+6
| | | | | | | | | | | | In preparation for unconditionally passing the struct tasklet_struct pointer to all tasklet callbacks, switch to using the new tasklet_setup() and from_tasklet() to pass the tasklet pointer explicitly. Signed-off-by: Romain Perier <romain.perier@gmail.com> Signed-off-by: Allen Pais <allen.lkml@gmail.com> Message-Id: <20200817091617.28119-3-allen.cryptic@gmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: remve duplicate code in __ipmi_bmc_register()Jing Xiangfeng2020-07-201-2/+0
| | | | | | | | | __ipmi_bmc_register() jumps to the label 'out_free_my_dev_name' in an error path. So we can remove duplicate code in the if (rv). Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com> Message-Id: <20200720080838.148737-1-jingxiangfeng@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Replace guid_copy() with import_guid() where it makes senseAndy Shevchenko2020-05-181-1/+1
| | | | | | | | | There is a specific API to treat raw data as GUID, i.e. import_guid(). Use it instead of guid_copy() with explicit casting. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20200422130348.38749-1-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: use vzalloc instead of kmalloc for user creationFeng Tang2020-05-181-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We met mulitple times of failure of staring bmc-watchdog, due to the runtime memory allocation failure of order 4. bmc-watchdog: page allocation failure: order:4, mode:0x40cc0(GFP_KERNEL|__GFP_COMP), nodemask=(null),cpuset=/,mems_allowed=0-1 CPU: 1 PID: 2571 Comm: bmc-watchdog Not tainted 5.5.0-00045-g7d6bb61d6188c #1 Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.00.01.0015.110720180833 11/07/2018 Call Trace: dump_stack+0x66/0x8b warn_alloc+0xfe/0x160 __alloc_pages_slowpath+0xd3e/0xd80 __alloc_pages_nodemask+0x2f0/0x340 kmalloc_order+0x18/0x70 kmalloc_order_trace+0x1d/0xb0 ipmi_create_user+0x55/0x2c0 [ipmi_msghandler] ipmi_open+0x72/0x110 [ipmi_devintf] chrdev_open+0xcb/0x1e0 do_dentry_open+0x1ce/0x380 path_openat+0x305/0x14f0 do_filp_open+0x9b/0x110 do_sys_open+0x1bd/0x250 do_syscall_64+0x5b/0x1f0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Using vzalloc/vfree for creating ipmi_user heals the problem Thanks to Stephen Rothwell for finding the vmalloc.h inclusion issue. Signed-off-by: Feng Tang <feng.tang@intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: fix hung processes in __get_guid()Wen Yang2020-04-031-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The wait_event() function is used to detect command completion. When send_guid_cmd() returns an error, smi_send() has not been called to send data. Therefore, wait_event() should not be used on the error path, otherwise it will cause the following warning: [ 1361.588808] systemd-udevd D 0 1501 1436 0x00000004 [ 1361.588813] ffff883f4b1298c0 0000000000000000 ffff883f4b188000 ffff887f7e3d9f40 [ 1361.677952] ffff887f64bd4280 ffffc90037297a68 ffffffff8173ca3b ffffc90000000010 [ 1361.767077] 00ffc90037297ad0 ffff887f7e3d9f40 0000000000000286 ffff883f4b188000 [ 1361.856199] Call Trace: [ 1361.885578] [<ffffffff8173ca3b>] ? __schedule+0x23b/0x780 [ 1361.951406] [<ffffffff8173cfb6>] schedule+0x36/0x80 [ 1362.010979] [<ffffffffa071f178>] get_guid+0x118/0x150 [ipmi_msghandler] [ 1362.091281] [<ffffffff810d5350>] ? prepare_to_wait_event+0x100/0x100 [ 1362.168533] [<ffffffffa071f755>] ipmi_register_smi+0x405/0x940 [ipmi_msghandler] [ 1362.258337] [<ffffffffa0230ae9>] try_smi_init+0x529/0x950 [ipmi_si] [ 1362.334521] [<ffffffffa022f350>] ? std_irq_setup+0xd0/0xd0 [ipmi_si] [ 1362.411701] [<ffffffffa0232bd2>] init_ipmi_si+0x492/0x9e0 [ipmi_si] [ 1362.487917] [<ffffffffa0232740>] ? ipmi_pci_probe+0x280/0x280 [ipmi_si] [ 1362.568219] [<ffffffff810021a0>] do_one_initcall+0x50/0x180 [ 1362.636109] [<ffffffff812231b2>] ? kmem_cache_alloc_trace+0x142/0x190 [ 1362.714330] [<ffffffff811b2ae1>] do_init_module+0x5f/0x200 [ 1362.781208] [<ffffffff81123ca8>] load_module+0x1898/0x1de0 [ 1362.848069] [<ffffffff811202e0>] ? __symbol_put+0x60/0x60 [ 1362.913886] [<ffffffff8130696b>] ? security_kernel_post_read_file+0x6b/0x80 [ 1362.998514] [<ffffffff81124465>] SYSC_finit_module+0xe5/0x120 [ 1363.068463] [<ffffffff81124465>] ? SYSC_finit_module+0xe5/0x120 [ 1363.140513] [<ffffffff811244be>] SyS_finit_module+0xe/0x10 [ 1363.207364] [<ffffffff81003c04>] do_syscall_64+0x74/0x180 Fixes: 50c812b2b951 ("[PATCH] ipmi: add full sysfs support") Signed-off-by: Wen Yang <wenyang@linux.alibaba.com> Cc: Corey Minyard <minyard@acm.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: openipmi-developer@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org # 2.6.17- Message-Id: <20200403090408.58745-1-wenyang@linux.alibaba.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* drivers: char: ipmi: ipmi_msghandler: Pass lockdep expression to RCU listsAmol Grover2020-03-121-4/+10
| | | | | | | | | | | | | | | | | | | | | | | intf->cmd_rcvrs is traversed with list_for_each_entry_rcu outside an RCU read-side critical section but under the protection of intf->cmd_rcvrs_mutex. ipmi_interfaces is traversed using list_for_each_entry_rcu outside an RCU read-side critical section but under the protection of ipmi_interfaces_mutex. Hence, add the corresponding lockdep expression to the list traversal primitive to silence false-positive lockdep warnings, and harden RCU lists. Add macro for the corresponding lockdep expression to make the code clean and concise. Signed-off-by: Amol Grover <frextrite@gmail.com> Message-Id: <20200117132521.31020-1-frextrite@gmail.com> Acked-by: Paul E. McKenney <paulmck@kernel.org> Tested-by: John Garry <john.garry@huawei.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Fix memory leak in __ipmi_bmc_registerNavid Emamdoost2019-10-221-1/+4
| | | | | | | | | | In the impelementation of __ipmi_bmc_register() the allocated memory for bmc should be released in case ida_simple_get() fails. Fixes: 68e7e50f195f ("ipmi: Don't use BMC product/dev ids in the BMC name") Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> Message-Id: <20191021200649.1511-1-navid.emamdoost@gmail.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: use %*ph to print small bufferAndy Shevchenko2019-10-221-23/+4
| | | | | | | | | | | | | | | | Use %*ph format to print small buffer as hex string. The change is safe since the specifier can handle up to 64 bytes and taking into account the buffer size of 100 bytes on stack the function has never been used to dump more than 32 bytes. Note, this also avoids potential buffer overflow if the length of the input buffer is bigger. This completely eliminates ipmi_debug_msg() in favour of Dynamic Debug. Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20191011155036.36748-1-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com>
* ipmi: Don't allow device module unload when in useCorey Minyard2019-10-221-7/+16
| | | | | | | | | | | | | | | | | | | | | | If something has the IPMI driver open, don't allow the device module to be unloaded. Before it would unload and the user would get errors on use. This change is made on user request, and it makes it consistent with the I2C driver, which has the same behavior. It does change things a little bit with respect to kernel users. If the ACPI or IPMI watchdog (or any other kernel user) has created a user, then the device module cannot be unloaded. Before it could be unloaded, This does not affect hot-plug. If the device goes away (it's on something removable that is removed or is hot-removed via sysfs) then it still behaves as it did before. Reported-by: tony camuso <tcamuso@redhat.com> Signed-off-by: Corey Minyard <cminyard@mvista.com> Tested-by: tony camuso <tcamuso@redhat.com>
* ipmi: move message error checking to avoid deadlockTony Camuso2019-08-221-57/+57
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | V1->V2: in handle_one_rcv_msg, if data_size > 2, set requeue to zero and goto out instead of calling ipmi_free_msg. Kosuke Tatsukawa <tatsu@ab.jp.nec.com> In the source stack trace below, function set_need_watch tries to take out the same si_lock that was taken earlier by ipmi_thread. ipmi_thread() [drivers/char/ipmi/ipmi_si_intf.c:995] smi_event_handler() [drivers/char/ipmi/ipmi_si_intf.c:765] handle_transaction_done() [drivers/char/ipmi/ipmi_si_intf.c:555] deliver_recv_msg() [drivers/char/ipmi/ipmi_si_intf.c:283] ipmi_smi_msg_received() [drivers/char/ipmi/ipmi_msghandler.c:4503] intf_err_seq() [drivers/char/ipmi/ipmi_msghandler.c:1149] smi_remove_watch() [drivers/char/ipmi/ipmi_msghandler.c:999] set_need_watch() [drivers/char/ipmi/ipmi_si_intf.c:1066] Upstream commit e1891cffd4c4896a899337a243273f0e23c028df adds code to ipmi_smi_msg_received() to call smi_remove_watch() via intf_err_seq() and this seems to be causing the deadlock. commit e1891cffd4c4896a899337a243273f0e23c028df Author: Corey Minyard <cminyard@mvista.com> Date: Wed Oct 24 15:17:04 2018 -0500 ipmi: Make the smi watcher be disabled immediately when not needed The fix is to put all messages in the queue and move the message checking code out of ipmi_smi_msg_received and into handle_one_recv_msg, which processes the message checking after ipmi_thread releases its locks. Additionally,Kosuke Tatsukawa <tatsu@ab.jp.nec.com> reported that handle_new_recv_msgs calls ipmi_free_msg when handle_one_rcv_msg returns zero, so that the call to ipmi_free_msg in handle_one_rcv_msg introduced another panic when "ipmitool sensor list" was run in a loop. He submitted this part of the patch. +free_msg: + requeue = 0; + goto out; Reported by: Osamu Samukawa <osa-samukawa@tg.jp.nec.com> Characterized by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com> Signed-off-by: Tony Camuso <tcamuso@redhat.com> Fixes: e1891cffd4c4 ("ipmi: Make the smi watcher be disabled immediately when not needed") Cc: stable@vger.kernel.org # 5.1 Signed-off-by: Corey Minyard <cminyard@mvista.com>
generated by cgit v1.2.3 (git 2.25.1) at 2025-01-01 15:14:54 +0000