Received: by 2002:ac0:946b:0:0:0:0:0 with SMTP id j40csp1992675imj; Fri, 8 Feb 2019 10:36:31 -0800 (PST) X-Google-Smtp-Source: AHgI3IZu6sS3xnridYmYK372Eqb4jBNegy7gASKgt9gGssOsQdYdnmdZR9iif/ICdDLdx7puirnz X-Received: by 2002:a63:4f5e:: with SMTP id p30mr11322607pgl.71.1549650991341; Fri, 08 Feb 2019 10:36:31 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1549650991; cv=none; d=google.com; s=arc-20160816; b=mtkaxCWdDlaCDVqc5woVfwCdK9HYXb/+UMALPXANRIKiZ/RDKvRn56dUoGYkk4Xdqy kN+afwliANDizCt1yEYYb/8hA7HQQ9k5b0oPRx1J+0/X+8vnjWpdsf/riv9YCRpCy9yd 1RptcFyN/8pplHYUF44XsaxqFmHHMdijlb3gsN4lPbq6uRLx8t4XrIdVhCJq7jsRU0Ap GQpfPe9ihcKoiJFVDSxkzX4EcuHcZ4dJ7wLII2kAt9SDW+QfQOD7TCg51he37EfwEcdy XfqsnEmxpSBavhArQHrhihw3cv3E+QmQlZ9CzH/m+2yF+2UPpr092cwC2Dl7R8WgkCIe iFKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=xg+qYp+4sFzWmUuF/Mb341Ib7ZOx1DBI0D76Lp7pNZs=; b=NO19TANzlBUAm2Y7jtbxgG2ktJ39neM8rUaO+LoXd2RxjXbH0w0YhGMhUGwsv3nWS2 ixe9rnqrc/4ehqTEU3fJlyu8m1sP1ardqIprj73vp2tQnKSMounJiRbVfDoZyta8+5NF 9U/mXI8hM517/XteC14CpKGqFO7OCCvjAE5nllb/b9P+txB/MV6x3+AgL5IE1ijaj5Dm VOMM43QCTcTnlR/3U8L0ZTNjKbFVOifhJIKB4H2hMzJl9yZ9hEP8tda/YHgqKrLU2WLb /RnSqhCtKbGfxtwbSVgiKsmPkL0SYX55PsEMDujLwFIaqKul/r3ajrekrdydDACZFVZ/ HEPA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@android.com header.s=20161025 header.b=AhmJuNl6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=android.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x34si2692488pgl.491.2019.02.08.10.36.15; Fri, 08 Feb 2019 10:36:31 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@android.com header.s=20161025 header.b=AhmJuNl6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=android.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727787AbfBHSft (ORCPT + 99 others); Fri, 8 Feb 2019 13:35:49 -0500 Received: from mail-pl1-f194.google.com ([209.85.214.194]:32959 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728052AbfBHSfn (ORCPT ); Fri, 8 Feb 2019 13:35:43 -0500 Received: by mail-pl1-f194.google.com with SMTP id z8so2105239plo.0 for ; Fri, 08 Feb 2019 10:35:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=android.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xg+qYp+4sFzWmUuF/Mb341Ib7ZOx1DBI0D76Lp7pNZs=; b=AhmJuNl6vH66UqY/COp73t7OcHIPL9zhQo1DxfctUTlIdcZzM8jhKd94TqR+C0AB6E OZXNRqr0/hsgDWlO5PNAoEpolcLd7lPm5UYjYsuw0s7sJg7Oet5/sEyFckNBlxY/do0K yJdINZk4OIU5iCPyZbnVd+8oIZWTJXTJGs3G8pMNjN0/gxWcINIR3T4ixTJbEycpfmvo 5W3+R6PzY5qDmNGsesjWpDwmNTOSSw5XzkTwoSucilt++40QInnUXvTqnJS7itfkQv69 TIAomYXoh1xmZH+539bkkYP6eSWAL0tXE1aqGu8/mpbEtMmZBU8v2GctSgc1nmyHzHoE 7B2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=xg+qYp+4sFzWmUuF/Mb341Ib7ZOx1DBI0D76Lp7pNZs=; b=tl+HWXxRC2AAMw5SZvvhTwBY5DIjkjLr4TyGxCKBFzrj5xBtJW6HcAf2bR0jMuVsoS HkVazvZSfL2UnD0i1DfPwTPhTsnZtcQzKhzfux0tLkEedRBfHj+ksXMa+OVAW80gtx0k BnCVY2YPIxwUVHIGv0t76uyAaDosOj3Z0Gx7RNrdvO3YxVD0FNEeVC1mva1om0o5Mh/r szTJeP0G4vesog/4V9BpUUPZpSQRwmouv2LM66rBEjIyzKMSU8z8ZF4i20CYPI8Y+bdC VfeDKi4GxxquzisxJGtkfaOACdT3uMEo3XLPZTbJPQaFXAXZ2dgbYiCHesmopDqzbhl9 sP4g== X-Gm-Message-State: AHQUAuaYncdUqZcS0NFu9aM1nLsp6G1SHH59FgxFImSqmJ9Edp1C3Gvf JoU9Wsy9Z9Iqnc70enUV75a76g== X-Received: by 2002:a17:902:b609:: with SMTP id b9mr24198050pls.57.1549650942622; Fri, 08 Feb 2019 10:35:42 -0800 (PST) Received: from ava-linux2.mtv.corp.google.com ([2620:0:1000:1601:6cc0:d41d:b970:fd7]) by smtp.googlemail.com with ESMTPSA id 14sm2306472pgq.22.2019.02.08.10.35.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Feb 2019 10:35:41 -0800 (PST) From: Todd Kjos X-Google-Original-From: Todd Kjos To: tkjos@google.com, gregkh@linuxfoundation.org, arve@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, maco@google.com Cc: joel@joelfernandes.org, kernel-team@android.com Subject: [PATCH v3 4/7] binder: avoid kernel vm_area for buffer fixups Date: Fri, 8 Feb 2019 10:35:17 -0800 Message-Id: <20190208183520.30886-5-tkjos@google.com> X-Mailer: git-send-email 2.20.1.791.gb4d0f1c61a-goog In-Reply-To: <20190208183520.30886-1-tkjos@google.com> References: <20190208183520.30886-1-tkjos@google.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- drivers/android/binder.c | 146 ++++++++++++++++++++++++++------------- 1 file changed, 97 insertions(+), 49 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 8063b405e4fa1..98163bf5f35c7 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -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. + * @proc: binder_proc owning the buffer * @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 * 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 * * 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 * verified; this function assumes that @num_valid elements * 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, - binder_size_t index, - binder_size_t *start, - binder_size_t num_valid) +static struct binder_buffer_object *binder_validate_ptr( + struct binder_proc *proc, + struct binder_buffer *b, + 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; - binder_size_t *offp; + size_t object_size; + binder_size_t object_offset; + unsigned long buffer_offset; if (index >= num_valid) return NULL; - offp = start + index; - buffer_obj = (struct binder_buffer_object *)(b->data + *offp); - if (buffer_obj->hdr.type != BINDER_TYPE_PTR) + buffer_offset = start_offset + sizeof(binder_size_t) * index; + binder_alloc_copy_from_buffer(&proc->alloc, &object_offset, + 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; + if (object_offsetp) + *object_offsetp = object_offset; - return buffer_obj; + return &object->bbo; } /** * binder_validate_fixup() - validates pointer/fd fixups happen in order. + * @proc: binder_proc owning the buffer * @b: transaction buffer - * @objects_start start of objects buffer - * @buffer: binder_buffer_object in which to fix up - * @offset: start offset in @buffer to fix up - * @last_obj: last binder_buffer_object that we fixed up in - * @last_min_offset: minimum fixup offset in @last_obj + * @objects_start_offset: offset to start of objects buffer + * @buffer_obj_offset: offset to binder_buffer_object in which to fix up + * @fixup_offset: start offset in @buffer to fix up + * @last_obj_offset: offset to last binder_buffer_object that we fixed + * @last_min_offset: minimum fixup offset in object at @last_obj_offset * * Return: %true if a fixup in buffer @buffer at offset @offset is * allowed. @@ -2164,28 +2179,41 @@ static struct binder_buffer_object *binder_validate_ptr(struct binder_buffer *b, * C (parent = A, offset = 16) * D (parent = B, offset = 0) // B is not A or any of A's parents */ -static bool binder_validate_fixup(struct binder_buffer *b, - binder_size_t *objects_start, - struct binder_buffer_object *buffer, +static bool binder_validate_fixup(struct binder_proc *proc, + struct binder_buffer *b, + binder_size_t objects_start_offset, + binder_size_t buffer_obj_offset, binder_size_t fixup_offset, - struct binder_buffer_object *last_obj, + binder_size_t last_obj_offset, binder_size_t last_min_offset) { - if (!last_obj) { + if (!last_obj_offset) { /* Nothing to fix up in */ 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 * 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; - last_min_offset = last_obj->parent_offset + sizeof(uintptr_t); - last_obj = (struct binder_buffer_object *) - (b->data + *(objects_start + last_obj->parent)); + last_min_offset = last_bbo->parent_offset + sizeof(uintptr_t); + buffer_offset = objects_start_offset + + 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); } @@ -2254,6 +2282,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, { binder_size_t *offp, *off_start, *off_end; int debug_id = buffer->debug_id; + binder_size_t off_start_offset; binder_debug(BINDER_DEBUG_TRANSACTION, "%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) binder_dec_node(buffer->target_node, 1, 0); - off_start = (binder_size_t *)(buffer->data + - ALIGN(buffer->data_size, sizeof(void *))); + off_start_offset = ALIGN(buffer->data_size, sizeof(void *)); + off_start = (binder_size_t *)(buffer->data + off_start_offset); if (failed_at) off_end = failed_at; else @@ -2350,6 +2379,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, case BINDER_TYPE_FDA: { struct binder_fd_array_object *fda; struct binder_buffer_object *parent; + struct binder_object ptr_object; uintptr_t parent_buffer; u32 *fd_array; 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); - parent = binder_validate_ptr(buffer, fda->parent, - off_start, + parent = binder_validate_ptr(proc, buffer, &ptr_object, + fda->parent, + off_start_offset, + NULL, offp - off_start); if (!parent) { 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, struct binder_thread *thread, struct binder_buffer_object *bp, - binder_size_t *off_start, + binder_size_t off_start_offset, 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) { 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_proc *proc = thread->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)) 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) { binder_user_error("%d:%d got transaction with invalid parent offset or type\n", proc->pid, thread->pid); return -EINVAL; } - if (!binder_validate_fixup(b, off_start, - parent, bp->parent_offset, - last_fixup_obj, + if (!binder_validate_fixup(target_proc, b, off_start_offset, + parent_offset, bp->parent_offset, + last_fixup_obj_off, last_fixup_min_off)) { binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n", proc->pid, thread->pid); @@ -2705,7 +2742,10 @@ static int binder_fixup_parent(struct binder_transaction *t, parent_buffer = (u8 *)((uintptr_t)parent->buffer - binder_alloc_get_user_buffer_offset( &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; } @@ -2825,6 +2865,7 @@ static void binder_transaction(struct binder_proc *proc, struct binder_work *w; struct binder_work *tcomplete; binder_size_t *offp, *off_end, *off_start; + binder_size_t off_start_offset; binder_size_t off_min; u8 *sg_bufp, *sg_buf_end; 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_param = 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; struct binder_context *context = proc->context; 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->target_node = target_node; trace_binder_transaction_alloc_buf(t->buffer); - off_start = (binder_size_t *)(t->buffer->data + - ALIGN(tr->data_size, sizeof(void *))); + off_start_offset = ALIGN(tr->data_size, sizeof(void *)); + off_start = (binder_size_t *)(t->buffer->data + off_start_offset); offp = off_start; if (binder_alloc_copy_user_to_buffer( @@ -3266,11 +3307,15 @@ static void binder_transaction(struct binder_proc *proc, fp, sizeof(*fp)); } break; case BINDER_TYPE_FDA: { + struct binder_object ptr_object; + binder_size_t parent_offset; struct binder_fd_array_object *fda = to_binder_fd_array_object(hdr); struct binder_buffer_object *parent = - binder_validate_ptr(t->buffer, fda->parent, - off_start, + binder_validate_ptr(target_proc, t->buffer, + &ptr_object, fda->parent, + off_start_offset, + &parent_offset, offp - off_start); if (!parent) { 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__; goto err_bad_parent; } - if (!binder_validate_fixup(t->buffer, off_start, - parent, fda->parent_offset, - last_fixup_obj, + if (!binder_validate_fixup(target_proc, t->buffer, + off_start_offset, + parent_offset, + fda->parent_offset, + last_fixup_obj_off, last_fixup_min_off)) { binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n", proc->pid, thread->pid); @@ -3299,7 +3346,7 @@ static void binder_transaction(struct binder_proc *proc, return_error_line = __LINE__; goto err_translate_failed; } - last_fixup_obj = parent; + last_fixup_obj_off = parent_offset; last_fixup_min_off = fda->parent_offset + sizeof(u32) * fda->num_fds; } break; @@ -3338,9 +3385,10 @@ static void binder_transaction(struct binder_proc *proc, &target_proc->alloc); 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, - last_fixup_obj, + last_fixup_obj_off, last_fixup_min_off); if (ret < 0) { 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, t->buffer, object_offset, bp, sizeof(*bp)); - last_fixup_obj = t->buffer->data + object_offset; + last_fixup_obj_off = object_offset; last_fixup_min_off = 0; } break; default: -- 2.20.1.791.gb4d0f1c61a-goog