forked from Minki/linux
staging: vchiq: avoid mixing kernel and user pointers
As found earlier, there is a problem in the create_pagelist() function that takes a pointer argument that either points into vmalloc space or into user space, with the pointer value controlled by user space allowing a malicious user to trick the driver into accessing the kernel instead. Avoid this problem by adding another function argument and passing kernel pointers separately from user pointers. This makes it possible to rely on sparse to point out invalid conversions, and it prevents user space from faking a kernel pointer. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Link: https://lore.kernel.org/r/20200925114424.2647144-2-arnd@arndb.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
4184da4f31
commit
1c954540c0
@ -70,7 +70,7 @@ static irqreturn_t
|
||||
vchiq_doorbell_irq(int irq, void *dev_id);
|
||||
|
||||
static struct vchiq_pagelist_info *
|
||||
create_pagelist(char __user *buf, size_t count, unsigned short type);
|
||||
create_pagelist(char *buf, char __user *ubuf, size_t count, unsigned short type);
|
||||
|
||||
static void
|
||||
free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
|
||||
@ -216,12 +216,12 @@ remote_event_signal(struct remote_event *event)
|
||||
}
|
||||
|
||||
enum vchiq_status
|
||||
vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset, int size,
|
||||
int dir)
|
||||
vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset,
|
||||
void __user *uoffset, int size, int dir)
|
||||
{
|
||||
struct vchiq_pagelist_info *pagelistinfo;
|
||||
|
||||
pagelistinfo = create_pagelist((char __user *)offset, size,
|
||||
pagelistinfo = create_pagelist(offset, uoffset, size,
|
||||
(dir == VCHIQ_BULK_RECEIVE)
|
||||
? PAGELIST_READ
|
||||
: PAGELIST_WRITE);
|
||||
@ -304,7 +304,8 @@ cleanup_pagelistinfo(struct vchiq_pagelist_info *pagelistinfo)
|
||||
*/
|
||||
|
||||
static struct vchiq_pagelist_info *
|
||||
create_pagelist(char __user *buf, size_t count, unsigned short type)
|
||||
create_pagelist(char *buf, char __user *ubuf,
|
||||
size_t count, unsigned short type)
|
||||
{
|
||||
struct pagelist *pagelist;
|
||||
struct vchiq_pagelist_info *pagelistinfo;
|
||||
@ -320,7 +321,10 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
|
||||
if (count >= INT_MAX - PAGE_SIZE)
|
||||
return NULL;
|
||||
|
||||
offset = ((unsigned int)(unsigned long)buf & (PAGE_SIZE - 1));
|
||||
if (buf)
|
||||
offset = (uintptr_t)buf & (PAGE_SIZE - 1);
|
||||
else
|
||||
offset = (uintptr_t)ubuf & (PAGE_SIZE - 1);
|
||||
num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
|
||||
|
||||
if (num_pages > (SIZE_MAX - sizeof(struct pagelist) -
|
||||
@ -368,14 +372,14 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
|
||||
pagelistinfo->scatterlist = scatterlist;
|
||||
pagelistinfo->scatterlist_mapped = 0;
|
||||
|
||||
if (is_vmalloc_addr((void __force *)buf)) {
|
||||
if (buf) {
|
||||
unsigned long length = count;
|
||||
unsigned int off = offset;
|
||||
|
||||
for (actual_pages = 0; actual_pages < num_pages;
|
||||
actual_pages++) {
|
||||
struct page *pg =
|
||||
vmalloc_to_page((void __force *)(buf +
|
||||
vmalloc_to_page((buf +
|
||||
(actual_pages * PAGE_SIZE)));
|
||||
size_t bytes = PAGE_SIZE - off;
|
||||
|
||||
@ -393,7 +397,7 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
|
||||
/* do not try and release vmalloc pages */
|
||||
} else {
|
||||
actual_pages = pin_user_pages_fast(
|
||||
(unsigned long)buf & PAGE_MASK,
|
||||
(unsigned long)ubuf & PAGE_MASK,
|
||||
num_pages,
|
||||
type == PAGELIST_READ,
|
||||
pages);
|
||||
|
@ -360,8 +360,8 @@ vchiq_bulk_transmit(unsigned int handle, const void *data,
|
||||
case VCHIQ_BULK_MODE_NOCALLBACK:
|
||||
case VCHIQ_BULK_MODE_CALLBACK:
|
||||
status = vchiq_bulk_transfer(handle,
|
||||
(void *)data, size,
|
||||
userdata, mode,
|
||||
(void *)data, NULL,
|
||||
size, userdata, mode,
|
||||
VCHIQ_BULK_TRANSMIT);
|
||||
break;
|
||||
case VCHIQ_BULK_MODE_BLOCKING:
|
||||
@ -397,7 +397,8 @@ enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data,
|
||||
switch (mode) {
|
||||
case VCHIQ_BULK_MODE_NOCALLBACK:
|
||||
case VCHIQ_BULK_MODE_CALLBACK:
|
||||
status = vchiq_bulk_transfer(handle, data, size, userdata,
|
||||
status = vchiq_bulk_transfer(handle, data, NULL,
|
||||
size, userdata,
|
||||
mode, VCHIQ_BULK_RECEIVE);
|
||||
break;
|
||||
case VCHIQ_BULK_MODE_BLOCKING:
|
||||
@ -477,7 +478,8 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
|
||||
}
|
||||
}
|
||||
|
||||
status = vchiq_bulk_transfer(handle, data, size, &waiter->bulk_waiter,
|
||||
status = vchiq_bulk_transfer(handle, data, NULL, size,
|
||||
&waiter->bulk_waiter,
|
||||
VCHIQ_BULK_MODE_BLOCKING, dir);
|
||||
if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
|
||||
!waiter->bulk_waiter.bulk) {
|
||||
@ -924,7 +926,7 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
|
||||
ret = -ENOTCONN;
|
||||
} else if (header->size <= args->bufsize) {
|
||||
/* Copy to user space if msgbuf is not NULL */
|
||||
if (!args->buf || (copy_to_user((void __user *)args->buf,
|
||||
if (!args->buf || (copy_to_user(args->buf,
|
||||
header->data, header->size) == 0)) {
|
||||
ret = header->size;
|
||||
vchiq_release_message(service->handle, header);
|
||||
@ -997,7 +999,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
|
||||
* accessing kernel data instead of user space, based on the
|
||||
* address.
|
||||
*/
|
||||
status = vchiq_bulk_transfer(args->handle, args->data, args->size,
|
||||
status = vchiq_bulk_transfer(args->handle, NULL, args->data, args->size,
|
||||
userdata, args->mode, dir);
|
||||
|
||||
if (!waiter) {
|
||||
|
@ -3015,8 +3015,8 @@ vchiq_remove_service(unsigned int handle)
|
||||
* structure.
|
||||
*/
|
||||
enum vchiq_status vchiq_bulk_transfer(unsigned int handle,
|
||||
void *offset, int size,
|
||||
void *userdata,
|
||||
void *offset, void __user *uoffset,
|
||||
int size, void *userdata,
|
||||
enum vchiq_bulk_mode mode,
|
||||
enum vchiq_bulk_dir dir)
|
||||
{
|
||||
@ -3032,7 +3032,8 @@ enum vchiq_status vchiq_bulk_transfer(unsigned int handle,
|
||||
int payload[2];
|
||||
|
||||
if (!service || service->srvstate != VCHIQ_SRVSTATE_OPEN ||
|
||||
!offset || vchiq_check_service(service) != VCHIQ_SUCCESS)
|
||||
(!offset && !uoffset) ||
|
||||
vchiq_check_service(service) != VCHIQ_SUCCESS)
|
||||
goto error_exit;
|
||||
|
||||
switch (mode) {
|
||||
@ -3088,7 +3089,8 @@ enum vchiq_status vchiq_bulk_transfer(unsigned int handle,
|
||||
bulk->size = size;
|
||||
bulk->actual = VCHIQ_BULK_ACTUAL_ABORTED;
|
||||
|
||||
if (vchiq_prepare_bulk_data(bulk, offset, size, dir) != VCHIQ_SUCCESS)
|
||||
if (vchiq_prepare_bulk_data(bulk, offset, uoffset, size, dir)
|
||||
!= VCHIQ_SUCCESS)
|
||||
goto unlock_error_exit;
|
||||
|
||||
wmb();
|
||||
|
@ -559,8 +559,8 @@ extern void
|
||||
remote_event_pollall(struct vchiq_state *state);
|
||||
|
||||
extern enum vchiq_status
|
||||
vchiq_bulk_transfer(unsigned int handle, void *offset, int size,
|
||||
void *userdata, enum vchiq_bulk_mode mode,
|
||||
vchiq_bulk_transfer(unsigned int handle, void *offset, void __user *uoffset,
|
||||
int size, void *userdata, enum vchiq_bulk_mode mode,
|
||||
enum vchiq_bulk_dir dir);
|
||||
|
||||
extern int
|
||||
@ -633,7 +633,7 @@ vchiq_queue_message(unsigned int handle,
|
||||
|
||||
extern enum vchiq_status
|
||||
vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset,
|
||||
int size, int dir);
|
||||
void __user *uoffset, int size, int dir);
|
||||
|
||||
extern void
|
||||
vchiq_complete_bulk(struct vchiq_bulk *bulk);
|
||||
|
Loading…
Reference in New Issue
Block a user