08 May, 2019
1 commit
-
…/git/gregkh/char-misc
Pull char/misc update part 2 from Greg KH:
"Here is the "real" big set of char/misc driver patches for 5.2-rc1Loads of different driver subsystem stuff in here, all over the places:
- thunderbolt driver updates
- habanalabs driver updates
- nvmem driver updates
- extcon driver updates
- intel_th driver updates
- mei driver updates
- coresight driver updates
- soundwire driver cleanups and updates
- fastrpc driver updates
- other minor driver updates
- chardev minor fixupsFeels like this tree is getting to be a dumping ground of "small
driver subsystems" these days. Which is fine with me, if it makes
things easier for those subsystem maintainers.All of these have been in linux-next for a while with no reported
issues"* tag 'char-misc-5.2-rc1-part2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: (255 commits)
intel_th: msu: Add current window tracking
intel_th: msu: Add a sysfs attribute to trigger window switch
intel_th: msu: Correct the block wrap detection
intel_th: Add switch triggering support
intel_th: gth: Factor out trace start/stop
intel_th: msu: Factor out pipeline draining
intel_th: msu: Switch over to scatterlist
intel_th: msu: Replace open-coded list_{first,last,next}_entry variants
intel_th: Only report useful IRQs to subdevices
intel_th: msu: Start handling IRQs
intel_th: pci: Use MSI interrupt signalling
intel_th: Communicate IRQ via resource
intel_th: Add "rtit" source device
intel_th: Skip subdevices if their MMIO is missing
intel_th: Rework resource passing between glue layers and core
intel_th: SPDX-ify the documentation
intel_th: msu: Fix single mode with IOMMU
coresight: funnel: Support static funnel
dt-bindings: arm: coresight: Unify funnel DT binding
coresight: replicator: Add new device id for static replicator
...
26 Apr, 2019
1 commit
-
When allocating space in the target buffer for the security context,
make sure the extra_buffers_size doesn't overflow. This can only
happen if the given size is invalid, but an overflow can turn it
into a valid size. Fail the transaction if an overflow is detected.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
25 Apr, 2019
1 commit
-
Restore the behavior of locking mmap_sem for reading in
binder_alloc_free_page(), as was first done in commit 3013bf62b67a
("binder: reduce mmap_sem write-side lock"). That change was
inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
munmap() and direct reclaim").In addition, change the name of the label for the error path to
accurately reflect that we're taking the lock for reading.Backporting note: This fix is only needed when *both* of the commits
mentioned above are applied. That's an unlikely situation since they
both landed during the development of v5.1 but only one of them is
targeted for stable.Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
Signed-off-by: Tyler Hicks
Acked-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
21 Mar, 2019
2 commits
-
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.Signed-off-by: Todd Kjos
Reviewed-by: Joel Fernandes (Google)
Cc: stable
Signed-off-by: Greg Kroah-Hartman -
The selinux-testsuite found an issue resulting in a BUG_ON()
where a conditional relied on a size_t going negative when
checking the validity of a buffer offset.Fixes: 7a67a39320df ("binder: add function to copy binder object from buffer")
Reported-by: Paul Moore
Tested-by: Paul Moore
Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
19 Feb, 2019
1 commit
-
binder has used write-side mmap_sem semaphore to release memory
mapped at address space of the process. However, right lock to
release pages is down_read, not down_write because page table lock
already protects the race for parallel freeing.Please do not use mmap_sem write-side lock which is well known
contented lock.Cc: Todd Kjos
Cc: Martijn Coenen
Cc: Arve Hjønnevåg
Signed-off-by: Minchan Kim
Signed-off-by: Greg Kroah-Hartman
15 Feb, 2019
1 commit
-
Fixes crash found by syzbot:
kernel BUG at drivers/android/binder_alloc.c:LINE! (2)Reported-and-tested-by: syzbot+55de1eb4975dec156d8f@syzkaller.appspotmail.com
Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
14 Feb, 2019
1 commit
-
Fixes sparse issues reported by the kbuild test robot running
on https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-testing: bde4a19fc04f5 ("binder: use userspace pointer as base
of buffer space")Error output (drivers/android/binder_alloc_selftest.c):
sparse: warning: incorrect type in assignment (different address spaces)
sparse: expected void *page_addr
sparse: got void [noderef] *user_data
sparse: error: subtraction of different types can't workFixed by adding necessary "__user" tags.
Reported-by: kbuild test robot
Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
12 Feb, 2019
7 commits
-
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.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
Remove user_buffer_offset since there is no kernel
buffer pointer anymore.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
Remove the kernel's vm_area and the code that maps
buffer pages into it.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
Refactor the functions to validate and fixup struct
binder_buffer pointer objects to avoid using vm_area
pointers. Instead copy to/from kernel space using
binder_alloc_copy_to_buffer() and
binder_alloc_copy_from_buffer(). The following
functions were refactored:refactor binder_validate_ptr()
binder_validate_fixup()
binder_fixup_parent()Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
When creating or tearing down a transaction, the binder driver
examines objects in the buffer and takes appropriate action.
To do this without needing to dereference pointers into the
buffer, the local copies of the objects are needed. This patch
introduces a function to validate and copy binder objects
from the buffer to a local structure.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
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.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
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.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
11 Feb, 2019
1 commit
-
We need the char-misc fixes in here as well.
Signed-off-by: Greg Kroah-Hartman
01 Feb, 2019
1 commit
-
binderfs should not have a separate device_initcall(). When a kernel is
compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
CONFIG_ANDROID_IPC. This use-case is especially sensible when users specify
CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
ANDROID_BINDER_DEVICES="".
When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
regression potential for legacy workloads.Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
30 Jan, 2019
2 commits
-
We currently adhere to the reserved devices limit when creating new
binderfs devices in binderfs instances not located in the inital ipc
namespace. But it is still possible to rob the host instances of their 4
reserved devices by creating the maximum allowed number of devices in a
single binderfs instance located in a non-initial ipc namespace and then
mounting 4 separate binderfs instances in non-initial ipc namespaces. That
happens because the limit is currently not respected for the creation of
the initial binder-control device node. Block this nonsense by performing
the same check in binderfs_binder_ctl_create() that we perform in
binderfs_binder_device_create().Fixes: 36bdf3cae09d ("binderfs: reserve devices for initial mount")
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
Several users have tried to only rely on binderfs to provide binder devices
and set CONFIG_ANDROID_BINDER_DEVICES="" empty. This is a great use-case of
binderfs and one that was always intended to work. However, this is
currently not possible since setting CONFIG_ANDROID_BINDER_DEVICES="" emtpy
will simply panic the kernel:kobject: (00000000028c2f79): attempted to be registered with empty name!
WARNING: CPU: 7 PID: 1703 at lib/kobject.c:228 kobject_add_internal+0x288/0x2b0
Modules linked in: binder_linux(+) bridge stp llc ipmi_ssif gpio_ich dcdbas coretemp kvm_intel kvm irqbypass serio_raw input_leds lpc_ich i5100_edac mac_hid ipmi_si ipmi_devintf ipmi_msghandler sch_fq_codel ib_i
CPU: 7 PID: 1703 Comm: modprobe Not tainted 5.0.0-rc2-brauner-binderfs #263
Hardware name: Dell DCS XS24-SC2 /XS24-SC2 , BIOS S59_3C20 04/07/2011
RIP: 0010:kobject_add_internal+0x288/0x2b0
Code: 12 95 48 c7 c7 78 63 3b 95 e8 77 35 71 ff e9 91 fe ff ff 0f 0b eb a7 0f 0b eb 9a 48 89 de 48 c7 c7 00 63 3b 95 e8 f8 95 6a ff 0b 41 bc ea ff ff ff e9 6d fe ff ff 41 bc fe ff ff ff e9 62 fe
RSP: 0018:ffff973f84237a30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8b53e2472010 RCX: 0000000000000006
RDX: 0000000000000007 RSI: 0000000000000086 RDI: ffff8b53edbd63a0
RBP: ffff973f84237a60 R08: 0000000000000342 R09: 0000000000000004
R10: ffff973f84237af0 R11: 0000000000000001 R12: 0000000000000000
R13: ffff8b53e9f1a1e0 R14: 00000000e9f1a1e0 R15: 0000000000a00037
FS: 00007fbac36f7540(0000) GS:ffff8b53edbc0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fbac364cfa7 CR3: 00000004a6d48000 CR4: 00000000000406e0
Call Trace:
kobject_add+0x71/0xd0
? _cond_resched+0x19/0x40
? mutex_lock+0x12/0x40
device_add+0x12e/0x6b0
device_create_groups_vargs+0xe4/0xf0
device_create_with_groups+0x3f/0x60
? _cond_resched+0x19/0x40
misc_register+0x140/0x180
binder_init+0x1ed/0x2d4 [binder_linux]
? trace_event_define_fields_binder_transaction_fd_send+0x8e/0x8e [binder_linux]
do_one_initcall+0x4a/0x1c9
? _cond_resched+0x19/0x40
? kmem_cache_alloc_trace+0x151/0x1c0
do_init_module+0x5f/0x216
load_module+0x223d/0x2b20
__do_sys_finit_module+0xfc/0x120
? __do_sys_finit_module+0xfc/0x120
__x64_sys_finit_module+0x1a/0x20
do_syscall_64+0x5a/0x120
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fbac3202839
Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffd1494a908 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 000055b629ebec60 RCX: 00007fbac3202839
RDX: 0000000000000000 RSI: 000055b629c20d2e RDI: 0000000000000003
RBP: 000055b629c20d2e R08: 0000000000000000 R09: 000055b629ec2310
R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000000000
R13: 000055b629ebed70 R14: 0000000000040000 R15: 000055b629ebec60So check for the empty string since strsep() will otherwise return the
emtpy string which will cause kobject_add_internal() to panic when trying
to add a kobject with an emtpy name.Fixes: ac4812c5ffbb ("binder: Support multiple /dev instances")
Cc: Martijn Coenen
Signed-off-by: Christian Brauner
Acked-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman
28 Jan, 2019
1 commit
-
We need the char-misc fixes in here as well.
Signed-off-by: Greg Kroah-Hartman
22 Jan, 2019
8 commits
-
To allow servers to verify client identity, allow a node
flag to be set that causes the sender's security context
to be delivered with the transaction. The BR_TRANSACTION
command is extended in BR_TRANSACTION_SEC_CTX to
contain a pointer to the security context string.Signed-off-by: Todd Kjos
Reviewed-by: Joel Fernandes (Google)
Signed-off-by: Greg Kroah-Hartman -
In a previous commit we switched from a d_alloc_name() + d_lookup()
combination to setup a new dentry and find potential duplicates to the more
idiomatic lookup_one_len(). As far as I understand, this also means we need
to switch from d_add() to d_instantiate() since lookup_one_len() will
create a new dentry when it doesn't find an existing one and add the new
dentry to the hash queues. So we only need to call d_instantiate() to
connect the dentry to the inode and turn it into a positive dentry.If we were to use d_add() we sure see stack traces like the following
indicating that adding the same dentry twice over the same inode:[ 744.441889] CPU: 4 PID: 2849 Comm: landscape-sysin Not tainted 5.0.0-rc1-brauner-binderfs #243
[ 744.441889] Hardware name: Dell DCS XS24-SC2 /XS24-SC2 , BIOS S59_3C20 04/07/2011
[ 744.441889] RIP: 0010:__d_lookup_rcu+0x76/0x190
[ 744.441889] Code: 89 75 c0 49 c1 e9 20 49 89 fd 45 89 ce 41 83 e6 07 42 8d 04 f5 00 00 00 00 89 45 c8 eb 0c 48 8b 1b 48 85 db 0f 84 81 00 00 00 8b 63 fc 4c 3b 6b 10 75 ea 48 83 7b 08 00 74 e3 41 83 e4 fe 41
[ 744.441889] RSP: 0018:ffffb8c984e27ad0 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
[ 744.441889] RAX: 0000000000000038 RBX: ffff9407ef770c08 RCX: ffffb8c980011000
[ 744.441889] RDX: ffffb8c984e27b54 RSI: ffffb8c984e27ce0 RDI: ffff9407e6689600
[ 744.441889] RBP: ffffb8c984e27b28 R08: ffffb8c984e27ba4 R09: 0000000000000007
[ 744.441889] R10: ffff9407e5c4f05c R11: 973f3eb9d84a94e5 R12: 0000000000000002
[ 744.441889] R13: ffff9407e6689600 R14: 0000000000000007 R15: 00000007bfef7a13
[ 744.441889] FS: 00007f0db13bb740(0000) GS:ffff9407f3b00000(0000) knlGS:0000000000000000
[ 744.441889] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 744.441889] CR2: 00007f0dacc51024 CR3: 000000032961a000 CR4: 00000000000006e0
[ 744.441889] Call Trace:
[ 744.441889] lookup_fast+0x53/0x300
[ 744.441889] walk_component+0x49/0x350
[ 744.441889] ? inode_permission+0x63/0x1a0
[ 744.441889] link_path_walk.part.33+0x1bc/0x5a0
[ 744.441889] ? path_init+0x190/0x310
[ 744.441889] path_lookupat+0x95/0x210
[ 744.441889] filename_lookup+0xb6/0x190
[ 744.441889] ? __check_object_size+0xb8/0x1b0
[ 744.441889] ? strncpy_from_user+0x50/0x1a0
[ 744.441889] user_path_at_empty+0x36/0x40
[ 744.441889] ? user_path_at_empty+0x36/0x40
[ 744.441889] vfs_statx+0x76/0xe0
[ 744.441889] __do_sys_newstat+0x3d/0x70
[ 744.441889] __x64_sys_newstat+0x16/0x20
[ 744.441889] do_syscall_64+0x5a/0x120
[ 744.441889] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 744.441889] RIP: 0033:0x7f0db0ec2775
[ 744.441889] Code: 00 00 00 75 05 48 83 c4 18 c3 e8 26 55 02 00 66 0f 1f 44 00 00 83 ff 01 48 89 f0 77 30 48 89 c7 48 89 d6 b8 04 00 00 00 0f 05 3d 00 f0 ff ff 77 03 f3 c3 90 48 8b 15 e1 b6 2d 00 f7 d8 64 89
[ 744.441889] RSP: 002b:00007ffc36bc9388 EFLAGS: 00000246 ORIG_RAX: 0000000000000004
[ 744.441889] RAX: ffffffffffffffda RBX: 00007ffc36bc9300 RCX: 00007f0db0ec2775
[ 744.441889] RDX: 00007ffc36bc9400 RSI: 00007ffc36bc9400 RDI: 00007f0dad26f050
[ 744.441889] RBP: 0000000000c0bc60 R08: 0000000000000000 R09: 0000000000000001
[ 744.441889] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc36bc9400
[ 744.441889] R13: 0000000000000001 R14: 00000000ffffff9c R15: 0000000000c0bc60Cc: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
The binderfs_binder_ctl_create() call is a no-op on subsequent calls and
the first call is done before we unlock the suberblock. Hence, there is no
need to take inode_lock() in there. Let's remove it.Suggested-by: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
Al pointed out that first calling kill_litter_super() before cleaning up
info is more correct since destroying info doesn't depend on the state of
the dentries and inodes. That the opposite remains true is not guaranteed.Suggested-by: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
- switch from d_alloc_name() + d_lookup() to lookup_one_len():
Instead of using d_alloc_name() and then doing a d_lookup() with the
allocated dentry to find whether a device with the name we're trying to
create already exists switch to using lookup_one_len(). The latter will
either return the existing dentry or a new one.- switch from kmalloc() + strscpy() to kmemdup():
Use a more idiomatic way to copy the name for the new dentry that
userspace gave us.Suggested-by: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
Al pointed out that on binderfs_fill_super() error
deactivate_locked_super() will call binderfs_kill_super() so all of the
freeing and putting we currently do in binderfs_fill_super() is unnecessary
and buggy. Let's simply return errors and let binderfs_fill_super() take
care of cleaning up on error.Suggested-by: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
- make binderfs control dentry immutable:
We don't allow to unlink it since it is crucial for binderfs to be
useable but if we allow to rename it we make the unlink trivial to
bypass. So prevent renaming too and simply treat the control dentry as
immutable.- add is_binderfs_control_device() helper:
Take the opportunity and turn the check for the control dentry into a
separate helper is_binderfs_control_device() since it's now used in two
places.- simplify binderfs_rename():
Instead of hand-rolling our custom version of simple_rename() just dumb
the whole function down to first check whether we're trying to rename the
control dentry. If we do EPERM the caller and if not call simple_rename().Suggested-by: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
The comment stems from an early version of that patchset and is just
confusing now.Cc: Al Viro
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
18 Jan, 2019
1 commit
-
Fix to return a negative error code -ENOMEM from the new_inode() and
d_make_root() error handling cases instead of 0, as done elsewhere in
this function.Fixes: 849d540ddfcd ("binderfs: implement "max" mount option")
Signed-off-by: Wei Yongjun
Reviewed-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
12 Jan, 2019
1 commit
-
kbuild reported a build faile in [1]. This is triggered when CONFIG_IPC_NS
is not set. So let's make the use of init_ipc_ns conditional on
CONFIG_IPC_NS being set.[1]: https://lists.01.org/pipermail/kbuild-all/2019-January/056903.html
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
11 Jan, 2019
3 commits
-
The binderfs instance in the initial ipc namespace will always have a
reserve of 4 binder devices unless explicitly capped by specifying a lower
value via the "max" mount option.
This ensures when binder devices are removed (on accident or on purpose)
they can always be recreated without risking that all minor numbers have
already been used up.Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
It doesn't make sense to call the header binder_ctl.h when its sole
existence is tied to binderfs. So give it a sensible name. Users will far
more easily remember binderfs.h than binder_ctl.h.Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
Since binderfs can be mounted by userns root in non-initial user namespaces
some precautions are in order. First, a way to set a maximum on the number
of binder devices that can be allocated per binderfs instance and second, a
way to reserve a reasonable chunk of binderfs devices for the initial ipc
namespace.
A first approach as seen in [1] used sysctls similiar to devpts but was
shown to be flawed (cf. [2] and [3]) since some aspects were unneeded. This
is an alternative approach which avoids sysctls completely and instead
switches to a single mount option.Starting with this commit binderfs instances can be mounted with a limit on
the number of binder devices that can be allocated. The max= mount
option serves as a per-instance limit. If max= is set then only
number of binder devices can be allocated in this binderfs
instance.This allows to safely bind-mount binderfs instances into unprivileged user
namespaces since userns root in a non-initial user namespace cannot change
the mount option as long as it does not own the mount namespace the
binderfs mount was created in and hence cannot drain the host of minor
device numbers[1]: https://lore.kernel.org/lkml/20181221133909.18794-1-christian@brauner.io/
[2]; https://lore.kernel.org/lkml/20181221163316.GA8517@kroah.com/
[3]: https://lore.kernel.org/lkml/CAHRSSEx+gDVW4fKKK8oZNAir9G5icJLyodO8hykv3O0O1jt2FQ@mail.gmail.com/
[4]: https://lore.kernel.org/lkml/20181221192044.5yvfnuri7gdop4rs@brauner.io/Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
08 Jan, 2019
2 commits
-
When currently mounting binderfs in the same ipc namespace twice:
mount -t binder binder /A
mount -t binder binder /Bthen the binderfs instances mounted on /A and /B will be the same, i.e.
they will have the same superblock. This was the first approach that seemed
reasonable. However, this leads to some problems and inconsistencies:/* private binderfs instance in same ipc namespace */
There is no way for a user to request a private binderfs instance in the
same ipc namespace.
This request has been made in a private mail to me by two independent
people./* bind-mounts */
If users want the same binderfs instance to appear in multiple places they
can use bind mounts. So there is no value in having a request for a new
binderfs mount giving them the same instance./* unexpected behavior */
It's surprising that request to mount binderfs is not giving the user a new
instance like tmpfs, devpts, ramfs, and others do./* past mistakes */
Other pseudo-filesystems once made the same mistakes of giving back the
same superblock when actually requesting a new mount (cf. devpts's
deprecated "newinstance" option).
We should not make the same mistake. Once we've committed to always giving
back the same superblock in the same IPC namespace with the next kernel
release we will not be able to make that change so better to do it now./* kdbusfs */
It was pointed out to me that kdbusfs - which is conceptually closely
related to binderfs - also allowed users to get a private kdbusfs instance
in the same IPC namespace by making each mount of kdbusfs a separate
instance. I think that makes a lot of sense.Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman -
The binderfs filesystem never needs to be mounted by the kernel itself.
This is conceptually wrong and should never have been done in the first
place.Fixes: 3ad20fe393b ("binder: implement binderfs")
Signed-off-by: Christian Brauner
Signed-off-by: Greg Kroah-Hartman
19 Dec, 2018
2 commits
-
As discussed at Linux Plumbers Conference 2018 in Vancouver [1] this is the
implementation of binderfs./* Abstract */
binderfs is a backwards-compatible filesystem for Android's binder ipc
mechanism. Each ipc namespace will mount a new binderfs instance. Mounting
binderfs multiple times at different locations in the same ipc namespace
will not cause a new super block to be allocated and hence it will be the
same filesystem instance.
Each new binderfs mount will have its own set of binder devices only
visible in the ipc namespace it has been mounted in. All devices in a new
binderfs mount will follow the scheme binder%d and numbering will always
start at 0./* Backwards compatibility */
Devices requested in the Kconfig via CONFIG_ANDROID_BINDER_DEVICES for the
initial ipc namespace will work as before. They will be registered via
misc_register() and appear in the devtmpfs mount. Specifically, the
standard devices binder, hwbinder, and vndbinder will all appear in their
standard locations in /dev. Mounting or unmounting the binderfs mount in
the initial ipc namespace will have no effect on these devices, i.e. they
will neither show up in the binderfs mount nor will they disappear when the
binderfs mount is gone./* binder-control */
Each new binderfs instance comes with a binder-control device. No other
devices will be present at first. The binder-control device can be used to
dynamically allocate binder devices. All requests operate on the binderfs
mount the binder-control device resides in.
Assuming a new instance of binderfs has been mounted at /dev/binderfs
via mount -t binderfs binderfs /dev/binderfs. Then a request to create a
new binder device can be made as illustrated in [2].
Binderfs devices can simply be removed via unlink()./* Implementation details */
- dynamic major number allocation:
When binderfs is registered as a new filesystem it will dynamically
allocate a new major number. The allocated major number will be returned
in struct binderfs_device when a new binder device is allocated.
- global minor number tracking:
Minor are tracked in a global idr struct that is capped at
BINDERFS_MAX_MINOR. The minor number tracker is protected by a global
mutex. This is the only point of contention between binderfs mounts.
- struct binderfs_info:
Each binderfs super block has its own struct binderfs_info that tracks
specific details about a binderfs instance:
- ipc namespace
- dentry of the binder-control device
- root uid and root gid of the user namespace the binderfs instance
was mounted in
- mountable by user namespace root:
binderfs can be mounted by user namespace root in a non-initial user
namespace. The devices will be owned by user namespace root.
- binderfs binder devices without misc infrastructure:
New binder devices associated with a binderfs mount do not use the
full misc_register() infrastructure.
The misc_register() infrastructure can only create new devices in the
host's devtmpfs mount. binderfs does however only make devices appear
under its own mountpoint and thus allocates new character device nodes
from the inode of the root dentry of the super block. This will have
the side-effect that binderfs specific device nodes do not appear in
sysfs. This behavior is similar to devpts allocated pts devices and
has no effect on the functionality of the ipc mechanism itself.[1]: https://goo.gl/JL2tfX
[2]: program to allocate a new binderfs binder device:#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#includeint main(int argc, char *argv[])
{
int fd, ret, saved_errno;
size_t len;
struct binderfs_device device = { 0 };if (argc < 2)
exit(EXIT_FAILURE);len = strlen(argv[1]);
if (len > BINDERFS_MAX_NAME)
exit(EXIT_FAILURE);memcpy(device.name, argv[1], len);
fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open binder-control device\n",
strerror(errno));
exit(EXIT_FAILURE);
}ret = ioctl(fd, BINDER_CTL_ADD, &device);
saved_errno = errno;
close(fd);
errno = saved_errno;
if (ret < 0) {
printf("%s - Failed to allocate new binder device\n",
strerror(errno));
exit(EXIT_FAILURE);
}printf("Allocated new binder device with major %d, minor %d, and "
"name %s\n", device.major, device.minor,
device.name);exit(EXIT_SUCCESS);
}Cc: Martijn Coenen
Cc: Greg Kroah-Hartman
Signed-off-by: Christian Brauner
Acked-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
44d8047f1d8 ("binder: use standard functions to allocate fds")
exposed a pre-existing issue in the binder driver.fdget() is used in ksys_ioctl() as a performance optimization.
One of the rules associated with fdget() is that ksys_close() must
not be called between the fdget() and the fdput(). There is a case
where this requirement is not met in the binder driver which results
in the reference count dropping to 0 when the device is still in
use. This can result in use-after-free or other issues.If userpace has passed a file-descriptor for the binder driver using
a BINDER_TYPE_FDA object, then kys_close() is called on it when
handling a binder_ioctl(BC_FREE_BUFFER) command. This violates
the assumptions for using fdget().The problem is fixed by deferring the close using task_work_add(). A
new variant of __close_fd() was created that returns a struct file
with a reference. The fput() is deferred instead of using ksys_close().Fixes: 44d8047f1d87a ("binder: use standard functions to allocate fds")
Suggested-by: Al Viro
Signed-off-by: Todd Kjos
Cc: stable
Signed-off-by: Greg Kroah-Hartman
06 Dec, 2018
3 commits
-
When dumping out binder transactions via a debug node,
the output is too verbose if a process has many nodes.
Change the output for transaction dumps to only display
nodes with pending async transactions.Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
Fix the incomplete kerneldoc header for struct binder_buffer.
Signed-off-by: Todd Kjos
Signed-off-by: Greg Kroah-Hartman -
We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
such a macro,so remove BINDER_DEBUG_ENTRY.Signed-off-by: Yangtao Li
Acked-by: Todd Kjos
Reviewed-by: Joey Pabalinas
Signed-off-by: Greg Kroah-Hartman
