linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Todd Kjos <tkjos@android.com>
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 3/7] binder: add function to copy binder object from buffer
Date: Fri,  8 Feb 2019 10:35:16 -0800	[thread overview]
Message-ID: <20190208183520.30886-4-tkjos@google.com> (raw)
In-Reply-To: <20190208183520.30886-1-tkjos@google.com>

When creating or tearing down a transaction, the binder driver
examines objects in the buffer and takes appropriate action.
To do this without needing to dereference pointers into the
buffer, the local copies of the objects are needed. This patch
introduces a function to validate and copy binder objects
from the buffer to a local structure.

Signed-off-by: Todd Kjos <tkjos@google.com>
---
v2: remove casts as suggested by Dan Carpenter

 drivers/android/binder.c | 75 +++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 17 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 74d0c1ff874e2..8063b405e4fa1 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -628,6 +628,26 @@ struct binder_transaction {
 	spinlock_t lock;
 };
 
+/**
+ * struct binder_object - union of flat binder object types
+ * @hdr:   generic object header
+ * @fbo:   binder object (nodes and refs)
+ * @fdo:   file descriptor object
+ * @bbo:   binder buffer pointer
+ * @fdao:  file descriptor array
+ *
+ * Used for type-independent object copies
+ */
+struct binder_object {
+	union {
+		struct binder_object_header hdr;
+		struct flat_binder_object fbo;
+		struct binder_fd_object fdo;
+		struct binder_buffer_object bbo;
+		struct binder_fd_array_object fdao;
+	};
+};
+
 /**
  * binder_proc_lock() - Acquire outer lock for given binder_proc
  * @proc:         struct binder_proc to acquire
@@ -2017,26 +2037,33 @@ static void binder_cleanup_transaction(struct binder_transaction *t,
 }
 
 /**
- * binder_validate_object() - checks for a valid metadata object in a buffer.
+ * binder_get_object() - gets object and checks for valid metadata
+ * @proc:	binder_proc owning the buffer
  * @buffer:	binder_buffer that we're parsing.
- * @offset:	offset in the buffer at which to validate an object.
+ * @offset:	offset in the @buffer at which to validate an object.
+ * @object:	struct binder_object to read into
  *
  * Return:	If there's a valid metadata object at @offset in @buffer, the
- *		size of that object. Otherwise, it returns zero.
+ *		size of that object. Otherwise, it returns zero. The object
+ *		is read into the struct binder_object pointed to by @object.
  */
-static size_t binder_validate_object(struct binder_buffer *buffer, u64 offset)
+static size_t binder_get_object(struct binder_proc *proc,
+				struct binder_buffer *buffer,
+				unsigned long offset,
+				struct binder_object *object)
 {
-	/* Check if we can read a header first */
+	size_t read_size;
 	struct binder_object_header *hdr;
 	size_t object_size = 0;
 
-	if (buffer->data_size < sizeof(*hdr) ||
-	    offset > buffer->data_size - sizeof(*hdr) ||
-	    !IS_ALIGNED(offset, sizeof(u32)))
+	read_size = min_t(size_t, sizeof(*object), buffer->data_size - offset);
+	if (read_size < sizeof(*hdr))
 		return 0;
+	binder_alloc_copy_from_buffer(&proc->alloc, object, buffer,
+				      offset, read_size);
 
-	/* Ok, now see if we can read a complete object. */
-	hdr = (struct binder_object_header *)(buffer->data + offset);
+	/* Ok, now see if we read a complete object. */
+	hdr = &object->hdr;
 	switch (hdr->type) {
 	case BINDER_TYPE_BINDER:
 	case BINDER_TYPE_WEAK_BINDER:
@@ -2245,6 +2272,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
 	for (offp = off_start; offp < off_end; offp++) {
 		struct binder_object_header *hdr;
 		size_t object_size;
+		struct binder_object object;
 		binder_size_t object_offset;
 		binder_size_t buffer_offset = (uintptr_t)offp -
 			(uintptr_t)buffer->data;
@@ -2252,14 +2280,14 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
 		binder_alloc_copy_from_buffer(&proc->alloc, &object_offset,
 					      buffer, buffer_offset,
 					      sizeof(object_offset));
-		object_size = binder_validate_object(buffer, object_offset);
+		object_size = binder_get_object(proc, buffer,
+						object_offset, &object);
 		if (object_size == 0) {
 			pr_err("transaction release %d bad object at offset %lld, size %zd\n",
 			       debug_id, (u64)object_offset, buffer->data_size);
 			continue;
 		}
-		hdr = (struct binder_object_header *)
-			(buffer->data + object_offset);
+		hdr = &object.hdr;
 		switch (hdr->type) {
 		case BINDER_TYPE_BINDER:
 		case BINDER_TYPE_WEAK_BINDER: {
@@ -3159,6 +3187,7 @@ static void binder_transaction(struct binder_proc *proc,
 	for (; offp < off_end; offp++) {
 		struct binder_object_header *hdr;
 		size_t object_size;
+		struct binder_object object;
 		binder_size_t object_offset;
 		binder_size_t buffer_offset =
 			(uintptr_t)offp - (uintptr_t)t->buffer->data;
@@ -3168,7 +3197,8 @@ static void binder_transaction(struct binder_proc *proc,
 					      t->buffer,
 					      buffer_offset,
 					      sizeof(object_offset));
-		object_size = binder_validate_object(t->buffer, object_offset);
+		object_size = binder_get_object(target_proc, t->buffer,
+						object_offset, &object);
 		if (object_size == 0 || object_offset < off_min) {
 			binder_user_error("%d:%d got transaction with invalid offset (%lld, min %lld max %lld) or object.\n",
 					  proc->pid, thread->pid,
@@ -3181,8 +3211,7 @@ static void binder_transaction(struct binder_proc *proc,
 			goto err_bad_offset;
 		}
 
-		hdr = (struct binder_object_header *)
-			(t->buffer->data + object_offset);
+		hdr = &object.hdr;
 		off_min = object_offset + object_size;
 		switch (hdr->type) {
 		case BINDER_TYPE_BINDER:
@@ -3197,6 +3226,9 @@ static void binder_transaction(struct binder_proc *proc,
 				return_error_line = __LINE__;
 				goto err_translate_failed;
 			}
+			binder_alloc_copy_to_buffer(&target_proc->alloc,
+						    t->buffer, object_offset,
+						    fp, sizeof(*fp));
 		} break;
 		case BINDER_TYPE_HANDLE:
 		case BINDER_TYPE_WEAK_HANDLE: {
@@ -3210,6 +3242,9 @@ static void binder_transaction(struct binder_proc *proc,
 				return_error_line = __LINE__;
 				goto err_translate_failed;
 			}
+			binder_alloc_copy_to_buffer(&target_proc->alloc,
+						    t->buffer, object_offset,
+						    fp, sizeof(*fp));
 		} break;
 
 		case BINDER_TYPE_FD: {
@@ -3226,6 +3261,9 @@ static void binder_transaction(struct binder_proc *proc,
 				goto err_translate_failed;
 			}
 			fp->pad_binder = 0;
+			binder_alloc_copy_to_buffer(&target_proc->alloc,
+						    t->buffer, object_offset,
+						    fp, sizeof(*fp));
 		} break;
 		case BINDER_TYPE_FDA: {
 			struct binder_fd_array_object *fda =
@@ -3310,7 +3348,10 @@ static void binder_transaction(struct binder_proc *proc,
 				return_error_line = __LINE__;
 				goto err_translate_failed;
 			}
-			last_fixup_obj = bp;
+			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_min_off = 0;
 		} break;
 		default:
-- 
2.20.1.791.gb4d0f1c61a-goog


  parent reply	other threads:[~2019-02-08 18:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-08 18:35 [PATCH v3 0/7] binder: eliminate use of vmalloc space for binder buffers Todd Kjos
2019-02-08 18:35 ` [PATCH v3 1/7] binder: create userspace-to-binder-buffer copy function Todd Kjos
2019-02-14 19:45   ` Joel Fernandes
2019-02-14 20:42     ` Todd Kjos
2019-02-14 20:53       ` Joel Fernandes
2019-02-14 21:25         ` Joel Fernandes
2019-02-14 21:55           ` Todd Kjos
2019-02-14 22:07             ` Joel Fernandes
2019-02-08 18:35 ` [PATCH v3 2/7] binder: add functions to copy to/from binder buffers Todd Kjos
2019-02-08 18:35 ` Todd Kjos [this message]
2019-02-08 18:35 ` [PATCH v3 4/7] binder: avoid kernel vm_area for buffer fixups Todd Kjos
2019-02-08 18:35 ` [PATCH v3 5/7] binder: remove kernel vm_area for buffer space Todd Kjos
2019-02-08 18:35 ` [PATCH v3 6/7] binder: remove user_buffer_offset Todd Kjos
2019-02-08 18:35 ` [PATCH v3 7/7] binder: use userspace pointer as base of buffer space Todd Kjos
2019-02-11 16:57 ` [PATCH v3 0/7] binder: eliminate use of vmalloc space for binder buffers Christoph Hellwig
2019-02-11 17:08   ` Todd Kjos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190208183520.30886-4-tkjos@google.com \
    --to=tkjos@android.com \
    --cc=arve@android.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel@joelfernandes.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@google.com \
    --cc=tkjos@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).