lkml.org 
[lkml]   [2020]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 559/717] binder: add flag to clear buffer on txn complete
    Date
    From: Todd Kjos <tkjos@google.com>

    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 <tkjos@google.com>
    Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    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 {

    \
     
     \ /
      Last update: 2020-12-28 15:44    [W:2.229 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site