binder: read pre-translated fds from sender buffer

This patch is to prepare for an up coming patch where we read
pre-translated fds from the sender buffer and translate them before
copying them to the target.  It does not change run time.

The patch adds two new parameters to binder_translate_fd_array() to
hold the sender buffer and sender buffer parent.  These parameters let
us call copy_from_user() directly from the sender instead of using
binder_alloc_copy_from_buffer() to copy from the target.  Also the patch
adds some new alignment checks.  Previously the alignment checks would
have been done in a different place, but this lets us print more
useful error messages.

Reviewed-by: Martijn Coenen <maco@android.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211130185152.437403-4-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 73ae3ce..608ff97 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2234,15 +2234,17 @@ err_fd_not_accepted:
 }
 
 static int binder_translate_fd_array(struct binder_fd_array_object *fda,
+                                    const void __user *sender_ubuffer,
                                     struct binder_buffer_object *parent,
+                                    struct binder_buffer_object *sender_uparent,
                                     struct binder_transaction *t,
                                     struct binder_thread *thread,
                                     struct binder_transaction *in_reply_to)
 {
        binder_size_t fdi, fd_buf_size;
        binder_size_t fda_offset;
+       const void __user *sender_ufda_base;
        struct binder_proc *proc = thread->proc;
-       struct binder_proc *target_proc = t->to_proc;
 
        fd_buf_size = sizeof(u32) * fda->num_fds;
        if (fda->num_fds >= SIZE_MAX / sizeof(u32)) {
@@ -2266,7 +2268,10 @@ static int binder_translate_fd_array(struct binder_fd_array_object *fda,
         */
        fda_offset = (parent->buffer - (uintptr_t)t->buffer->user_data) +
                fda->parent_offset;
-       if (!IS_ALIGNED((unsigned long)fda_offset, sizeof(u32))) {
+       sender_ufda_base = (void __user *)sender_uparent->buffer + fda->parent_offset;
+
+       if (!IS_ALIGNED((unsigned long)fda_offset, sizeof(u32)) ||
+           !IS_ALIGNED((unsigned long)sender_ufda_base, sizeof(u32))) {
                binder_user_error("%d:%d parent offset not aligned correctly.\n",
                                  proc->pid, thread->pid);
                return -EINVAL;
@@ -2275,10 +2280,9 @@ static int binder_translate_fd_array(struct binder_fd_array_object *fda,
                u32 fd;
                int ret;
                binder_size_t offset = fda_offset + fdi * sizeof(fd);
+               binder_size_t sender_uoffset = fdi * sizeof(fd);
 
-               ret = binder_alloc_copy_from_buffer(&target_proc->alloc,
-                                                   &fd, t->buffer,
-                                                   offset, sizeof(fd));
+               ret = copy_from_user(&fd, sender_ufda_base + sender_uoffset, sizeof(fd));
                if (!ret)
                        ret = binder_translate_fd(fd, offset, t, thread,
                                                  in_reply_to);
@@ -2951,6 +2955,8 @@ static void binder_transaction(struct binder_proc *proc,
                case BINDER_TYPE_FDA: {
                        struct binder_object ptr_object;
                        binder_size_t parent_offset;
+                       struct binder_object user_object;
+                       size_t user_parent_size;
                        struct binder_fd_array_object *fda =
                                to_binder_fd_array_object(hdr);
                        size_t num_valid = (buffer_offset - off_start_offset) /
@@ -2982,8 +2988,27 @@ static void binder_transaction(struct binder_proc *proc,
                                return_error_line = __LINE__;
                                goto err_bad_parent;
                        }
-                       ret = binder_translate_fd_array(fda, parent, t, thread,
-                                                       in_reply_to);
+                       /*
+                        * We need to read the user version of the parent
+                        * object to get the original user offset
+                        */
+                       user_parent_size =
+                               binder_get_object(proc, user_buffer, t->buffer,
+                                                 parent_offset, &user_object);
+                       if (user_parent_size != sizeof(user_object.bbo)) {
+                               binder_user_error("%d:%d invalid ptr object size: %zd vs %zd\n",
+                                                 proc->pid, thread->pid,
+                                                 user_parent_size,
+                                                 sizeof(user_object.bbo));
+                               return_error = BR_FAILED_REPLY;
+                               return_error_param = -EINVAL;
+                               return_error_line = __LINE__;
+                               goto err_bad_parent;
+                       }
+                       ret = binder_translate_fd_array(fda, user_buffer,
+                                                       parent,
+                                                       &user_object.bbo, t,
+                                                       thread, in_reply_to);
                        if (!ret)
                                ret = binder_alloc_copy_to_buffer(&target_proc->alloc,
                                                                  t->buffer,