From patchwork Mon Dec 28 12:49:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 352685 Delivered-To: patch@linaro.org Received: by 2002:a02:85a7:0:0:0:0:0 with SMTP id d36csp9799856jai; Mon, 28 Dec 2020 06:46:45 -0800 (PST) X-Google-Smtp-Source: ABdhPJx9uT7quZQS0Hr/BUYEutWujxfzwtQ6BwcaH1tai+MdGfofc+9GQeC7MpkyVfkVWJqFxJTk X-Received: by 2002:a50:8b02:: with SMTP id l2mr27947591edl.322.1609166716937; Mon, 28 Dec 2020 06:45:16 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1609166716; cv=none; d=google.com; s=arc-20160816; b=T2sV5w9EpM5TZDJRK+Abn1j8anDKfo2lsEgR2n22i0fDVVX/erRMTFgdYcTyNODn0n 5bomWBSj9eZ5fyIJ9BdcnLG2eU9Nse3X77xV3928cUa4PuYmGWzW8KFlhVeOahjv0WkA nJG/k+K+m2sJrRq7ksvhD9Vr2jq5VwHnvLmnODFD7UqUwCcbFlxQwBh6z+1StB3uBrE1 KLFVVNqY0F5ie2edClQEmWFz40alUJAdVq2TEVosHxc+IT/uiIPTSmrwzjSdKacy0aLJ sh+yS8Egl9AVL+VmTY1Mha531xrR9OLdf9tl+qPUJHnKEflTfspqXcpt2VuFHPid0zNB sFbw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=VcEX0TvNGyMaM22MpNY5a/N7kaablR4CbuhqhKIhdck=; b=TN60vXkBwZBBIpjEXA5BzZkmc+ZiAbQb+Mk8kppQ+9g7AacLOB10YlU5xDf8IFxQ2l FuJdVvQmiQlyyCliFl5pH36ZikLORfik6zkHwnMCCxuVMzPqpUxoZR1zhs9k+4yB9Y7R p/KVFp7i3+MDro5gEJAC8/+54YHM+R8U1g3eHtBiTNDayb5VSIlm0bnIs1N9l3Vkot9l /q9kbr0UnATDLIoA09Zig12IUmIZOaxmGBjfb3DUfHTDI/XSVQKnp80Smxa6NcWPoL5J wjqVE4troFwm/HpMpfFhc3bnEnMjMU06mWf4XZIyP+QxEIMqXe+ENK9fpckkq+7LeKf8 lOTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=sUypuMdj; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id ck15si19656600edb.37.2020.12.28.06.45.16; Mon, 28 Dec 2020 06:45:16 -0800 (PST) Received-SPF: pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=sUypuMdj; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2503735AbgL1O0L (ORCPT + 14 others); Mon, 28 Dec 2020 09:26:11 -0500 Received: from mail.kernel.org ([198.145.29.99]:33738 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2503331AbgL1OZz (ORCPT ); Mon, 28 Dec 2020 09:25:55 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id B8DB022AEC; Mon, 28 Dec 2020 14:25:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1609165514; bh=R7hPqRPRgIIrkzn0lvGXXPQkYZa7EbjSF48MeX3NhJA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sUypuMdjJ/m4NQYG1MYkiFhLVMmFUTjjwHU7rowJ3cOFB3izt+fH9dgRZw1oiQbiv WhYCO/NyVRBNlL5nrG0qYs0EAGEDWSHjxGsnSpi4BwQ/WSxBxjyLpezkgdyTcMpgfh ecKUsVPV4sHmtT3ICplCy2Ycr5MmzGxUO9/Nm3ls= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Todd Kjos Subject: [PATCH 5.10 559/717] binder: add flag to clear buffer on txn complete Date: Mon, 28 Dec 2020 13:49:17 +0100 Message-Id: <20201228125047.705472163@linuxfoundation.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201228125020.963311703@linuxfoundation.org> References: <20201228125020.963311703@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Todd Kjos commit 0f966cba95c78029f491b433ea95ff38f414a761 upstream. Add a per-transaction flag to indicate that the buffer must be cleared when the transaction is complete to prevent copies of sensitive data from being preserved in memory. Signed-off-by: Todd Kjos Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 1 drivers/android/binder_alloc.c | 48 ++++++++++++++++++++++++++++++++++++ drivers/android/binder_alloc.h | 4 ++- include/uapi/linux/android/binder.h | 1 4 files changed, 53 insertions(+), 1 deletion(-) --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -3146,6 +3146,7 @@ static void binder_transaction(struct bi t->buffer->debug_id = t->debug_id; t->buffer->transaction = t; t->buffer->target_node = target_node; + t->buffer->clear_on_free = !!(t->flags & TF_CLEAR_BUF); trace_binder_transaction_alloc_buf(t->buffer); if (binder_alloc_copy_user_to_buffer( --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -696,6 +696,8 @@ static void binder_free_buf_locked(struc binder_insert_free_buffer(alloc, buffer); } +static void binder_alloc_clear_buf(struct binder_alloc *alloc, + struct binder_buffer *buffer); /** * binder_alloc_free_buf() - free a binder buffer * @alloc: binder_alloc for this proc @@ -706,6 +708,18 @@ static void binder_free_buf_locked(struc void binder_alloc_free_buf(struct binder_alloc *alloc, struct binder_buffer *buffer) { + /* + * We could eliminate the call to binder_alloc_clear_buf() + * from binder_alloc_deferred_release() by moving this to + * binder_alloc_free_buf_locked(). However, that could + * increase contention for the alloc mutex if clear_on_free + * is used frequently for large buffers. The mutex is not + * needed for correctness here. + */ + if (buffer->clear_on_free) { + binder_alloc_clear_buf(alloc, buffer); + buffer->clear_on_free = false; + } mutex_lock(&alloc->mutex); binder_free_buf_locked(alloc, buffer); mutex_unlock(&alloc->mutex); @@ -802,6 +816,10 @@ void binder_alloc_deferred_release(struc /* Transaction should already have been freed */ BUG_ON(buffer->transaction); + if (buffer->clear_on_free) { + binder_alloc_clear_buf(alloc, buffer); + buffer->clear_on_free = false; + } binder_free_buf_locked(alloc, buffer); buffers++; } @@ -1136,6 +1154,36 @@ static struct page *binder_alloc_get_pag } /** + * binder_alloc_clear_buf() - zero out buffer + * @alloc: binder_alloc for this proc + * @buffer: binder buffer to be cleared + * + * memset the given buffer to 0 + */ +static void binder_alloc_clear_buf(struct binder_alloc *alloc, + struct binder_buffer *buffer) +{ + size_t bytes = binder_alloc_buffer_size(alloc, buffer); + binder_size_t buffer_offset = 0; + + while (bytes) { + unsigned long size; + struct page *page; + pgoff_t pgoff; + void *kptr; + + page = binder_alloc_get_page(alloc, buffer, + buffer_offset, &pgoff); + size = min_t(size_t, bytes, PAGE_SIZE - pgoff); + kptr = kmap(page) + pgoff; + memset(kptr, 0, size); + kunmap(page); + bytes -= size; + buffer_offset += size; + } +} + +/** * binder_alloc_copy_user_to_buffer() - copy src user to tgt user * @alloc: binder_alloc for this proc * @buffer: binder buffer to be accessed --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -23,6 +23,7 @@ struct binder_transaction; * @entry: entry alloc->buffers * @rb_node: node for allocated_buffers/free_buffers rb trees * @free: %true if buffer is free + * @clear_on_free: %true if buffer must be zeroed after use * @allow_user_free: %true if user is allowed to free buffer * @async_transaction: %true if buffer is in use for an async txn * @debug_id: unique ID for debugging @@ -41,9 +42,10 @@ struct binder_buffer { struct rb_node rb_node; /* free entry by size or allocated entry */ /* by address */ unsigned free:1; + unsigned clear_on_free:1; unsigned allow_user_free:1; unsigned async_transaction:1; - unsigned debug_id:29; + unsigned debug_id:28; struct binder_transaction *transaction; --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -248,6 +248,7 @@ enum transaction_flags { TF_ROOT_OBJECT = 0x04, /* contents are the component's root object */ TF_STATUS_CODE = 0x08, /* contents are a 32-bit status code */ TF_ACCEPT_FDS = 0x10, /* allow replies with file descriptors */ + TF_CLEAR_BUF = 0x20, /* clear buffer on txn complete */ }; struct binder_transaction_data {