Skip to content

Commit bde4a19

Browse files
Todd Kjosgregkh
authored andcommitted
binder: use userspace pointer as base of buffer space
Now that alloc->buffer points to the userspace vm_area rename buffer->data to buffer->user_data and rename local pointers that hold user addresses. Also use the "__user" tag to annotate all user pointers so sparse can flag cases where user pointer vaues are copied to kernel pointers. Refactor code to use offsets instead of user pointers. Signed-off-by: Todd Kjos <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent c41358a commit bde4a19

File tree

5 files changed

+118
-99
lines changed

5 files changed

+118
-99
lines changed

drivers/android/binder.c

Lines changed: 67 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,33 +2278,30 @@ static void binder_deferred_fd_close(int fd)
22782278

22792279
static void binder_transaction_buffer_release(struct binder_proc *proc,
22802280
struct binder_buffer *buffer,
2281-
binder_size_t *failed_at)
2281+
binder_size_t failed_at,
2282+
bool is_failure)
22822283
{
2283-
binder_size_t *offp, *off_start, *off_end;
22842284
int debug_id = buffer->debug_id;
2285-
binder_size_t off_start_offset;
2285+
binder_size_t off_start_offset, buffer_offset, off_end_offset;
22862286

22872287
binder_debug(BINDER_DEBUG_TRANSACTION,
2288-
"%d buffer release %d, size %zd-%zd, failed at %pK\n",
2288+
"%d buffer release %d, size %zd-%zd, failed at %llx\n",
22892289
proc->pid, buffer->debug_id,
2290-
buffer->data_size, buffer->offsets_size, failed_at);
2290+
buffer->data_size, buffer->offsets_size,
2291+
(unsigned long long)failed_at);
22912292

22922293
if (buffer->target_node)
22932294
binder_dec_node(buffer->target_node, 1, 0);
22942295

22952296
off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
2296-
off_start = (binder_size_t *)(buffer->data + off_start_offset);
2297-
if (failed_at)
2298-
off_end = failed_at;
2299-
else
2300-
off_end = (void *)off_start + buffer->offsets_size;
2301-
for (offp = off_start; offp < off_end; offp++) {
2297+
off_end_offset = is_failure ? failed_at :
2298+
off_start_offset + buffer->offsets_size;
2299+
for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
2300+
buffer_offset += sizeof(binder_size_t)) {
23022301
struct binder_object_header *hdr;
23032302
size_t object_size;
23042303
struct binder_object object;
23052304
binder_size_t object_offset;
2306-
binder_size_t buffer_offset = (uintptr_t)offp -
2307-
(uintptr_t)buffer->data;
23082305

23092306
binder_alloc_copy_from_buffer(&proc->alloc, &object_offset,
23102307
buffer, buffer_offset,
@@ -2380,9 +2377,10 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
23802377
struct binder_fd_array_object *fda;
23812378
struct binder_buffer_object *parent;
23822379
struct binder_object ptr_object;
2383-
u32 *fd_array;
2380+
binder_size_t fda_offset;
23842381
size_t fd_index;
23852382
binder_size_t fd_buf_size;
2383+
binder_size_t num_valid;
23862384

23872385
if (proc->tsk != current->group_leader) {
23882386
/*
@@ -2393,12 +2391,14 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
23932391
continue;
23942392
}
23952393

2394+
num_valid = (buffer_offset - off_start_offset) /
2395+
sizeof(binder_size_t);
23962396
fda = to_binder_fd_array_object(hdr);
23972397
parent = binder_validate_ptr(proc, buffer, &ptr_object,
23982398
fda->parent,
23992399
off_start_offset,
24002400
NULL,
2401-
offp - off_start);
2401+
num_valid);
24022402
if (!parent) {
24032403
pr_err("transaction release %d bad parent offset\n",
24042404
debug_id);
@@ -2417,14 +2417,21 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
24172417
debug_id, (u64)fda->num_fds);
24182418
continue;
24192419
}
2420-
fd_array = (u32 *)(uintptr_t)
2421-
(parent->buffer + fda->parent_offset);
2420+
/*
2421+
* the source data for binder_buffer_object is visible
2422+
* to user-space and the @buffer element is the user
2423+
* pointer to the buffer_object containing the fd_array.
2424+
* Convert the address to an offset relative to
2425+
* the base of the transaction buffer.
2426+
*/
2427+
fda_offset =
2428+
(parent->buffer - (uintptr_t)buffer->user_data) +
2429+
fda->parent_offset;
24222430
for (fd_index = 0; fd_index < fda->num_fds;
24232431
fd_index++) {
24242432
u32 fd;
2425-
binder_size_t offset =
2426-
(uintptr_t)&fd_array[fd_index] -
2427-
(uintptr_t)buffer->data;
2433+
binder_size_t offset = fda_offset +
2434+
fd_index * sizeof(fd);
24282435

24292436
binder_alloc_copy_from_buffer(&proc->alloc,
24302437
&fd,
@@ -2638,7 +2645,7 @@ static int binder_translate_fd_array(struct binder_fd_array_object *fda,
26382645
struct binder_transaction *in_reply_to)
26392646
{
26402647
binder_size_t fdi, fd_buf_size;
2641-
u32 *fd_array;
2648+
binder_size_t fda_offset;
26422649
struct binder_proc *proc = thread->proc;
26432650
struct binder_proc *target_proc = t->to_proc;
26442651

@@ -2655,18 +2662,24 @@ static int binder_translate_fd_array(struct binder_fd_array_object *fda,
26552662
proc->pid, thread->pid, (u64)fda->num_fds);
26562663
return -EINVAL;
26572664
}
2658-
fd_array = (u32 *)(uintptr_t)(parent->buffer + fda->parent_offset);
2659-
if (!IS_ALIGNED((unsigned long)fd_array, sizeof(u32))) {
2665+
/*
2666+
* the source data for binder_buffer_object is visible
2667+
* to user-space and the @buffer element is the user
2668+
* pointer to the buffer_object containing the fd_array.
2669+
* Convert the address to an offset relative to
2670+
* the base of the transaction buffer.
2671+
*/
2672+
fda_offset = (parent->buffer - (uintptr_t)t->buffer->user_data) +
2673+
fda->parent_offset;
2674+
if (!IS_ALIGNED((unsigned long)fda_offset, sizeof(u32))) {
26602675
binder_user_error("%d:%d parent offset not aligned correctly.\n",
26612676
proc->pid, thread->pid);
26622677
return -EINVAL;
26632678
}
26642679
for (fdi = 0; fdi < fda->num_fds; fdi++) {
26652680
u32 fd;
26662681
int ret;
2667-
binder_size_t offset =
2668-
(uintptr_t)&fd_array[fdi] -
2669-
(uintptr_t)t->buffer->data;
2682+
binder_size_t offset = fda_offset + fdi * sizeof(fd);
26702683

26712684
binder_alloc_copy_from_buffer(&target_proc->alloc,
26722685
&fd, t->buffer,
@@ -2724,7 +2737,7 @@ static int binder_fixup_parent(struct binder_transaction *t,
27242737
return -EINVAL;
27252738
}
27262739
buffer_offset = bp->parent_offset +
2727-
(uintptr_t)parent->buffer - (uintptr_t)b->data;
2740+
(uintptr_t)parent->buffer - (uintptr_t)b->user_data;
27282741
binder_alloc_copy_to_buffer(&target_proc->alloc, b, buffer_offset,
27292742
&bp->buffer, sizeof(bp->buffer));
27302743

@@ -2845,10 +2858,10 @@ static void binder_transaction(struct binder_proc *proc,
28452858
struct binder_transaction *t;
28462859
struct binder_work *w;
28472860
struct binder_work *tcomplete;
2848-
binder_size_t *offp, *off_end, *off_start;
2849-
binder_size_t off_start_offset;
2861+
binder_size_t buffer_offset = 0;
2862+
binder_size_t off_start_offset, off_end_offset;
28502863
binder_size_t off_min;
2851-
u8 *sg_bufp, *sg_buf_end;
2864+
binder_size_t sg_buf_offset, sg_buf_end_offset;
28522865
struct binder_proc *target_proc = NULL;
28532866
struct binder_thread *target_thread = NULL;
28542867
struct binder_node *target_node = NULL;
@@ -3141,7 +3154,7 @@ static void binder_transaction(struct binder_proc *proc,
31413154
ALIGN(extra_buffers_size, sizeof(void *)) -
31423155
ALIGN(secctx_sz, sizeof(u64));
31433156

3144-
t->security_ctx = (uintptr_t)t->buffer->data + buf_offset;
3157+
t->security_ctx = (uintptr_t)t->buffer->user_data + buf_offset;
31453158
binder_alloc_copy_to_buffer(&target_proc->alloc,
31463159
t->buffer, buf_offset,
31473160
secctx, secctx_sz);
@@ -3152,9 +3165,6 @@ static void binder_transaction(struct binder_proc *proc,
31523165
t->buffer->transaction = t;
31533166
t->buffer->target_node = target_node;
31543167
trace_binder_transaction_alloc_buf(t->buffer);
3155-
off_start_offset = ALIGN(tr->data_size, sizeof(void *));
3156-
off_start = (binder_size_t *)(t->buffer->data + off_start_offset);
3157-
offp = off_start;
31583168

31593169
if (binder_alloc_copy_user_to_buffer(
31603170
&target_proc->alloc,
@@ -3200,17 +3210,18 @@ static void binder_transaction(struct binder_proc *proc,
32003210
return_error_line = __LINE__;
32013211
goto err_bad_offset;
32023212
}
3203-
off_end = (void *)off_start + tr->offsets_size;
3204-
sg_bufp = (u8 *)(PTR_ALIGN(off_end, sizeof(void *)));
3205-
sg_buf_end = sg_bufp + extra_buffers_size;
3213+
off_start_offset = ALIGN(tr->data_size, sizeof(void *));
3214+
buffer_offset = off_start_offset;
3215+
off_end_offset = off_start_offset + tr->offsets_size;
3216+
sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));
3217+
sg_buf_end_offset = sg_buf_offset + extra_buffers_size;
32063218
off_min = 0;
3207-
for (; offp < off_end; offp++) {
3219+
for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
3220+
buffer_offset += sizeof(binder_size_t)) {
32083221
struct binder_object_header *hdr;
32093222
size_t object_size;
32103223
struct binder_object object;
32113224
binder_size_t object_offset;
3212-
binder_size_t buffer_offset =
3213-
(uintptr_t)offp - (uintptr_t)t->buffer->data;
32143225

32153226
binder_alloc_copy_from_buffer(&target_proc->alloc,
32163227
&object_offset,
@@ -3290,12 +3301,14 @@ static void binder_transaction(struct binder_proc *proc,
32903301
binder_size_t parent_offset;
32913302
struct binder_fd_array_object *fda =
32923303
to_binder_fd_array_object(hdr);
3304+
size_t num_valid = (buffer_offset - off_start_offset) *
3305+
sizeof(binder_size_t);
32933306
struct binder_buffer_object *parent =
32943307
binder_validate_ptr(target_proc, t->buffer,
32953308
&ptr_object, fda->parent,
32963309
off_start_offset,
32973310
&parent_offset,
3298-
offp - off_start);
3311+
num_valid);
32993312
if (!parent) {
33003313
binder_user_error("%d:%d got transaction with invalid parent offset or type\n",
33013314
proc->pid, thread->pid);
@@ -3332,9 +3345,8 @@ static void binder_transaction(struct binder_proc *proc,
33323345
case BINDER_TYPE_PTR: {
33333346
struct binder_buffer_object *bp =
33343347
to_binder_buffer_object(hdr);
3335-
size_t buf_left = sg_buf_end - sg_bufp;
3336-
binder_size_t sg_buf_offset = (uintptr_t)sg_bufp -
3337-
(uintptr_t)t->buffer->data;
3348+
size_t buf_left = sg_buf_end_offset - sg_buf_offset;
3349+
size_t num_valid;
33383350

33393351
if (bp->length > buf_left) {
33403352
binder_user_error("%d:%d got transaction with too large buffer\n",
@@ -3359,12 +3371,15 @@ static void binder_transaction(struct binder_proc *proc,
33593371
goto err_copy_data_failed;
33603372
}
33613373
/* Fixup buffer pointer to target proc address space */
3362-
bp->buffer = (uintptr_t)sg_bufp;
3363-
sg_bufp += ALIGN(bp->length, sizeof(u64));
3374+
bp->buffer = (uintptr_t)
3375+
t->buffer->user_data + sg_buf_offset;
3376+
sg_buf_offset += ALIGN(bp->length, sizeof(u64));
33643377

3378+
num_valid = (buffer_offset - off_start_offset) *
3379+
sizeof(binder_size_t);
33653380
ret = binder_fixup_parent(t, thread, bp,
33663381
off_start_offset,
3367-
offp - off_start,
3382+
num_valid,
33683383
last_fixup_obj_off,
33693384
last_fixup_min_off);
33703385
if (ret < 0) {
@@ -3456,7 +3471,8 @@ static void binder_transaction(struct binder_proc *proc,
34563471
err_copy_data_failed:
34573472
binder_free_txn_fixups(t);
34583473
trace_binder_transaction_failed_buffer_release(t->buffer);
3459-
binder_transaction_buffer_release(target_proc, t->buffer, offp);
3474+
binder_transaction_buffer_release(target_proc, t->buffer,
3475+
buffer_offset, true);
34603476
if (target_node)
34613477
binder_dec_node_tmpref(target_node);
34623478
target_node = NULL;
@@ -3557,7 +3573,7 @@ binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer)
35573573
binder_node_inner_unlock(buf_node);
35583574
}
35593575
trace_binder_transaction_buffer_release(buffer);
3560-
binder_transaction_buffer_release(proc, buffer, NULL);
3576+
binder_transaction_buffer_release(proc, buffer, 0, false);
35613577
binder_alloc_free_buf(&proc->alloc, buffer);
35623578
}
35633579

@@ -4451,7 +4467,7 @@ static int binder_thread_read(struct binder_proc *proc,
44514467
}
44524468
trd->data_size = t->buffer->data_size;
44534469
trd->offsets_size = t->buffer->offsets_size;
4454-
trd->data.ptr.buffer = (uintptr_t)t->buffer->data;
4470+
trd->data.ptr.buffer = (uintptr_t)t->buffer->user_data;
44554471
trd->data.ptr.offsets = trd->data.ptr.buffer +
44564472
ALIGN(t->buffer->data_size,
44574473
sizeof(void *));
@@ -5489,7 +5505,7 @@ static void print_binder_transaction_ilocked(struct seq_file *m,
54895505
seq_printf(m, " node %d", buffer->target_node->debug_id);
54905506
seq_printf(m, " size %zd:%zd data %pK\n",
54915507
buffer->data_size, buffer->offsets_size,
5492-
buffer->data);
5508+
buffer->user_data);
54935509
}
54945510

54955511
static void print_binder_work_ilocked(struct seq_file *m,

0 commit comments

Comments
 (0)