1
0
Fork 0

binder: avoid kernel vm_area for buffer fixups

Refactor the functions to validate and fixup struct
binder_buffer pointer objects to avoid using vm_area
pointers. Instead copy to/from kernel space using
binder_alloc_copy_to_buffer() and
binder_alloc_copy_from_buffer(). The following
functions were refactored:

	refactor binder_validate_ptr()
	binder_validate_fixup()
	binder_fixup_parent()

Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
hifive-unleashed-5.1
Todd Kjos 2019-02-08 10:35:17 -08:00 committed by Greg Kroah-Hartman
parent 7a67a39320
commit db6b0b810b
1 changed files with 97 additions and 49 deletions

View File

@ -2092,10 +2092,13 @@ static size_t binder_get_object(struct binder_proc *proc,
/** /**
* binder_validate_ptr() - validates binder_buffer_object in a binder_buffer. * binder_validate_ptr() - validates binder_buffer_object in a binder_buffer.
* @proc: binder_proc owning the buffer
* @b: binder_buffer containing the object * @b: binder_buffer containing the object
* @object: struct binder_object to read into
* @index: index in offset array at which the binder_buffer_object is * @index: index in offset array at which the binder_buffer_object is
* located * located
* @start: points to the start of the offset array * @start_offset: points to the start of the offset array
* @object_offsetp: offset of @object read from @b
* @num_valid: the number of valid offsets in the offset array * @num_valid: the number of valid offsets in the offset array
* *
* Return: If @index is within the valid range of the offset array * Return: If @index is within the valid range of the offset array
@ -2106,34 +2109,46 @@ static size_t binder_get_object(struct binder_proc *proc,
* Note that the offset found in index @index itself is not * Note that the offset found in index @index itself is not
* verified; this function assumes that @num_valid elements * verified; this function assumes that @num_valid elements
* from @start were previously verified to have valid offsets. * from @start were previously verified to have valid offsets.
* If @object_offsetp is non-NULL, then the offset within
* @b is written to it.
*/ */
static struct binder_buffer_object *binder_validate_ptr(struct binder_buffer *b, static struct binder_buffer_object *binder_validate_ptr(
binder_size_t index, struct binder_proc *proc,
binder_size_t *start, struct binder_buffer *b,
binder_size_t num_valid) struct binder_object *object,
binder_size_t index,
binder_size_t start_offset,
binder_size_t *object_offsetp,
binder_size_t num_valid)
{ {
struct binder_buffer_object *buffer_obj; size_t object_size;
binder_size_t *offp; binder_size_t object_offset;
unsigned long buffer_offset;
if (index >= num_valid) if (index >= num_valid)
return NULL; return NULL;
offp = start + index; buffer_offset = start_offset + sizeof(binder_size_t) * index;
buffer_obj = (struct binder_buffer_object *)(b->data + *offp); binder_alloc_copy_from_buffer(&proc->alloc, &object_offset,
if (buffer_obj->hdr.type != BINDER_TYPE_PTR) b, buffer_offset, sizeof(object_offset));
object_size = binder_get_object(proc, b, object_offset, object);
if (!object_size || object->hdr.type != BINDER_TYPE_PTR)
return NULL; return NULL;
if (object_offsetp)
*object_offsetp = object_offset;
return buffer_obj; return &object->bbo;
} }
/** /**
* binder_validate_fixup() - validates pointer/fd fixups happen in order. * binder_validate_fixup() - validates pointer/fd fixups happen in order.
* @proc: binder_proc owning the buffer
* @b: transaction buffer * @b: transaction buffer
* @objects_start start of objects buffer * @objects_start_offset: offset to start of objects buffer
* @buffer: binder_buffer_object in which to fix up * @buffer_obj_offset: offset to binder_buffer_object in which to fix up
* @offset: start offset in @buffer to fix up * @fixup_offset: start offset in @buffer to fix up
* @last_obj: last binder_buffer_object that we fixed up in * @last_obj_offset: offset to last binder_buffer_object that we fixed
* @last_min_offset: minimum fixup offset in @last_obj * @last_min_offset: minimum fixup offset in object at @last_obj_offset
* *
* Return: %true if a fixup in buffer @buffer at offset @offset is * Return: %true if a fixup in buffer @buffer at offset @offset is
* allowed. * allowed.
@ -2164,28 +2179,41 @@ static struct binder_buffer_object *binder_validate_ptr(struct binder_buffer *b,
* C (parent = A, offset = 16) * C (parent = A, offset = 16)
* D (parent = B, offset = 0) // B is not A or any of A's parents * D (parent = B, offset = 0) // B is not A or any of A's parents
*/ */
static bool binder_validate_fixup(struct binder_buffer *b, static bool binder_validate_fixup(struct binder_proc *proc,
binder_size_t *objects_start, struct binder_buffer *b,
struct binder_buffer_object *buffer, binder_size_t objects_start_offset,
binder_size_t buffer_obj_offset,
binder_size_t fixup_offset, binder_size_t fixup_offset,
struct binder_buffer_object *last_obj, binder_size_t last_obj_offset,
binder_size_t last_min_offset) binder_size_t last_min_offset)
{ {
if (!last_obj) { if (!last_obj_offset) {
/* Nothing to fix up in */ /* Nothing to fix up in */
return false; return false;
} }
while (last_obj != buffer) { while (last_obj_offset != buffer_obj_offset) {
unsigned long buffer_offset;
struct binder_object last_object;
struct binder_buffer_object *last_bbo;
size_t object_size = binder_get_object(proc, b, last_obj_offset,
&last_object);
if (object_size != sizeof(*last_bbo))
return false;
last_bbo = &last_object.bbo;
/* /*
* Safe to retrieve the parent of last_obj, since it * Safe to retrieve the parent of last_obj, since it
* was already previously verified by the driver. * was already previously verified by the driver.
*/ */
if ((last_obj->flags & BINDER_BUFFER_FLAG_HAS_PARENT) == 0) if ((last_bbo->flags & BINDER_BUFFER_FLAG_HAS_PARENT) == 0)
return false; return false;
last_min_offset = last_obj->parent_offset + sizeof(uintptr_t); last_min_offset = last_bbo->parent_offset + sizeof(uintptr_t);
last_obj = (struct binder_buffer_object *) buffer_offset = objects_start_offset +
(b->data + *(objects_start + last_obj->parent)); sizeof(binder_size_t) * last_bbo->parent,
binder_alloc_copy_from_buffer(&proc->alloc, &last_obj_offset,
b, buffer_offset,
sizeof(last_obj_offset));
} }
return (fixup_offset >= last_min_offset); return (fixup_offset >= last_min_offset);
} }
@ -2254,6 +2282,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
{ {
binder_size_t *offp, *off_start, *off_end; binder_size_t *offp, *off_start, *off_end;
int debug_id = buffer->debug_id; int debug_id = buffer->debug_id;
binder_size_t off_start_offset;
binder_debug(BINDER_DEBUG_TRANSACTION, binder_debug(BINDER_DEBUG_TRANSACTION,
"%d buffer release %d, size %zd-%zd, failed at %pK\n", "%d buffer release %d, size %zd-%zd, failed at %pK\n",
@ -2263,8 +2292,8 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
if (buffer->target_node) if (buffer->target_node)
binder_dec_node(buffer->target_node, 1, 0); binder_dec_node(buffer->target_node, 1, 0);
off_start = (binder_size_t *)(buffer->data + off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
ALIGN(buffer->data_size, sizeof(void *))); off_start = (binder_size_t *)(buffer->data + off_start_offset);
if (failed_at) if (failed_at)
off_end = failed_at; off_end = failed_at;
else else
@ -2350,6 +2379,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
case BINDER_TYPE_FDA: { case BINDER_TYPE_FDA: {
struct binder_fd_array_object *fda; struct binder_fd_array_object *fda;
struct binder_buffer_object *parent; struct binder_buffer_object *parent;
struct binder_object ptr_object;
uintptr_t parent_buffer; uintptr_t parent_buffer;
u32 *fd_array; u32 *fd_array;
size_t fd_index; size_t fd_index;
@ -2365,8 +2395,10 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
} }
fda = to_binder_fd_array_object(hdr); fda = to_binder_fd_array_object(hdr);
parent = binder_validate_ptr(buffer, fda->parent, parent = binder_validate_ptr(proc, buffer, &ptr_object,
off_start, fda->parent,
off_start_offset,
NULL,
offp - off_start); offp - off_start);
if (!parent) { if (!parent) {
pr_err("transaction release %d bad parent offset\n", pr_err("transaction release %d bad parent offset\n",
@ -2665,9 +2697,9 @@ static int binder_translate_fd_array(struct binder_fd_array_object *fda,
static int binder_fixup_parent(struct binder_transaction *t, static int binder_fixup_parent(struct binder_transaction *t,
struct binder_thread *thread, struct binder_thread *thread,
struct binder_buffer_object *bp, struct binder_buffer_object *bp,
binder_size_t *off_start, binder_size_t off_start_offset,
binder_size_t num_valid, binder_size_t num_valid,
struct binder_buffer_object *last_fixup_obj, binder_size_t last_fixup_obj_off,
binder_size_t last_fixup_min_off) binder_size_t last_fixup_min_off)
{ {
struct binder_buffer_object *parent; struct binder_buffer_object *parent;
@ -2675,20 +2707,25 @@ static int binder_fixup_parent(struct binder_transaction *t,
struct binder_buffer *b = t->buffer; struct binder_buffer *b = t->buffer;
struct binder_proc *proc = thread->proc; struct binder_proc *proc = thread->proc;
struct binder_proc *target_proc = t->to_proc; struct binder_proc *target_proc = t->to_proc;
struct binder_object object;
binder_size_t buffer_offset;
binder_size_t parent_offset;
if (!(bp->flags & BINDER_BUFFER_FLAG_HAS_PARENT)) if (!(bp->flags & BINDER_BUFFER_FLAG_HAS_PARENT))
return 0; return 0;
parent = binder_validate_ptr(b, bp->parent, off_start, num_valid); parent = binder_validate_ptr(target_proc, b, &object, bp->parent,
off_start_offset, &parent_offset,
num_valid);
if (!parent) { if (!parent) {
binder_user_error("%d:%d got transaction with invalid parent offset or type\n", binder_user_error("%d:%d got transaction with invalid parent offset or type\n",
proc->pid, thread->pid); proc->pid, thread->pid);
return -EINVAL; return -EINVAL;
} }
if (!binder_validate_fixup(b, off_start, if (!binder_validate_fixup(target_proc, b, off_start_offset,
parent, bp->parent_offset, parent_offset, bp->parent_offset,
last_fixup_obj, last_fixup_obj_off,
last_fixup_min_off)) { last_fixup_min_off)) {
binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n", binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n",
proc->pid, thread->pid); proc->pid, thread->pid);
@ -2705,7 +2742,10 @@ static int binder_fixup_parent(struct binder_transaction *t,
parent_buffer = (u8 *)((uintptr_t)parent->buffer - parent_buffer = (u8 *)((uintptr_t)parent->buffer -
binder_alloc_get_user_buffer_offset( binder_alloc_get_user_buffer_offset(
&target_proc->alloc)); &target_proc->alloc));
*(binder_uintptr_t *)(parent_buffer + bp->parent_offset) = bp->buffer; buffer_offset = bp->parent_offset +
(uintptr_t)parent_buffer - (uintptr_t)b->data;
binder_alloc_copy_to_buffer(&target_proc->alloc, b, buffer_offset,
&bp->buffer, sizeof(bp->buffer));
return 0; return 0;
} }
@ -2825,6 +2865,7 @@ static void binder_transaction(struct binder_proc *proc,
struct binder_work *w; struct binder_work *w;
struct binder_work *tcomplete; struct binder_work *tcomplete;
binder_size_t *offp, *off_end, *off_start; binder_size_t *offp, *off_end, *off_start;
binder_size_t off_start_offset;
binder_size_t off_min; binder_size_t off_min;
u8 *sg_bufp, *sg_buf_end; u8 *sg_bufp, *sg_buf_end;
struct binder_proc *target_proc = NULL; struct binder_proc *target_proc = NULL;
@ -2835,7 +2876,7 @@ static void binder_transaction(struct binder_proc *proc,
uint32_t return_error = 0; uint32_t return_error = 0;
uint32_t return_error_param = 0; uint32_t return_error_param = 0;
uint32_t return_error_line = 0; uint32_t return_error_line = 0;
struct binder_buffer_object *last_fixup_obj = NULL; binder_size_t last_fixup_obj_off = 0;
binder_size_t last_fixup_min_off = 0; binder_size_t last_fixup_min_off = 0;
struct binder_context *context = proc->context; struct binder_context *context = proc->context;
int t_debug_id = atomic_inc_return(&binder_last_id); int t_debug_id = atomic_inc_return(&binder_last_id);
@ -3132,8 +3173,8 @@ static void binder_transaction(struct binder_proc *proc,
t->buffer->transaction = t; t->buffer->transaction = t;
t->buffer->target_node = target_node; t->buffer->target_node = target_node;
trace_binder_transaction_alloc_buf(t->buffer); trace_binder_transaction_alloc_buf(t->buffer);
off_start = (binder_size_t *)(t->buffer->data + off_start_offset = ALIGN(tr->data_size, sizeof(void *));
ALIGN(tr->data_size, sizeof(void *))); off_start = (binder_size_t *)(t->buffer->data + off_start_offset);
offp = off_start; offp = off_start;
if (binder_alloc_copy_user_to_buffer( if (binder_alloc_copy_user_to_buffer(
@ -3266,11 +3307,15 @@ static void binder_transaction(struct binder_proc *proc,
fp, sizeof(*fp)); fp, sizeof(*fp));
} break; } break;
case BINDER_TYPE_FDA: { case BINDER_TYPE_FDA: {
struct binder_object ptr_object;
binder_size_t parent_offset;
struct binder_fd_array_object *fda = struct binder_fd_array_object *fda =
to_binder_fd_array_object(hdr); to_binder_fd_array_object(hdr);
struct binder_buffer_object *parent = struct binder_buffer_object *parent =
binder_validate_ptr(t->buffer, fda->parent, binder_validate_ptr(target_proc, t->buffer,
off_start, &ptr_object, fda->parent,
off_start_offset,
&parent_offset,
offp - off_start); offp - off_start);
if (!parent) { if (!parent) {
binder_user_error("%d:%d got transaction with invalid parent offset or type\n", binder_user_error("%d:%d got transaction with invalid parent offset or type\n",
@ -3280,9 +3325,11 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__; return_error_line = __LINE__;
goto err_bad_parent; goto err_bad_parent;
} }
if (!binder_validate_fixup(t->buffer, off_start, if (!binder_validate_fixup(target_proc, t->buffer,
parent, fda->parent_offset, off_start_offset,
last_fixup_obj, parent_offset,
fda->parent_offset,
last_fixup_obj_off,
last_fixup_min_off)) { last_fixup_min_off)) {
binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n", binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n",
proc->pid, thread->pid); proc->pid, thread->pid);
@ -3299,7 +3346,7 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__; return_error_line = __LINE__;
goto err_translate_failed; goto err_translate_failed;
} }
last_fixup_obj = parent; last_fixup_obj_off = parent_offset;
last_fixup_min_off = last_fixup_min_off =
fda->parent_offset + sizeof(u32) * fda->num_fds; fda->parent_offset + sizeof(u32) * fda->num_fds;
} break; } break;
@ -3338,9 +3385,10 @@ static void binder_transaction(struct binder_proc *proc,
&target_proc->alloc); &target_proc->alloc);
sg_bufp += ALIGN(bp->length, sizeof(u64)); sg_bufp += ALIGN(bp->length, sizeof(u64));
ret = binder_fixup_parent(t, thread, bp, off_start, ret = binder_fixup_parent(t, thread, bp,
off_start_offset,
offp - off_start, offp - off_start,
last_fixup_obj, last_fixup_obj_off,
last_fixup_min_off); last_fixup_min_off);
if (ret < 0) { if (ret < 0) {
return_error = BR_FAILED_REPLY; return_error = BR_FAILED_REPLY;
@ -3351,7 +3399,7 @@ static void binder_transaction(struct binder_proc *proc,
binder_alloc_copy_to_buffer(&target_proc->alloc, binder_alloc_copy_to_buffer(&target_proc->alloc,
t->buffer, object_offset, t->buffer, object_offset,
bp, sizeof(*bp)); bp, sizeof(*bp));
last_fixup_obj = t->buffer->data + object_offset; last_fixup_obj_off = object_offset;
last_fixup_min_off = 0; last_fixup_min_off = 0;
} break; } break;
default: default: