[PATCH OLK-6.6 0/3] Fix some bugs.
Fix some bugs. Jinjie Ruan (3): xcall2.0: Fix module unload issue by fixing refcount xcall2.0: Fix the semantic changes and concurrency issues in xcall_enable xcall2.0: Fix stack-out-of-bounds bug in xcall_write() arch/arm64/kernel/xcall/core.c | 8 ++++++++ fs/proc/proc_xcall.c | 25 ++++++++++++++++++++----- include/linux/xcall.h | 2 ++ kernel/fork.c | 3 ++- 4 files changed, 32 insertions(+), 6 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 split clear_xcall_area() into free_xcall_area() and clear_xcall_area(), call put_xcall() in mmput() and free the mm->xcall in mmdrop(). Fixes: b05676644e95 ("xcall2.0: Add xcall_area") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- arch/arm64/kernel/xcall/core.c | 8 ++++++++ include/linux/xcall.h | 2 ++ kernel/fork.c | 3 ++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kernel/xcall/core.c b/arch/arm64/kernel/xcall/core.c index a88c4ed6e575..f3a381c9106a 100644 --- a/arch/arm64/kernel/xcall/core.c +++ b/arch/arm64/kernel/xcall/core.c @@ -279,6 +279,14 @@ void clear_xcall_area(struct mm_struct *mm) if (area->xcall) put_xcall(area->xcall); +} + +void free_xcall_area(struct mm_struct *mm) +{ + struct xcall_area *area = mm_xcall_area(mm); + + if (!area) + return; kfree(area); mm->xcall = NULL; diff --git a/include/linux/xcall.h b/include/linux/xcall.h index 510aebe4e7c0..15da16728278 100644 --- a/include/linux/xcall.h +++ b/include/linux/xcall.h @@ -33,6 +33,7 @@ extern int xcall_prog_register(struct xcall_prog *prog); extern void xcall_prog_unregister(struct xcall_prog *prog); extern void mm_init_xcall_area(struct mm_struct *mm, struct task_struct *p); extern void clear_xcall_area(struct mm_struct *mm); +extern void free_xcall_area(struct mm_struct *mm); extern int xcall_mmap(struct vm_area_struct *vma, struct mm_struct *mm); #else /* !CONFIG_DYNAMIC_XCALL */ static inline int xcall_prog_register(struct xcall_prog *prog) @@ -42,6 +43,7 @@ static inline int xcall_prog_register(struct xcall_prog *prog) static inline void xcall_prog_unregister(struct xcall_prog *prog) {} static inline void mm_init_xcall_area(struct mm_struct *mm, struct task_struct *p) {} static inline void clear_xcall_area(struct mm_struct *mm) {} +extern void free_xcall_area(struct mm_struct *mm) {} static inline int xcall_mmap(struct vm_area_struct *vma, struct mm_struct *mm) { return 0; diff --git a/kernel/fork.c b/kernel/fork.c index f0271e915b0e..4c820dfbfd75 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -975,7 +975,7 @@ void __mmdrop(struct mm_struct *mm) mm_free_pgd(mm); destroy_context(mm); mmu_notifier_subscriptions_destroy(mm); - clear_xcall_area(mm); + free_xcall_area(mm); check_mm(mm); put_user_ns(mm->user_ns); mm_pasid_drop(mm); @@ -1429,6 +1429,7 @@ static inline void __mmput(struct mm_struct *mm) { VM_BUG_ON(atomic_read(&mm->mm_users)); + clear_xcall_area(mm); uprobe_clear_state(mm); exit_aio(mm); ksm_exit(mm); -- 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. Moreover, concurrent modifications xcall enable to the same system-call number of the same process have also changed its semantics, potentially introducing race conditions, this is fixed by protecting the critical section with a spin_lock. 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 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fs/proc/proc_xcall.c b/fs/proc/proc_xcall.c index 8f7375235803..b83dcc6eaca5 100644 --- a/fs/proc/proc_xcall.c +++ b/fs/proc/proc_xcall.c @@ -61,11 +61,13 @@ static int xcall_open(struct inode *inode, struct file *filp) return single_open(filp, xcall_show, inode); } +static DEFINE_SPINLOCK(xcall_enable_write_lock); static ssize_t xcall_write(struct file *file, const char __user *ubuf, size_t count, loff_t *offset) { unsigned int sc_no = __NR_syscalls; struct task_struct *p; + int is_clear = 0; char buf[5]; int ret = 0; @@ -82,7 +84,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,7 +95,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] != '!'); + spin_lock(&xcall_enable_write_lock); + 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; + spin_unlock(&xcall_enable_write_lock); ret = 0; out: -- 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 reserver 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 b83dcc6eaca5..7549cd463f5b 100644 --- a/fs/proc/proc_xcall.c +++ b/fs/proc/proc_xcall.c @@ -62,13 +62,15 @@ static int xcall_open(struct inode *inode, struct file *filp) } static DEFINE_SPINLOCK(xcall_enable_write_lock); +#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)) @@ -78,8 +80,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/19067 邮件列表地址：https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/SDG... 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/19067 Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/SDG...
participants (2)
-
Jinjie Ruan -
patchwork bot