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>
Bug: 171501513
Change-Id: Ic9338c85cbe3b11ab6f2bda55dce9964bb48447a
(cherry picked from commit 0f966cba95c78029f491b433ea95ff38f414a761)
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
The most common cause of the binder transaction buffer filling up is a
client rapidly firing oneway transactions into a process, before it has
a chance to handle them. Yet the root cause of this is often hard to
debug, because either the system or the app will stop, and by that time
binder debug information we dump in bugreports is no longer relevant.
This change warns as soon as a process dips below 80% of its oneway
space (less than 100kB available in the configuration), when any one
process is responsible for either more than 50 transactions, or more
than 50% of the oneway space.
Signed-off-by: Martijn Coenen <maco@android.com>
Signed-off-by: Martijn Coenen <maco@google.com>
Acked-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20200821122544.1277051-1-maco@android.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 261e7818f06ec51e488e007f787ccd7e77272918
git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/
char-misc-next)
Bug: 147795659
Change-Id: Idc2b03ddc779880ca4716fdae47a70df43211f25
The 4.9.188 merge was missing the change to the
binder driver associated with the linux-4.9.y
commit 16903f1a5b ("coredump: fix race condition
between mmget_not_zero()/get_task_mm() and core dumping").
It was left out because the android-4.9 binder
driver has been significantly refactored compared
to linux-4.9.y.
This patch applies the missing change from that
patch to the binder driver.
Change-Id: I803e558cb629fedff04b14e23ac9f83e98628ede
Fixes: d4fff2d0a879 ("Merge 4.9.188 into android-4.9")
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
commit 5cec2d2e5839f9c0fec319c523a911e0a7fd299f upstream.
An munmap() on a binder device causes binder_vma_close() to be called
which clears the alloc->vma pointer.
If direct reclaim causes binder_alloc_free_page() to be called, there
is a race where alloc->vma is read into a local vma pointer and then
used later after the mm->mmap_sem is acquired. This can result in
calling zap_page_range() with an invalid vma which manifests as a
use-after-free in zap_page_range().
The fix is to check alloc->vma after acquiring the mmap_sem (which we
were acquiring anyway) and skip zap_page_range() if it has changed
to NULL.
Bug: 120025196
Change-Id: I2f3284d294326ec7736303374769640a1e028783
Cc: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Todd Kjos <tkjos@google.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Cc: stable <stable@vger.kernel.org> # 4.14
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that alloc->buffer points to the userspace vm_area
rename buffer->data to buffer->user_data and rename
local pointers that hold user addresses. Also use the
"__user" tag to annotate all user pointers so sparse
can flag cases where user pointer vaues are copied to
kernel pointers. Refactor code to use offsets instead
of user pointers.
(cherry pick from commit bde4a19fc04f5f46298c86b1acb7a4af1d5f138d)
Bug: 67668716
Change-Id: I9d04b844c5994d1f6214da795799e6b373bc9816
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove the kernel's vm_area and the code that maps
buffer pages into it.
(cherry pick from commit 880211667b203dd32724f3be224c44c0400aa0a6)
Bug: 67668716
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Change-Id: I2595bb8416c2bbfcf97ad3d7380ae94e29c209fb
Avoid vm_area when copying to or from binder buffers.
Instead, new copy functions are added that copy from
kernel space to binder buffer space. These use
kmap_atomic() and kunmap_atomic() to create temporary
mappings and then memcpy() is used to copy within
that page.
Also, kmap_atomic() / kunmap_atomic() use the appropriate
cache flushing to support VIVT cache architectures.
Allow binder to build if CPU_CACHE_VIVT is defined.
Several uses of the new functions are added here. More
to follow in subsequent patches.
(cherry picked from commit 8ced0c6231ead26eca8cb416dcb7cc1c2cdd41d8)
Bug: 67668716
Change-Id: I6a93d2396d0a80c352a1d563fc7fb523a753e38c
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The binder driver uses a vm_area to map the per-process
binder buffer space. For 32-bit android devices, this is
now taking too much vmalloc space. This patch removes
the use of vm_area when copying the transaction data
from the sender to the buffer space. Instead of using
copy_from_user() for multi-page copies, it now uses
binder_alloc_copy_user_to_buffer() which uses kmap()
and kunmap() to map each page, and uses copy_from_user()
for copying to that page.
(cherry picked from 1a7c3d9bb7a926e88d5f57643e75ad1abfc55013)
Bug: 67668716
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Change-Id: I59ff83455984fce4626476e30601ed8b99858a92
commit 7bada55ab50697861eee6bb7d60b41e68a961a9c upstream
Malicious code can attempt to free buffers using the BC_FREE_BUFFER
ioctl to binder. There are protections against a user freeing a buffer
while in use by the kernel, however there was a window where
BC_FREE_BUFFER could be used to free a recently allocated buffer that
was not completely initialized. This resulted in a use-after-free
detected by KASAN with a malicious test program.
This window is closed by setting the buffer's allow_user_free attribute
to 0 when the buffer is allocated or when the user has previously freed
it instead of waiting for the caller to set it. The problem was that
when the struct buffer was recycled, allow_user_free was stale and set
to 1 allowing a free to go through.
Bug: 116855682
Change-Id: I0b38089f6fdb1adbf7e1102747e4119c9a05b191
Signed-off-by: Todd Kjos <tkjos@google.com>
Acked-by: Arve Hjønnevåg <arve@android.com>
Cc: stable <stable@vger.kernel.org> # 4.14
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder_update_page_range needs down_write of mmap_sem because
vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless
it is set. However, when I profile binder working, it seems
every binder buffers should be mapped in advance by binder_mmap.
It means we could set VM_MIXEDMAP in binder_mmap time which is
already hold a mmap_sem as down_write so binder_update_page_range
doesn't need to hold a mmap_sem as down_write.
Please use proper API down_read. It would help mmap_sem contention
problem as well as fixing down_write abuse.
Ganesh Mahendran tested app launching and binder throughput test
and he said he couldn't find any problem and I did binder latency
test per Greg KH request(Thanks Martijn to teach me how I can do)
I cannot find any problem, too.
Cc: Ganesh Mahendran <opensource.ganesh@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Todd Kjos <tkjos@google.com>
Reviewed-by: Martijn Coenen <maco@android.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 720c241924046aff83f5f2323232f34a30a4c281)
Change-Id: I8358ceaaab4030f7122c95308dcad59557cad411
VM_IOREMAP is used to access hardware through a mechanism called
I/O mapped memory. Android binder is a IPC machanism which will
not access I/O memory.
And VM_IOREMAP has alignment requiement which may not needed in
binder.
__get_vm_area_node()
{
...
if (flags & VM_IOREMAP)
align = 1ul << clamp_t(int, fls_long(size),
PAGE_SHIFT, IOREMAP_MAX_ORDER);
...
}
This patch will save some kernel vm area, especially for 32bit os.
In 32bit OS, kernel vm area is only 240MB. We may got below
error when launching a app:
<3>[ 4482.440053] binder_alloc: binder_alloc_mmap_handler: 15728 8ce67000-8cf65000 get_vm_area failed -12
<3>[ 4483.218817] binder_alloc: binder_alloc_mmap_handler: 15745 8ce67000-8cf65000 get_vm_area failed -12
Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
Acked-by: Martijn Coenen <maco@android.com>
Acked-by: Todd Kjos <tkjos@google.com>
Cc: stable <stable@vger.kernel.org>
----
V3: update comments
V2: update comments
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit aac6830ec1cb681544212838911cdc57f2638216)
Change-Id: Ide458abc6a4d3ec07973733aa223c4247eef20e6
The function binder_alloc_new_buf_locked() is only used in this file, so
make it static. Also clean up sparse warning:
drivers/android/binder_alloc.c:330:23: warning: no previous prototype
for ‘binder_alloc_new_buf_locked’ [-Wmissing-prototypes]
In addition, the line of the function name exceeds 80 characters when
add static for this function, hence indent its arguments anew.
Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 3f827245463a57f5ef64a665e1ca64eed0da00a5)
Change-Id: I6b379df815d30f9b3e9f1dd50334375123b25bbc
Show the high watermark of the index into the alloc->pages
array, to facilitate sizing the buffer on a per-process
basis.
Change-Id: I2b40cd16628e0ee45216c51dc9b3c5b0c862032e
Signed-off-by: Martijn Coenen <maco@android.com>
(from https://patchwork.kernel.org/patch/9990323/)
Don't access next->data in kernel debug message when the
next buffer is null.
Bug: 36007193
Change-Id: Ib8240d7e9a7087a2256e88c0ae84b9df0f2d0224
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9990321/)
binder_shrinker struct is not used anywhere outside of
binder_alloc.c and should be static.
Bug: 63926541
Change-Id: I7a13d4ddbaaf3721cddfe1d860e34c7be80dd082
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9954125/)
Use binder_alloc struct's mm_struct rather than getting
a reference to the mm struct through get_task_mm to
avoid a potential deadlock between lru lock, task lock and
dentry lock, since a thread can be holding the task lock
and the dentry lock while trying to acquire the lru lock.
Test: ran binderLibTest, throughputtest, interfacetest and
mempressure w/lockdep
Bug: 63926541
Change-Id: Icc661404eb7a4a2ecc5234b1bf8f0104665f9b45
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9954123/)
The vma argument in update_page_range is no longer
used after 74310e06 ("android: binder: Move buffer
out of area shared with user space"), since mmap_handler
no longer calls update_page_range with a vma.
Test: ran binderLibTest, throughputtest, interfacetest and
mempressure w/lockdep
Bug: 36007193
Change-Id: Ibd6f24c11750f8f7e6ed56e40dd18c08e02ace25
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9945123/)
Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.
Use mmput_async when fail to acquire mmap sem
in an atomic context.
Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
Bug: 63926541
Change-Id: I45dbada421b715abed9a66d03d30ae2285671ca1
Fixes: f2517eb76f1f2 ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan <kyan@codeaurora.org>
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928611/)
Add the number of active, lru, and free pages for
each binder process in binder stats
Bug: 63926541
Change-Id: I12618e4eb8ecc08f4f05fe4cba454a88830897f9
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928613/)
Add tracepoints in binder transaction allocator to
record lru hits and alloc/free page.
Bug: 63926541
Change-Id: I2e24fe8e7b6534349df4a87ff865a6843ac9a30b
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928615/)
Hold on to the pages allocated and mapped for transaction
buffers until the system is under memory pressure. When
that happens, use linux shrinker to free pages. Without
using shrinker, patch "android: binder: Move buffer out
of area shared with user space" will cause a significant
slow down for small transactions that fit into the first
page because free list buffer header used to be inlined
with buffer data.
In addition to prevent the performance regression for
small transactions, this patch improves the performance
for transactions that take up more than one page.
Modify alloc selftest to work with the shrinker change.
Test: Run memory intensive applications (Chrome and Camera)
to trigger shrinker callbacks. Binder frees memory as expected.
Test: Run binderThroughputTest with high memory pressure
option enabled.
Bug: 63926541
Change-Id: I3abfc43b405e7e0a6228da37e0689a4b944f0e00
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928607/)
Binder driver allocates buffer meta data in a region that is mapped
in user space. These meta data contain pointers in the kernel.
This patch allocates buffer meta data on the kernel heap that is
not mapped in user space, and uses a pointer to refer to the data mapped.
Also move alloc->buffers initialization from mmap to init since it's
now used even when mmap failed or was not called.
Bug: 36007193
Change-Id: Id5136048bdb7b796f59de066de7ea7df410498f5
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928605/)
Use helper functions buffer_next and buffer_prev instead
of list_entry to get the next and previous buffers.
Bug: 36007193
Change-Id: I422dce84afde3d2138a6d976593b109a9cc49003
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9817797/)
Display information about allocated/free space whenever
binder buffer allocation fails on synchronous
transactions.
Change-Id: Ia718c897dfbfd50991746232ea3c6c9cfab5d1a3
Signed-off-by: Martijn Coenen <maco@android.com>
Signed-off-by: Siqi Lin <siqilin@google.com>
(from https://patchwork.kernel.org/patch/9817815/)
Adds protection against malicious user code freeing
the same buffer at the same time which could cause
a crash. Cannot happen under normal use.
Change-Id: I9461229401aaa7f3b5b2477960f79d4d1bd17fee
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817751/)
Add additional information to determine the cause of binder
failures. Adds the following to failed transaction log and
kernel messages:
return_error : value returned for transaction
return_error_param : errno returned by binder allocator
return_error_line : line number where error detected
Also, return BR_DEAD_REPLY if an allocation error indicates
a dead proc (-ESRCH)
Change-Id: If9f203dee30036003aa1823aaf3f7098f488a3e6
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817753/)
Move the binder allocator functionality to its own file
Continuation of splitting the binder allocator from the binder
driver. Split binder_alloc functions from normal binder functions.
Add kernel doc comments to functions declared extern in
binder_alloc.h
Change-Id: I6a10138d003632ebcb1589f22c7437f2bec8bff6
Signed-off-by: Todd Kjos <tkjos@google.com>
