From e14239d495d3568291d6566327f97e7c902afaef Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 14 Feb 2022 13:16:49 +0300 Subject: [PATCH] scst_event: Fix crash when copying an event from user space We copy struct scst_event with the payload from user space, but use event_entry_len as the length of this buffer. event_entry_len contains the length of struct scst_event_entry and the payload. struct scst_event is part of struct scst_event_entry and is therefore shorter in length. Thus, use the length of struct scst_event + payload when copying the event from user space. This patch should fix the following bug: usercopy: Kernel memory overwrite attempt detected to SLUB object 'kmalloc-512' (offset 232, size 296)! ------------[ cut here ]------------ kernel BUG at mm/usercopy.c:102! ... RIP: 0010:usercopy_abort+0x74/0x76 Call Trace: __check_heap_object+0xd3/0x100 __check_object_size+0xff/0x16b scst_event_get_event_from_user+0xbb/0x2e0 [scst] scst_event_ioctl+0x5f1/0xde0 [scst] do_vfs_ioctl+0xa4/0x680 ? syscall_trace_enter+0x1d3/0x2c0 ksys_ioctl+0x60/0x90 __x64_sys_ioctl+0x16/0x20 do_syscall_64+0x5b/0x1a0 entry_SYSCALL_64_after_hwframe+0x65/0xca Fixes: ffd85476 ("scst: Suppress a false positive Coverity memory corruption complaint") Fixes: https://github.com/SCST-project/scst/issues/25 --- scst/src/scst_event.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/scst/src/scst_event.c b/scst/src/scst_event.c index 724a0e712..ee4f50c66 100644 --- a/scst/src/scst_event.c +++ b/scst/src/scst_event.c @@ -613,9 +613,11 @@ out: static int scst_event_get_event_from_user(struct scst_event_user __user *arg, struct scst_event_entry **out_event_entry) { - int res, rc, event_entry_len; + int res, rc; + int event_entry_len, event_len; uint32_t payload_len; struct scst_event_entry *event_entry; + struct scst_event *event; TRACE_ENTRY(); @@ -646,9 +648,10 @@ static int scst_event_get_event_from_user(struct scst_event_user __user *arg, TRACE_MEM("Allocated event entry %p", event_entry); - rc = copy_from_user((u8 *)event_entry + - offsetof(typeof(*event_entry), event), arg, - event_entry_len); + event = &event_entry->event; + event_len = sizeof(*event) + payload_len; + + rc = copy_from_user((u8 *)event, arg, event_len); if (rc != 0) { PRINT_ERROR("Failed to copy %d user's bytes", rc); res = -EFAULT; @@ -656,16 +659,16 @@ static int scst_event_get_event_from_user(struct scst_event_user __user *arg, } /* payload_len has been recopied, so recheck it. */ - if (event_entry->event.payload_len != event_entry_len) { + if (event->payload_len != event_len) { PRINT_ERROR("Payload len changed while being read"); res = -EINVAL; goto out_free; } - event_entry->event.issuer_name[sizeof(event_entry->event.issuer_name)-1] = '\0'; + event->issuer_name[sizeof(event->issuer_name) - 1] = '\0'; TRACE_DBG("user event: event_code %d, issuer_name %s", - event_entry->event.event_code, event_entry->event.issuer_name); + event->event_code, event->issuer_name); *out_event_entry = event_entry;