[PATCH OLK-6.6 v2 0/3] Fix bugs
Fix bugs. Jinjie Ruan (3): xcall2.0: Fix module unload issue by fixing refcount xcall2.0: Fix the semantic changes in xcall_enable xcall2.0: Fix stack-out-of-bounds bug in xcall_write() fs/proc/proc_xcall.c | 26 +++++++++++++++++++------- kernel/fork.c | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) -- 2.34.1
hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/release-management/issues/ID5CMS -------------------------------- After the process exits and xcall is unregistered, kernel module cannot be unloaded properly. The reason is that mmput() just need to reduce the refcount of 'mm->mm_users' to 0 which indicates that no user-space processes are using this address space, but mmdrop() need to reduce the refcount of 'mm->mm_count' to zero, which indicates that all references to the memory descriptor have reached zero and the mm_struct can now be safely freed and this condition may not be satisfied after process exit. So put_xcall() in mmdrop() can not guarantee the refcount of xcall will be decremented immediately, so module_put() in put_xcall() may not called. Fix this problem by calling clear_xcall_area() in mmput() before mmdrop() and after exit_mmap() which do mmu_notifier_release() to release xcall kernel module resource. Fixes: b05676644e95 ("xcall2.0: Add xcall_area") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- kernel/fork.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/fork.c b/kernel/fork.c index f0271e915b0e..328bbf6a36d2 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -975,7 +975,6 @@ void __mmdrop(struct mm_struct *mm) mm_free_pgd(mm); destroy_context(mm); mmu_notifier_subscriptions_destroy(mm); - clear_xcall_area(mm); check_mm(mm); put_user_ns(mm->user_ns); mm_pasid_drop(mm); @@ -1434,6 +1433,7 @@ static inline void __mmput(struct mm_struct *mm) ksm_exit(mm); khugepaged_exit(mm); /* must run before exit_mmap */ exit_mmap(mm); + clear_xcall_area(mm); sp_mm_clean(mm); mm_put_huge_zero_page(mm); set_mm_exe_file(mm, NULL); -- 2.34.1
hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/release-management/issues/ID5CMS -------------------------------- Originally, xcall_enable could only be set to 1 when it was disabled, or cleared to 0 when it was enabled, now the semantic has changed and causing abnormalities in DT testing, therefore add a check before modifying it. Fixes: fd83d0fe16f1 ("xcall: Rework the early exception vector of XCALL and SYNC") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- fs/proc/proc_xcall.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/proc/proc_xcall.c b/fs/proc/proc_xcall.c index 8f7375235803..b57d84c281a3 100644 --- a/fs/proc/proc_xcall.c +++ b/fs/proc/proc_xcall.c @@ -66,6 +66,7 @@ static ssize_t xcall_write(struct file *file, const char __user *ubuf, { unsigned int sc_no = __NR_syscalls; struct task_struct *p; + int is_clear = 0; char buf[5]; int ret = 0; @@ -82,7 +83,8 @@ static ssize_t xcall_write(struct file *file, const char __user *ubuf, goto out; } - if (kstrtouint((buf + (int)(buf[0] == '!')), 10, &sc_no)) { + is_clear = (buf[0] == '!'); + if (kstrtouint((buf + is_clear), 10, &sc_no)) { ret = -EINVAL; goto out; } @@ -92,9 +94,12 @@ static ssize_t xcall_write(struct file *file, const char __user *ubuf, goto out; } - (TASK_XINFO(p))->xcall_enable[sc_no] = (int)(buf[0] != '!'); - ret = 0; - + if (!is_clear && !(TASK_XINFO(p))->xcall_enable[sc_no]) + (TASK_XINFO(p))->xcall_enable[sc_no] = 1; + else if (is_clear && (TASK_XINFO(p))->xcall_enable[sc_no]) + (TASK_XINFO(p))->xcall_enable[sc_no] = 0; + else + ret = -EINVAL; out: put_task_struct(p); -- 2.34.1
hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/release-management/issues/ID5CMS -------------------------------- The following command cause a stack-out-of-bounds KASAN bug. commit fd83d0fe16f1 ("xcall: Rework the early exception vector of XCALL and SYNC") delete the count limit for copy_from_user(). Due to copy_from_user() copy one extra character into the "buf", which overwrites the original trailing '\0' character in the "buf", causing kstrtouint() to access out of bounds. Fix it by limiting copy_from_user() to copy at most the buf's maximum size minus one to reserve space for a '\0'. echo 1234 > /proc/1/xcall or echo !123 > /proc/1/xcall xcall_write count: 5 ================================================================== BUG: KASAN: stack-out-of-bounds in _kstrtoull+0x120/0x150 Read of size 1 at addr ffff800085d67b85 by task sh/1209 CPU: 4 PID: 1209 Comm: sh Not tainted 6.6.0-27103-g5fd643bb2ba7-dirty #232 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x94/0xf0 show_stack+0x1c/0x30 dump_stack_lvl+0x44/0x58 print_report+0x164/0x5b8 kasan_report+0x80/0xc0 __asan_load1+0x58/0x60 _kstrtoull+0x120/0x150 kstrtoull+0x38/0x48 kstrtouint+0x70/0xe0 xcall_write+0x274/0x3e0 vfs_write+0x228/0x548 ksys_write+0xc8/0x170 __arm64_sys_write+0x48/0x60 invoke_syscall+0x64/0x148 el0_svc_common.constprop.0+0x7c/0x110 do_el0_svc+0x68/0xb8 el0_slow_syscall+0x3c/0x128 .slow_syscall+0x16c/0x170 The buggy address belongs to stack of task sh/1209 and is located at offset 69 in frame: xcall_write+0x0/0x3e0 This frame has 3 objects: [32, 36) 'sc_no' [48, 52) 'val' [64, 69) 'buf' The buggy address belongs to the virtual mapping at [ffff800085d60000, ffff800085d69000) created by: kernel_clone+0xf4/0x550 The buggy address belongs to the physical page: page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x105556 flags: 0x2000000000000000(node=0|zone=2) raw: 2000000000000000 0000000000000000 dead000000000122 0000000000000000 raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff800085d67a80: f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00 00 00 00 00 ffff800085d67b00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 04 f2 >ffff800085d67b80: 05 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 ^ ffff800085d67c00: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 f2 f2 f2 ffff800085d67c80: f2 f2 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00 ================================================================== Disabling lock debugging due to kernel taint Fixes: fd83d0fe16f1 ("xcall: Rework the early exception vector of XCALL and SYNC") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- fs/proc/proc_xcall.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/fs/proc/proc_xcall.c b/fs/proc/proc_xcall.c index b57d84c281a3..47f3d4f25393 100644 --- a/fs/proc/proc_xcall.c +++ b/fs/proc/proc_xcall.c @@ -61,13 +61,15 @@ static int xcall_open(struct inode *inode, struct file *filp) return single_open(filp, xcall_show, inode); } +#define MAX_SYSNO_DIGITS 3 +#define MAX_BUF_SIZE (1 + MAX_SYSNO_DIGITS + 1) // "!" + digits + '\0' static ssize_t xcall_write(struct file *file, const char __user *ubuf, size_t count, loff_t *offset) { unsigned int sc_no = __NR_syscalls; + char buf[MAX_BUF_SIZE]; struct task_struct *p; int is_clear = 0; - char buf[5]; int ret = 0; if (!static_key_enabled(&xcall_enable)) @@ -77,8 +79,13 @@ static ssize_t xcall_write(struct file *file, const char __user *ubuf, if (!p || !TASK_XINFO(p)) return -ESRCH; - memset(buf, '\0', 5); - if (!count || (count > 5) || copy_from_user(buf, ubuf, count)) { + memset(buf, '\0', MAX_BUF_SIZE); + if (!count || (count > MAX_BUF_SIZE)) { + ret = -EFAULT; + goto out; + } + + if (copy_from_user(buf, ubuf, count > MAX_BUF_SIZE - 1 ? MAX_BUF_SIZE - 1 : count)) { ret = -EFAULT; goto out; } -- 2.34.1
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/19087 邮件列表地址:https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/HKE... FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/19087 Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/HKE...
participants (2)
-
Jinjie Ruan -
patchwork bot