From nobody Fri Apr 17 10:36:01 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A34BC433FE for ; Wed, 30 Nov 2022 03:58:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232985AbiK3D61 (ORCPT ); Tue, 29 Nov 2022 22:58:27 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42384 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232313AbiK3D6X (ORCPT ); Tue, 29 Nov 2022 22:58:23 -0500 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21B1469DD6 for ; Tue, 29 Nov 2022 19:58:22 -0800 (PST) Received: by mail-pg1-x54a.google.com with SMTP id 69-20020a630148000000b00478118684c4so6768960pgb.20 for ; Tue, 29 Nov 2022 19:58:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=R/mDU43axoS4cq3JBulkYGaq85aE3ZaiGZq+FgpPg9E=; b=HzZeQKtPp3RncupxgwQWA517UvaRhsRSjgANwNpAnbELTwwwOjoMwaCTH4AN+1U4Yv v7dIQ7b9chzgrHjDAn6mbDVlviS0kYeVjq7yP+tOk7bCvxodggOXzt4A0onGwh5LqQi6 ScJ/eXxgkNdt6Q6HD3VKezMCuloUjHjgUNQdqRjHsML2ZGccLKxY8cmKDU0kVcL0V0kM U4KYnQcpy/gq53NGCTPOn3YxWAcpznuK/J7RTuca6wpK2Cp8YiavTatAV7YGk5SaYBh7 j+A3E+OiIH9RTMXYQ4P7lrd3xcXlPRpmhWwqYO7CLhc69eK04TX47ZhC2zGMZvBd41zo uAMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=R/mDU43axoS4cq3JBulkYGaq85aE3ZaiGZq+FgpPg9E=; b=VjLkZ5GUTsPhV7FTLdmM82ExsckSQt1vWtLNqy7N+VR+YQHbd4yeqT2tx54BkPGhXZ 0dkqTeyMcnAOi6FHjYxYGqmKZp0btQ3BVWZYr1anuhLsT7frG1GYqMOQurufoUqTqwTa 1FS1a/APAaq7FdVv6Xooxrug8XVyUmV97pMDp5M+6OBrq984//Ad9058o8AmOsVfjtWp PYGOVuI1UmfhSFrDLC4WklSwq4etNYoXPdJ/ZEF/L4RaoIq3IIeQcpOOmruUfWHSPk4q eoroBdQFyvgDmQwUGJHScmcJcw+ruUVLsjxcw1Pha1b7qRJA7Bm/fgoKpZ1/nIwCyoQ8 d5iA== X-Gm-Message-State: ANoB5pmPqwZ7tVIDOX5eQMies4nfW13Y1wARx2kLy2jvD3/iMJdRumE/ j4WlX8o7BN3rtbsm1bXOv83ASiDYGBcIwg== X-Google-Smtp-Source: AA0mqf7iX+LjrY8qaURMRLX8GrSlfHlXDhbWLj/rYiNtfPv1lOK2lPXJNrczcWwoAVcFK1CAyNohelKbwjLXTA== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a17:90b:1942:b0:219:42d8:bd3a with SMTP id nk2-20020a17090b194200b0021942d8bd3amr10353630pjb.177.1669780701588; Tue, 29 Nov 2022 19:58:21 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:00 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-2-cmllamas@google.com> Subject: [PATCH 5.10 1/6] binder: avoid potential data leakage when copying txn From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan , Brian Swetland Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Todd Kjos , Christian Brauner , Carlos Llamas , Greg Kroah-Hartman Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Todd Kjos commit 6d98eb95b450a75adb4516a1d33652dc78d2b20c upstream. Transactions are copied from the sender to the target first and objects like BINDER_TYPE_PTR and BINDER_TYPE_FDA are then fixed up. This means there is a short period where the sender's version of these objects are visible to the target prior to the fixups. Instead of copying all of the data first, copy data only after any needed fixups have been applied. Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Reviewed-by: Martijn Coenen Acked-by: Christian Brauner Signed-off-by: Todd Kjos Link: https://lore.kernel.org/r/20211130185152.437403-3-tkjos@google.com Signed-off-by: Greg Kroah-Hartman [cmllamas: fix trivial merge conflict] Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 94 ++++++++++++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 24 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index cfb1393a0891..58ab76b7a787 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2008,15 +2008,21 @@ static void binder_cleanup_transaction(struct binde= r_transaction *t, /** * binder_get_object() - gets object and checks for valid metadata * @proc: binder_proc owning the buffer + * @u: sender's user pointer to base of buffer * @buffer: binder_buffer that we're parsing. * @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 + * Copy the binder object at the given offset into @object. If @u is + * provided then the copy is from the sender's buffer. If not, then + * it is copied from the target's @buffer. + * + * Return: If there's a valid metadata object at @offset, the * 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_get_object(struct binder_proc *proc, + const void __user *u, struct binder_buffer *buffer, unsigned long offset, struct binder_object *object) @@ -2026,10 +2032,16 @@ static size_t binder_get_object(struct binder_proc = *proc, size_t object_size =3D 0; =20 read_size =3D min_t(size_t, sizeof(*object), buffer->data_size - offset); - if (offset > buffer->data_size || read_size < sizeof(*hdr) || - binder_alloc_copy_from_buffer(&proc->alloc, object, buffer, - offset, read_size)) + if (offset > buffer->data_size || read_size < sizeof(*hdr)) return 0; + if (u) { + if (copy_from_user(object, u + offset, read_size)) + return 0; + } else { + if (binder_alloc_copy_from_buffer(&proc->alloc, object, buffer, + offset, read_size)) + return 0; + } =20 /* Ok, now see if we read a complete object. */ hdr =3D &object->hdr; @@ -2102,7 +2114,7 @@ static struct binder_buffer_object *binder_validate_p= tr( b, buffer_offset, sizeof(object_offset))) return NULL; - object_size =3D binder_get_object(proc, b, object_offset, object); + object_size =3D binder_get_object(proc, NULL, b, object_offset, object); if (!object_size || object->hdr.type !=3D BINDER_TYPE_PTR) return NULL; if (object_offsetp) @@ -2167,7 +2179,8 @@ static bool binder_validate_fixup(struct binder_proc = *proc, unsigned long buffer_offset; struct binder_object last_object; struct binder_buffer_object *last_bbo; - size_t object_size =3D binder_get_object(proc, b, last_obj_offset, + size_t object_size =3D binder_get_object(proc, NULL, b, + last_obj_offset, &last_object); if (object_size !=3D sizeof(*last_bbo)) return false; @@ -2282,7 +2295,7 @@ static void binder_transaction_buffer_release(struct = binder_proc *proc, if (!binder_alloc_copy_from_buffer(&proc->alloc, &object_offset, buffer, buffer_offset, sizeof(object_offset))) - object_size =3D binder_get_object(proc, buffer, + object_size =3D binder_get_object(proc, NULL, buffer, object_offset, &object); if (object_size =3D=3D 0) { pr_err("transaction release %d bad object at offset %lld, size %zd\n", @@ -2848,6 +2861,7 @@ static void binder_transaction(struct binder_proc *pr= oc, binder_size_t off_start_offset, off_end_offset; binder_size_t off_min; binder_size_t sg_buf_offset, sg_buf_end_offset; + binder_size_t user_offset =3D 0; struct binder_proc *target_proc =3D NULL; struct binder_thread *target_thread =3D NULL; struct binder_node *target_node =3D NULL; @@ -2862,6 +2876,8 @@ static void binder_transaction(struct binder_proc *pr= oc, int t_debug_id =3D atomic_inc_return(&binder_last_id); char *secctx =3D NULL; u32 secctx_sz =3D 0; + const void __user *user_buffer =3D (const void __user *) + (uintptr_t)tr->data.ptr.buffer; =20 e =3D binder_transaction_log_add(&binder_transaction_log); e->debug_id =3D t_debug_id; @@ -3173,19 +3189,6 @@ static void binder_transaction(struct binder_proc *p= roc, t->buffer->clear_on_free =3D !!(t->flags & TF_CLEAR_BUF); trace_binder_transaction_alloc_buf(t->buffer); =20 - if (binder_alloc_copy_user_to_buffer( - &target_proc->alloc, - t->buffer, 0, - (const void __user *) - (uintptr_t)tr->data.ptr.buffer, - tr->data_size)) { - binder_user_error("%d:%d got transaction with invalid data ptr\n", - proc->pid, thread->pid); - return_error =3D BR_FAILED_REPLY; - return_error_param =3D -EFAULT; - return_error_line =3D __LINE__; - goto err_copy_data_failed; - } if (binder_alloc_copy_user_to_buffer( &target_proc->alloc, t->buffer, @@ -3230,6 +3233,7 @@ static void binder_transaction(struct binder_proc *pr= oc, size_t object_size; struct binder_object object; binder_size_t object_offset; + binder_size_t copy_size; =20 if (binder_alloc_copy_from_buffer(&target_proc->alloc, &object_offset, @@ -3241,8 +3245,27 @@ static void binder_transaction(struct binder_proc *p= roc, return_error_line =3D __LINE__; goto err_bad_offset; } - object_size =3D binder_get_object(target_proc, t->buffer, - object_offset, &object); + + /* + * Copy the source user buffer up to the next object + * that will be processed. + */ + copy_size =3D object_offset - user_offset; + if (copy_size && (user_offset > object_offset || + binder_alloc_copy_user_to_buffer( + &target_proc->alloc, + t->buffer, user_offset, + user_buffer + user_offset, + copy_size))) { + binder_user_error("%d:%d got transaction with invalid data ptr\n", + proc->pid, thread->pid); + return_error =3D BR_FAILED_REPLY; + return_error_param =3D -EFAULT; + return_error_line =3D __LINE__; + goto err_copy_data_failed; + } + object_size =3D binder_get_object(target_proc, user_buffer, + t->buffer, object_offset, &object); if (object_size =3D=3D 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, @@ -3254,6 +3277,11 @@ static void binder_transaction(struct binder_proc *p= roc, return_error_line =3D __LINE__; goto err_bad_offset; } + /* + * Set offset to the next buffer fragment to be + * copied + */ + user_offset =3D object_offset + object_size; =20 hdr =3D &object.hdr; off_min =3D object_offset + object_size; @@ -3349,9 +3377,14 @@ static void binder_transaction(struct binder_proc *p= roc, } ret =3D binder_translate_fd_array(fda, parent, t, thread, in_reply_to); - if (ret < 0) { + if (!ret) + ret =3D binder_alloc_copy_to_buffer(&target_proc->alloc, + t->buffer, + object_offset, + fda, sizeof(*fda)); + if (ret) { return_error =3D BR_FAILED_REPLY; - return_error_param =3D ret; + return_error_param =3D ret > 0 ? -EINVAL : ret; return_error_line =3D __LINE__; goto err_translate_failed; } @@ -3421,6 +3454,19 @@ static void binder_transaction(struct binder_proc *p= roc, goto err_bad_object_type; } } + /* Done processing objects, copy the rest of the buffer */ + if (binder_alloc_copy_user_to_buffer( + &target_proc->alloc, + t->buffer, user_offset, + user_buffer + user_offset, + tr->data_size - user_offset)) { + binder_user_error("%d:%d got transaction with invalid data ptr\n", + proc->pid, thread->pid); + return_error =3D BR_FAILED_REPLY; + return_error_param =3D -EFAULT; + return_error_line =3D __LINE__; + goto err_copy_data_failed; + } tcomplete->type =3D BINDER_WORK_TRANSACTION_COMPLETE; t->work.type =3D BINDER_WORK_TRANSACTION; =20 --=20 2.38.1.584.g0f3c55d4c2-goog From nobody Fri Apr 17 10:36:01 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69357C4332F for ; Wed, 30 Nov 2022 03:58:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232638AbiK3D6o (ORCPT ); Tue, 29 Nov 2022 22:58:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42412 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232618AbiK3D6Z (ORCPT ); Tue, 29 Nov 2022 22:58:25 -0500 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 12C5069DD6 for ; Tue, 29 Nov 2022 19:58:24 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id d2-20020a170902cec200b001899479b1d8so6435676plg.22 for ; Tue, 29 Nov 2022 19:58:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=IbgjxXco2ICYz4eUnp8sVnVcMsNs7Pg8hjijnkTmMjU=; b=UHy4vNcQIx9t/WwFNmy5jfp9/nM/BZmdBFjznHBzmbCuI5CcfSueXovUoveXMtM7DO NJ7ycoBF1WcHSkotHoiF0M+1tKeXPLXYfGFmnOamSkfT/TNHOQuwjZfqkhKUArePyvKu g6JWXT6/7LlHK3Z9bsIvcMs9xS23QZCmxsC/LzEmVoApbfgIT2su6wySuvwQKRmoNtIh HgkNnXnqM9nL79zkPMENXD75mI85RZ+LqRxcwOEqBY6jmIr5/OxOibyR8fqdm+1AjonR gGIanVj4M65jME3iVwyk84QuXDGDpnNo4ZeM1e8btTLHftEQEJLxlEGRExrlL73AFyif 1MmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=IbgjxXco2ICYz4eUnp8sVnVcMsNs7Pg8hjijnkTmMjU=; b=lf/sQdaiUXpZHt1W5jmSzolLCBWnbYhn/g7dNWqLfbNGSCBbfuwLtok36dB1mtJM6F hzxCbhrHarW5xKhn+xmHlK85EwfH+jKadOCWMTDbwSxaLqtEudMZxE9xtaeb9AIcS7YX ca57d1trjwHXCYQr+gv7zOcnGnOkp7dlrNODJc2U+LR4A7y0U5gYaArUqZ6WDiPOGcZ5 rJcAWllHgA6RrcHi0DNdEa5z+DRrePOe/mQJj0Kjfu1W8LoDv06+1JS646yPQ3mpbCht ee+AfWF5mYeJP/ojgMzjb5qWF/Vp7kwF7Yw9GaKJpo5mEZ2A04egRPLGUmEnKcwlwtIC wR8g== X-Gm-Message-State: ANoB5pmD/go3WvlI/xliTHYC/VaA86PtXXGMyGrZGehQ/eFy2dgrEXhy See/ZLD+oLIsaDaE3y5i8gX6qXM16nJosA== X-Google-Smtp-Source: AA0mqf5yCfLxL4ypzda97sRkLV0YY9kHvf0KdarnDWdpPS1Ecz4mtuceRBqUWTyuWwSkAqvBbl9dVRN51ZZhUg== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:aa7:9045:0:b0:56c:a321:eecb with SMTP id n5-20020aa79045000000b0056ca321eecbmr61328481pfo.19.1669780703628; Tue, 29 Nov 2022 19:58:23 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:01 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-3-cmllamas@google.com> Subject: [PATCH 5.10 2/6] binder: read pre-translated fds from sender buffer From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Todd Kjos , Christian Brauner , Carlos Llamas Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Todd Kjos commit 656e01f3ab54afe71bed066996fc2640881e1220 upstream. 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 Acked-by: Christian Brauner Signed-off-by: Todd Kjos Link: https://lore.kernel.org/r/20211130185152.437403-4-tkjos@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 58ab76b7a787..4abb621300ec 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2634,15 +2634,17 @@ static int binder_translate_fd(u32 fd, binder_size_= t fd_offset, } =20 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 =3D thread->proc; - struct binder_proc *target_proc =3D t->to_proc; =20 fd_buf_size =3D sizeof(u32) * fda->num_fds; if (fda->num_fds >=3D SIZE_MAX / sizeof(u32)) { @@ -2666,7 +2668,10 @@ static int binder_translate_fd_array(struct binder_f= d_array_object *fda, */ fda_offset =3D (parent->buffer - (uintptr_t)t->buffer->user_data) + fda->parent_offset; - if (!IS_ALIGNED((unsigned long)fda_offset, sizeof(u32))) { + sender_ufda_base =3D (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; @@ -2675,10 +2680,9 @@ static int binder_translate_fd_array(struct binder_f= d_array_object *fda, u32 fd; int ret; binder_size_t offset =3D fda_offset + fdi * sizeof(fd); + binder_size_t sender_uoffset =3D fdi * sizeof(fd); =20 - ret =3D binder_alloc_copy_from_buffer(&target_proc->alloc, - &fd, t->buffer, - offset, sizeof(fd)); + ret =3D copy_from_user(&fd, sender_ufda_base + sender_uoffset, sizeof(fd= )); if (!ret) ret =3D binder_translate_fd(fd, offset, t, thread, in_reply_to); @@ -3344,6 +3348,8 @@ static void binder_transaction(struct binder_proc *pr= oc, 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 =3D to_binder_fd_array_object(hdr); size_t num_valid =3D (buffer_offset - off_start_offset) / @@ -3375,8 +3381,27 @@ static void binder_transaction(struct binder_proc *p= roc, return_error_line =3D __LINE__; goto err_bad_parent; } - ret =3D 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 =3D + binder_get_object(proc, user_buffer, t->buffer, + parent_offset, &user_object); + if (user_parent_size !=3D 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 =3D BR_FAILED_REPLY; + return_error_param =3D -EINVAL; + return_error_line =3D __LINE__; + goto err_bad_parent; + } + ret =3D binder_translate_fd_array(fda, user_buffer, + parent, + &user_object.bbo, t, + thread, in_reply_to); if (!ret) ret =3D binder_alloc_copy_to_buffer(&target_proc->alloc, t->buffer, --=20 2.38.1.584.g0f3c55d4c2-goog From nobody Fri Apr 17 10:36:01 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51CC4C433FE for ; Wed, 30 Nov 2022 03:58:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233147AbiK3D6v (ORCPT ); Tue, 29 Nov 2022 22:58:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232971AbiK3D61 (ORCPT ); Tue, 29 Nov 2022 22:58:27 -0500 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 12D816B3BB for ; Tue, 29 Nov 2022 19:58:26 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id s15-20020a170902ea0f00b0018970bb67bfso12144135plg.3 for ; Tue, 29 Nov 2022 19:58:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=KOsH/bFIWgu5fdNnaSWhJCSXHCJqTEAav0opi71BD+E=; b=F6nLFZcL3O+nHiPT/Z1nsWSDfpXkUulr/bc70ren1qyDMP5v9hFhmWedZxhBnnLcwL mxiXvZajm//FL/F08xyG4LSnQscyXowNVpzujJ1T3DaNUsoZieokW64KrajgR00TWhxU JzdZf37tLITu57EXoE75D/nrJCXzPljm9dx6TwSb2BFHZedf/0ta8Pd5xgqCISHwwroq Wnwd5oGItDglYs8Sdhga5Y4pX1C6N/FkjSMyuO0l2AlTFU8hmpIbelWtpkuAEIeOdOPM 9xjd2njUGucMbdQEavpxfoSwsI+qWBz48GN6ESF5iPb/yPxIzYeScJ6Y4m7R8dCfzqj1 ysmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KOsH/bFIWgu5fdNnaSWhJCSXHCJqTEAav0opi71BD+E=; b=Od8IrDiiSuKaGh/4Fn01L21tdpJeEMIMbq1WeDdowVL0DlS7BetxWmz2KXQbwMA9dg V8FQ/bm2IMzJM/ATLgqNsRW3gLVa4l99DGL0yogc866w19d9xTZI73/xVx6TJKDmZSCa urQzUhS+VFJ1UXYbmSUldBtOlB1h4A1xuZhUC/L9q+KairT/ahCFN70WgjdMYfRTBVDZ E1JiGWnnRylx1FL+MexNwyuRkGxMXXzRw3fTUmkyyhS50TABBkfWnfuyQuHe3Jif5Hvy FCj5oaULUAF3asr7kaFYbw5FfZlw1mLnpIsY5PSJHRkczEBLPfv7Tt3rXOnCRcdusC6w kpaw== X-Gm-Message-State: ANoB5pmPOUAhMlu3IWJZUJ4tM60MnysHKAYFhmgeE2VPNv1RbjkOf7GH bUvmDM1d08kE3BZI0NI8ehYP2Z9wG5ZUVg== X-Google-Smtp-Source: AA0mqf72rABBhLMj5QtpmOvV1l926QDs5RvObIxN8VCCoi+RIcg196OFaxNXEAPmFbw2I91PRwwiIwmBSWW5Dg== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a17:90b:3802:b0:213:48b2:123d with SMTP id mq2-20020a17090b380200b0021348b2123dmr67503222pjb.200.1669780705661; Tue, 29 Nov 2022 19:58:25 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:02 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-4-cmllamas@google.com> Subject: [PATCH 5.10 3/6] binder: defer copies of pre-patched txn data From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Todd Kjos , Carlos Llamas Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Todd Kjos commit 09184ae9b5756cc469db6fd1d1cfdcffbf627c2d upstream. BINDER_TYPE_PTR objects point to memory areas in the source process to be copied into the target buffer as part of a transaction. This implements a scatter- gather model where non-contiguous memory in a source process is "gathered" into a contiguous region in the target buffer. The data can include pointers that must be fixed up to correctly point to the copied data. To avoid making source process pointers visible to the target process, this patch defers the copy until the fixups are known and then copies and fixeups are done together. There is a special case of BINDER_TYPE_FDA which applies the fixup later in the target process context. In this case the user data is skipped (so no untranslated fds become visible to the target). Reviewed-by: Martijn Coenen Signed-off-by: Todd Kjos Link: https://lore.kernel.org/r/20211130185152.437403-5-tkjos@google.com Signed-off-by: Greg Kroah-Hartman [cmllamas: fix trivial merge conflict] Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 299 +++++++++++++++++++++++++++++++++++---- 1 file changed, 274 insertions(+), 25 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 4abb621300ec..83c4501153b4 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2633,7 +2633,246 @@ static int binder_translate_fd(u32 fd, binder_size_= t fd_offset, return ret; } =20 -static int binder_translate_fd_array(struct binder_fd_array_object *fda, +/** + * struct binder_ptr_fixup - data to be fixed-up in target buffer + * @offset offset in target buffer to fixup + * @skip_size bytes to skip in copy (fixup will be written later) + * @fixup_data data to write at fixup offset + * @node list node + * + * This is used for the pointer fixup list (pf) which is created and consu= med + * during binder_transaction() and is only accessed locally. No + * locking is necessary. + * + * The list is ordered by @offset. + */ +struct binder_ptr_fixup { + binder_size_t offset; + size_t skip_size; + binder_uintptr_t fixup_data; + struct list_head node; +}; + +/** + * struct binder_sg_copy - scatter-gather data to be copied + * @offset offset in target buffer + * @sender_uaddr user address in source buffer + * @length bytes to copy + * @node list node + * + * This is used for the sg copy list (sgc) which is created and consumed + * during binder_transaction() and is only accessed locally. No + * locking is necessary. + * + * The list is ordered by @offset. + */ +struct binder_sg_copy { + binder_size_t offset; + const void __user *sender_uaddr; + size_t length; + struct list_head node; +}; + +/** + * binder_do_deferred_txn_copies() - copy and fixup scatter-gather data + * @alloc: binder_alloc associated with @buffer + * @buffer: binder buffer in target process + * @sgc_head: list_head of scatter-gather copy list + * @pf_head: list_head of pointer fixup list + * + * Processes all elements of @sgc_head, applying fixups from @pf_head + * and copying the scatter-gather data from the source process' user + * buffer to the target's buffer. It is expected that the list creation + * and processing all occurs during binder_transaction() so these lists + * are only accessed in local context. + * + * Return: 0=3Dsuccess, else -errno + */ +static int binder_do_deferred_txn_copies(struct binder_alloc *alloc, + struct binder_buffer *buffer, + struct list_head *sgc_head, + struct list_head *pf_head) +{ + int ret =3D 0; + struct binder_sg_copy *sgc, *tmpsgc; + struct binder_ptr_fixup *pf =3D + list_first_entry_or_null(pf_head, struct binder_ptr_fixup, + node); + + list_for_each_entry_safe(sgc, tmpsgc, sgc_head, node) { + size_t bytes_copied =3D 0; + + while (bytes_copied < sgc->length) { + size_t copy_size; + size_t bytes_left =3D sgc->length - bytes_copied; + size_t offset =3D sgc->offset + bytes_copied; + + /* + * We copy up to the fixup (pointed to by pf) + */ + copy_size =3D pf ? min(bytes_left, (size_t)pf->offset - offset) + : bytes_left; + if (!ret && copy_size) + ret =3D binder_alloc_copy_user_to_buffer( + alloc, buffer, + offset, + sgc->sender_uaddr + bytes_copied, + copy_size); + bytes_copied +=3D copy_size; + if (copy_size !=3D bytes_left) { + BUG_ON(!pf); + /* we stopped at a fixup offset */ + if (pf->skip_size) { + /* + * we are just skipping. This is for + * BINDER_TYPE_FDA where the translated + * fds will be fixed up when we get + * to target context. + */ + bytes_copied +=3D pf->skip_size; + } else { + /* apply the fixup indicated by pf */ + if (!ret) + ret =3D binder_alloc_copy_to_buffer( + alloc, buffer, + pf->offset, + &pf->fixup_data, + sizeof(pf->fixup_data)); + bytes_copied +=3D sizeof(pf->fixup_data); + } + list_del(&pf->node); + kfree(pf); + pf =3D list_first_entry_or_null(pf_head, + struct binder_ptr_fixup, node); + } + } + list_del(&sgc->node); + kfree(sgc); + } + BUG_ON(!list_empty(pf_head)); + BUG_ON(!list_empty(sgc_head)); + + return ret > 0 ? -EINVAL : ret; +} + +/** + * binder_cleanup_deferred_txn_lists() - free specified lists + * @sgc_head: list_head of scatter-gather copy list + * @pf_head: list_head of pointer fixup list + * + * Called to clean up @sgc_head and @pf_head if there is an + * error. + */ +static void binder_cleanup_deferred_txn_lists(struct list_head *sgc_head, + struct list_head *pf_head) +{ + struct binder_sg_copy *sgc, *tmpsgc; + struct binder_ptr_fixup *pf, *tmppf; + + list_for_each_entry_safe(sgc, tmpsgc, sgc_head, node) { + list_del(&sgc->node); + kfree(sgc); + } + list_for_each_entry_safe(pf, tmppf, pf_head, node) { + list_del(&pf->node); + kfree(pf); + } +} + +/** + * binder_defer_copy() - queue a scatter-gather buffer for copy + * @sgc_head: list_head of scatter-gather copy list + * @offset: binder buffer offset in target process + * @sender_uaddr: user address in source process + * @length: bytes to copy + * + * Specify a scatter-gather block to be copied. The actual copy must + * be deferred until all the needed fixups are identified and queued. + * Then the copy and fixups are done together so un-translated values + * from the source are never visible in the target buffer. + * + * We are guaranteed that repeated calls to this function will have + * monotonically increasing @offset values so the list will naturally + * be ordered. + * + * Return: 0=3Dsuccess, else -errno + */ +static int binder_defer_copy(struct list_head *sgc_head, binder_size_t off= set, + const void __user *sender_uaddr, size_t length) +{ + struct binder_sg_copy *bc =3D kzalloc(sizeof(*bc), GFP_KERNEL); + + if (!bc) + return -ENOMEM; + + bc->offset =3D offset; + bc->sender_uaddr =3D sender_uaddr; + bc->length =3D length; + INIT_LIST_HEAD(&bc->node); + + /* + * We are guaranteed that the deferred copies are in-order + * so just add to the tail. + */ + list_add_tail(&bc->node, sgc_head); + + return 0; +} + +/** + * binder_add_fixup() - queue a fixup to be applied to sg copy + * @pf_head: list_head of binder ptr fixup list + * @offset: binder buffer offset in target process + * @fixup: bytes to be copied for fixup + * @skip_size: bytes to skip when copying (fixup will be applied later) + * + * Add the specified fixup to a list ordered by @offset. When copying + * the scatter-gather buffers, the fixup will be copied instead of + * data from the source buffer. For BINDER_TYPE_FDA fixups, the fixup + * will be applied later (in target process context), so we just skip + * the bytes specified by @skip_size. If @skip_size is 0, we copy the + * value in @fixup. + * + * This function is called *mostly* in @offset order, but there are + * exceptions. Since out-of-order inserts are relatively uncommon, + * we insert the new element by searching backward from the tail of + * the list. + * + * Return: 0=3Dsuccess, else -errno + */ +static int binder_add_fixup(struct list_head *pf_head, binder_size_t offse= t, + binder_uintptr_t fixup, size_t skip_size) +{ + struct binder_ptr_fixup *pf =3D kzalloc(sizeof(*pf), GFP_KERNEL); + struct binder_ptr_fixup *tmppf; + + if (!pf) + return -ENOMEM; + + pf->offset =3D offset; + pf->fixup_data =3D fixup; + pf->skip_size =3D skip_size; + INIT_LIST_HEAD(&pf->node); + + /* Fixups are *mostly* added in-order, but there are some + * exceptions. Look backwards through list for insertion point. + */ + list_for_each_entry_reverse(tmppf, pf_head, node) { + if (tmppf->offset < pf->offset) { + list_add(&pf->node, &tmppf->node); + return 0; + } + } + /* + * if we get here, then the new offset is the lowest so + * insert at the head + */ + list_add(&pf->node, pf_head); + return 0; +} + +static int binder_translate_fd_array(struct list_head *pf_head, + struct binder_fd_array_object *fda, const void __user *sender_ubuffer, struct binder_buffer_object *parent, struct binder_buffer_object *sender_uparent, @@ -2645,6 +2884,7 @@ static int binder_translate_fd_array(struct binder_fd= _array_object *fda, binder_size_t fda_offset; const void __user *sender_ufda_base; struct binder_proc *proc =3D thread->proc; + int ret; =20 fd_buf_size =3D sizeof(u32) * fda->num_fds; if (fda->num_fds >=3D SIZE_MAX / sizeof(u32)) { @@ -2676,9 +2916,12 @@ static int binder_translate_fd_array(struct binder_f= d_array_object *fda, proc->pid, thread->pid); return -EINVAL; } + ret =3D binder_add_fixup(pf_head, fda_offset, 0, fda->num_fds * sizeof(u3= 2)); + if (ret) + return ret; + for (fdi =3D 0; fdi < fda->num_fds; fdi++) { u32 fd; - int ret; binder_size_t offset =3D fda_offset + fdi * sizeof(fd); binder_size_t sender_uoffset =3D fdi * sizeof(fd); =20 @@ -2692,7 +2935,8 @@ static int binder_translate_fd_array(struct binder_fd= _array_object *fda, return 0; } =20 -static int binder_fixup_parent(struct binder_transaction *t, +static int binder_fixup_parent(struct list_head *pf_head, + struct binder_transaction *t, struct binder_thread *thread, struct binder_buffer_object *bp, binder_size_t off_start_offset, @@ -2738,14 +2982,7 @@ static int binder_fixup_parent(struct binder_transac= tion *t, } buffer_offset =3D bp->parent_offset + (uintptr_t)parent->buffer - (uintptr_t)b->user_data; - if (binder_alloc_copy_to_buffer(&target_proc->alloc, b, buffer_offset, - &bp->buffer, sizeof(bp->buffer))) { - binder_user_error("%d:%d got transaction with invalid parent offset\n", - proc->pid, thread->pid); - return -EINVAL; - } - - return 0; + return binder_add_fixup(pf_head, buffer_offset, bp->buffer, 0); } =20 /** @@ -2880,8 +3117,12 @@ static void binder_transaction(struct binder_proc *p= roc, int t_debug_id =3D atomic_inc_return(&binder_last_id); char *secctx =3D NULL; u32 secctx_sz =3D 0; + struct list_head sgc_head; + struct list_head pf_head; const void __user *user_buffer =3D (const void __user *) (uintptr_t)tr->data.ptr.buffer; + INIT_LIST_HEAD(&sgc_head); + INIT_LIST_HEAD(&pf_head); =20 e =3D binder_transaction_log_add(&binder_transaction_log); e->debug_id =3D t_debug_id; @@ -3398,8 +3639,8 @@ static void binder_transaction(struct binder_proc *pr= oc, return_error_line =3D __LINE__; goto err_bad_parent; } - ret =3D binder_translate_fd_array(fda, user_buffer, - parent, + ret =3D binder_translate_fd_array(&pf_head, fda, + user_buffer, parent, &user_object.bbo, t, thread, in_reply_to); if (!ret) @@ -3431,19 +3672,14 @@ static void binder_transaction(struct binder_proc *= proc, return_error_line =3D __LINE__; goto err_bad_offset; } - if (binder_alloc_copy_user_to_buffer( - &target_proc->alloc, - t->buffer, - sg_buf_offset, - (const void __user *) - (uintptr_t)bp->buffer, - bp->length)) { - binder_user_error("%d:%d got transaction with invalid offsets ptr\n", - proc->pid, thread->pid); - return_error_param =3D -EFAULT; + ret =3D binder_defer_copy(&sgc_head, sg_buf_offset, + (const void __user *)(uintptr_t)bp->buffer, + bp->length); + if (ret) { return_error =3D BR_FAILED_REPLY; + return_error_param =3D ret; return_error_line =3D __LINE__; - goto err_copy_data_failed; + goto err_translate_failed; } /* Fixup buffer pointer to target proc address space */ bp->buffer =3D (uintptr_t) @@ -3452,7 +3688,8 @@ static void binder_transaction(struct binder_proc *pr= oc, =20 num_valid =3D (buffer_offset - off_start_offset) / sizeof(binder_size_t); - ret =3D binder_fixup_parent(t, thread, bp, + ret =3D binder_fixup_parent(&pf_head, t, + thread, bp, off_start_offset, num_valid, last_fixup_obj_off, @@ -3492,6 +3729,17 @@ static void binder_transaction(struct binder_proc *p= roc, return_error_line =3D __LINE__; goto err_copy_data_failed; } + + ret =3D binder_do_deferred_txn_copies(&target_proc->alloc, t->buffer, + &sgc_head, &pf_head); + if (ret) { + binder_user_error("%d:%d got transaction with invalid offsets ptr\n", + proc->pid, thread->pid); + return_error =3D BR_FAILED_REPLY; + return_error_param =3D ret; + return_error_line =3D __LINE__; + goto err_copy_data_failed; + } tcomplete->type =3D BINDER_WORK_TRANSACTION_COMPLETE; t->work.type =3D BINDER_WORK_TRANSACTION; =20 @@ -3558,6 +3806,7 @@ static void binder_transaction(struct binder_proc *pr= oc, err_bad_offset: err_bad_parent: err_copy_data_failed: + binder_cleanup_deferred_txn_lists(&sgc_head, &pf_head); binder_free_txn_fixups(t); trace_binder_transaction_failed_buffer_release(t->buffer); binder_transaction_buffer_release(target_proc, NULL, t->buffer, --=20 2.38.1.584.g0f3c55d4c2-goog From nobody Fri Apr 17 10:36:01 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EE90C433FE for ; Wed, 30 Nov 2022 03:58:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232950AbiK3D64 (ORCPT ); Tue, 29 Nov 2022 22:58:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42520 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232697AbiK3D6b (ORCPT ); Tue, 29 Nov 2022 22:58:31 -0500 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE7C06B384 for ; Tue, 29 Nov 2022 19:58:28 -0800 (PST) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-348608c1cd3so144869137b3.10 for ; Tue, 29 Nov 2022 19:58:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=uianWyO94cQpF/P6HGzSV03i6PkpsS/S96Y6uBo/3Ds=; b=E8mhl3MsUu8J7sEV9cjA5n8kjdZMdvqNIv8yFsHsA6PeKqXy5VCYYgmw89HSnDDLa/ Bem4EzOGEkFQmAo/7/tXsCa1QaQeFap30WRLI2JrzeD7ZAVamY4WY/qrFIVlDWJPP2GX 0r+MTtAS3JhwEWrwAmYq0Qw9ve+og1OpfxlpyeLj/xFv18SGeWkoivPzSFqRoRc8X0y0 3ZyN5XXXrUnedET8Uj016XcYNAHfFEI/HmtjuKQf5s0ABZUqdH1TS4PFqDxnbFAQEXOl tP5GY0K71O+vWgI8dBg0DH4FlZ4lbJetRzKwh3bhb1NdgequVj+4OYokPYhR9O0iOj9l r+2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=uianWyO94cQpF/P6HGzSV03i6PkpsS/S96Y6uBo/3Ds=; b=7eHdkyjVqY0CZWyWeeoxLBpj6cUrLeTXlfcbF/1VoCvbm/ENja3ia7QeU77E7aN8CG hbVAH0zrNccoAPsEwyalxaVVzoQwzL6zC2looXHnvbI67f3CeJUQ3rvyJ3ZZ38mc6LsM A0sBowtUOcFzcUWQkKjvn9GaTK97W6IvXFdw0ClymVe2hsf2jX+eJgLyrMh0SKDFzsWO RQaYNjsjMhSp0YidWqV8mDEjm/8Gd4eTcAsfZ2YO4lFk2k3zcAu2y0DyFcr9fpT+E3HQ nrCouMBFKRG4/1emIIhU+ZJEO1TNfhJnGc/5LQF20hZxUJ4k6l1IJkrbKcThdKIggUTD iJBQ== X-Gm-Message-State: ANoB5pkn3oMlt5rgner8KI1FlgK/jZf8jT9dEX5JY3yF6p/fIi3UCnJG 4gFhWQQ7/MojeWClhHeMYIYGK01DELk/bQ== X-Google-Smtp-Source: AA0mqf4w5pglV1vHvNzYl4KLvK1hn1UvzZu5PRErTMNPxsAeSqOpbhJYWmb+p+kn3nFS/EFIlONMWnu63PsjYQ== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a25:e758:0:b0:6f7:b717:d64d with SMTP id e85-20020a25e758000000b006f7b717d64dmr10263671ybh.236.1669780707969; Tue, 29 Nov 2022 19:58:27 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:03 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-5-cmllamas@google.com> Subject: [PATCH 5.10 4/6] binder: fix pointer cast warning From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Arnd Bergmann , Todd Kjos , Randy Dunlap , Christian Brauner , Carlos Llamas Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Arnd Bergmann commit 9a0a930fe2535a76ad70d3f43caeccf0d86a3009 upstream. binder_uintptr_t is not the same as uintptr_t, so converting it into a pointer requires a second cast: drivers/android/binder.c: In function 'binder_translate_fd_array': drivers/android/binder.c:2511:28: error: cast to pointer from integer of di= fferent size [-Werror=3Dint-to-pointer-cast] 2511 | sender_ufda_base =3D (void __user *)sender_uparent->buffer = + fda->parent_offset; | ^ Fixes: 656e01f3ab54 ("binder: read pre-translated fds from sender buffer") Acked-by: Todd Kjos Acked-by: Randy Dunlap # build-tested Acked-by: Christian Brauner Signed-off-by: Arnd Bergmann Link: https://lore.kernel.org/r/20211207122448.1185769-1-arnd@kernel.org Signed-off-by: Greg Kroah-Hartman Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 83c4501153b4..398ce65b578b 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2908,7 +2908,8 @@ static int binder_translate_fd_array(struct list_head= *pf_head, */ fda_offset =3D (parent->buffer - (uintptr_t)t->buffer->user_data) + fda->parent_offset; - sender_ufda_base =3D (void __user *)sender_uparent->buffer + fda->parent_= offset; + sender_ufda_base =3D (void __user *)(uintptr_t)sender_uparent->buffer + + fda->parent_offset; =20 if (!IS_ALIGNED((unsigned long)fda_offset, sizeof(u32)) || !IS_ALIGNED((unsigned long)sender_ufda_base, sizeof(u32))) { --=20 2.38.1.584.g0f3c55d4c2-goog From nobody Fri Apr 17 10:36:01 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDE15C4332F for ; Wed, 30 Nov 2022 03:59:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233218AbiK3D7B (ORCPT ); Tue, 29 Nov 2022 22:59:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42804 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233039AbiK3D6k (ORCPT ); Tue, 29 Nov 2022 22:58:40 -0500 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 753876C72B for ; Tue, 29 Nov 2022 19:58:30 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id k18-20020a170902c41200b001896d523dc8so12460228plk.19 for ; Tue, 29 Nov 2022 19:58:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=bodxPEkPe4EoxkpER4J2wU9Dhfdb08+KFdFbz39L+wk=; b=kI/n4iRgKxKH5uxHiuPwWoa6M/EbGola0ZxRsu3axly/Gbuc+LNMywjPFOJXNyPkw3 ADTWIa4A64GNx5SmTgzoQZi0Xsxc7CNjghGjRd2NLapRIEdHqcVwMN08uPUFiPGa/rCn zwbLJ5+BTnwTGwyEHTJAgjjTtQLyHp8QOayLbcFCvLIdqQ/FXkBP4QOn6JoMEYwygLo+ rQarpLHPEEfYG4U+9UXjT+qMuWv0EgIah3SS+hjyOBhhOJ1bTxw0ioESPcuKZTNeVCno d+AvT5AXI6j3k9KgY1IMIav8wiGK1Fn5Zf432HCkWpAP68tnvre8Nk+ur0SjSfsnEfTd dqnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bodxPEkPe4EoxkpER4J2wU9Dhfdb08+KFdFbz39L+wk=; b=ANAvaTDc/Ah0gg/8VZM/NgYxN6RXjHA0hEST0Ig1QGYSPO/u5MkD/egetxwqG9xICc m5ejJ0qQo/JhkXZ4XC4VBc0iQZEN7sEe1QaLw2jXYT3g4w9omE8Nl9F4Bi5XGGhC5R9Y ueqkRkRkQ3CbRtJM3Z+IGw0omxBeBvDvtRcCVpatgyv4rTMZnXCrxyPpNmkeUcPQBvMz RSysWx1OvkrstalKoEYOw7QcJi376Ph3+mIq4zawHKp2hObJPaSl0chYTRR3OWnXoRdc 0xx5E+vJkq6dwjHc3fjknhHJ+BTb+7BtZTrkMcRyzIK/O5ZZNPbny2lcI1Hdk2V+EAZ/ hOzA== X-Gm-Message-State: ANoB5pmppQ1iWyZZT5lhmsSh+88lEu6HNx8dJBo0GcsPH588dyuh+Z25 cN1x8hVw9NCXm/4V+pFICsAvT42oU4UlrA== X-Google-Smtp-Source: AA0mqf4oOqP/dHbQlKCbQkAXhBwXW+J2JdNnASKuchZD8PJhWRKB8yudMi3B8jnl0F2vu81s27+XmYmH+O4thw== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a05:6a00:1696:b0:571:2b7c:6693 with SMTP id k22-20020a056a00169600b005712b7c6693mr42366845pfc.48.1669780709932; Tue, 29 Nov 2022 19:58:29 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:04 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-6-cmllamas@google.com> Subject: [PATCH 5.10 5/6] binder: Address corner cases in deferred copy and fixup From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Alessandro Astone , Todd Kjos , Carlos Llamas Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Alessandro Astone commit 2d1746e3fda0c3612143d7c06f8e1d1830c13e23 upstream. When handling BINDER_TYPE_FDA object we are pushing a parent fixup with a certain skip_size but no scatter-gather copy object, since the copy is handled standalone. If BINDER_TYPE_FDA is the last children the scatter-gather copy loop will never stop to skip it, thus we are left with an item in the parent fixup list. This will trigger the BUG_ON(). This is reproducible in android when playing a video. We receive a transaction that looks like this: obj[0] BINDER_TYPE_PTR, parent obj[1] BINDER_TYPE_PTR, child obj[2] BINDER_TYPE_PTR, child obj[3] BINDER_TYPE_FDA, child Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data") Acked-by: Todd Kjos Cc: stable Signed-off-by: Alessandro Astone Link: https://lore.kernel.org/r/20220415120015.52684-2-ales.astone@gmail.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 398ce65b578b..1ba8a98094b4 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2695,6 +2695,7 @@ static int binder_do_deferred_txn_copies(struct binde= r_alloc *alloc, { int ret =3D 0; struct binder_sg_copy *sgc, *tmpsgc; + struct binder_ptr_fixup *tmppf; struct binder_ptr_fixup *pf =3D list_first_entry_or_null(pf_head, struct binder_ptr_fixup, node); @@ -2749,7 +2750,11 @@ static int binder_do_deferred_txn_copies(struct bind= er_alloc *alloc, list_del(&sgc->node); kfree(sgc); } - BUG_ON(!list_empty(pf_head)); + list_for_each_entry_safe(pf, tmppf, pf_head, node) { + BUG_ON(pf->skip_size =3D=3D 0); + list_del(&pf->node); + kfree(pf); + } BUG_ON(!list_empty(sgc_head)); =20 return ret > 0 ? -EINVAL : ret; --=20 2.38.1.584.g0f3c55d4c2-goog From nobody Fri Apr 17 10:36:02 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38F62C4332F for ; Wed, 30 Nov 2022 03:59:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233139AbiK3D7P (ORCPT ); Tue, 29 Nov 2022 22:59:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42824 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233093AbiK3D6m (ORCPT ); Tue, 29 Nov 2022 22:58:42 -0500 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E34ED74630 for ; Tue, 29 Nov 2022 19:58:32 -0800 (PST) Received: by mail-pg1-x54a.google.com with SMTP id x16-20020a63b210000000b0045f5c1e18d0so10828936pge.0 for ; Tue, 29 Nov 2022 19:58:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=wINQ5STM8fgnJ+k0v7AW6rztvA+U+PodJ0UkvqzDm9U=; b=ICqmrKSfyvKNqYSL017ULXFBdPJlBp+v3kahuoV3hqFc0hvHR8ZtKmg/keYdPL4Q+/ jf8QU/Vb3tjdpiQBZH39c1SObLKGIQrDeRLyCLqHrJr12yeLkAQSRaKAnXU3UXokG/y+ L6au2yUNUXZzeBTiQF/GzgRtDsMlhEqkVIIRtsqNfWAa+YOy7lPN9uD/ppow8eXgGRQn zQdDJIqFIC8IUwN/md3/3/Gx4B1CPiIITD93QPvBDENIo5uSSCJkPL0jQ4pQRowljS6T JtkWb6+BDxl3XTrmuKx+h9H5QEDoGNcvQTwd7JmrhT5SRy+4TfCrgFpKjnjFKJ9w17Lm SITA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wINQ5STM8fgnJ+k0v7AW6rztvA+U+PodJ0UkvqzDm9U=; b=z5+vSyzEU0aY0YOG0+s5yrLRnDU3To+K+m6OgUTNXWJ9pNNWuOSyVdr4SNy48tasg8 59nDrgMVrIu2RNO2joujRN94REZWn8//cTh3PstLs2IdB6oACjSs+67fJERSWo1Tpt79 mLi6uCztq6WJv7plDUmRdbxWu3JZtPdih3rh03auh6qks3ZytUglSN/LHO8rkKFW/LC5 Sct9mKnRGB5M74Ytf7f9fJzVkUdVVM45pNNC9jRW0vTNKN7+OXCYWUTTLgmpwlgWBgzG +cLJpnpw+0nK9q/gOetJHjxbBUPcWFIoo1HoOMLewzFDqvfWOm0NRZUkSjFHQKrYCp2W /yHw== X-Gm-Message-State: ANoB5pn/7dFWAWr5iqjF7bxliQwHzO9UzDGcdu/AdDMUrB5dTKiPpQLB h+Sq3mLFHnDBKIWaldzvyXG4+O8P+xcM8g== X-Google-Smtp-Source: AA0mqf46VOSPVyV8gpFA1NdY3rdJLYYeVdc/9MzUk4PZPVcYJm4qOZVHAiCwCdinTlUp7WknzNnEXaosXFboQg== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:aa7:9534:0:b0:575:c857:edc0 with SMTP id c20-20020aa79534000000b00575c857edc0mr3112444pfp.22.1669780711857; Tue, 29 Nov 2022 19:58:31 -0800 (PST) Date: Wed, 30 Nov 2022 03:58:05 +0000 In-Reply-To: <20221130035805.1823970-1-cmllamas@google.com> Mime-Version: 1.0 References: <20221130035805.1823970-1-cmllamas@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130035805.1823970-7-cmllamas@google.com> Subject: [PATCH 5.10 6/6] binder: Gracefully handle BINDER_TYPE_FDA objects with num_fds=0 From: Carlos Llamas To: stable@kernel.org, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Alessandro Astone , Todd Kjos , Carlos Llamas Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Alessandro Astone commit ef38de9217a04c9077629a24652689d8fdb4c6c6 upstream. Some android userspace is sending BINDER_TYPE_FDA objects with num_fds=3D0. Like the previous patch, this is reproducible when playing a video. Before commit 09184ae9b575 BINDER_TYPE_FDA objects with num_fds=3D0 were 'correctly handled', as in no fixup was performed. After commit 09184ae9b575 we aggregate fixup and skip regions in binder_ptr_fixup structs and distinguish between the two by using the skip_size field: if it's 0, then it's a fixup, otherwise skip. When processing BINDER_TYPE_FDA objects with num_fds=3D0 we add a skip region of skip_size=3D0, and this causes issues because now binder_do_deferred_txn_copies will think this was a fixup region. To address that, return early from binder_translate_fd_array to avoid adding an empty skip region. Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data") Acked-by: Todd Kjos Cc: stable Signed-off-by: Alessandro Astone Link: https://lore.kernel.org/r/20220415120015.52684-1-ales.astone@gmail.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Carlos Llamas --- drivers/android/binder.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 1ba8a98094b4..4473adef2f5a 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2891,6 +2891,9 @@ static int binder_translate_fd_array(struct list_head= *pf_head, struct binder_proc *proc =3D thread->proc; int ret; =20 + if (fda->num_fds =3D=3D 0) + return 0; + fd_buf_size =3D sizeof(u32) * fda->num_fds; if (fda->num_fds >=3D SIZE_MAX / sizeof(u32)) { binder_user_error("%d:%d got transaction with invalid number of fds (%ll= d)\n", --=20 2.38.1.584.g0f3c55d4c2-goog