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
This commit is contained in:
Gleb Chesnokov
2022-02-14 13:16:49 +03:00
parent 307835218c
commit e14239d495

View File

@@ -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;