RE: [PATCH v1] binder: implement binderfs(Internet mail)
> chouryzhou@, can you confirm that this implementation works for your > android-in-container use-case? > > -Todd > We are running Android Pie in container now. If it works for later Android release, it will works for us. - choury ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V4] binder: ipc namespace support for android binder
> On Fri, Nov 16, 2018 at 11:19:41AM -0800, Todd Kjos wrote: > > On Thu, Nov 15, 2018 at 2:54 PM gre...@linuxfoundation.org > > wrote: > > ... > > > > > > A number of us have talked about this in the plumbers Android track, and > > > a different proposal for how to solve this has been made that should be > > > much more resiliant. So I will drop this patch from my queue and wait > > > for the patches based on the discussions we had there. > > > > > > I think there's some notes/slides on the discussion online somewhere, > > > but it hasn't been published as the conference is still happening, > > > otherwise I would link to it here... > > > > Here is a link to the session where you can look at the slides [1]. > > There was consensus that "binderfs" (slide 5) was the most promising > > -- but would be behind a config option to provide backwards > > compatibility for non-container use-cases. > > > > The etherpad notes are at [2] (look at "Dynamically Allocated Binder > > Devices" section) > > > > Christian Brauner will be sending out more details. > > Ok, sorry for the delay I got caught up in other work. > > The idea is to implement binderfs which I'm starting to work on. > binderfs will be a separate pseudo-filesystem that will be mountable > per-ipc namespace. > This has the advantage that the ipc namespace is attached to the > superblock of the mount and can be easily retrieved by the binder > driver. It also implies that - in contrast to the proposed patch here - > an open() on a given binder device will not be able to change the ipc > namespace of said devices. The obvious corollary is that you can > bind-mount binder devices or the whole binderfs mount between different > ipc namespaces and given the right permissions (CAP_IPC_OWNER in the > owning userns of the ipcns) can see services on the host which is > something that people wanted to be able to do. > Additionally, each binderfs mount will come with a binder-control > device. This device is functionally similar to loop-control and will > allow for dynamic allocation (and possibly deallocation) of binder > devices. With this we can remove the restriction to hard-code the number > of binder devices at compile time. > Backwards compatibility can either be guaranteed by hiding binderfs > behind a compile flag or by special-casing the inital binderfs mount to > pre-create the standard binder devices. The jury is still out on this. > > Christian > Since you are working on this, I will withdraw this patch. We will evaluate whether to your solution in our environment after you implement it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V4] binder: ipc namespace support for android binder
> I have not received an answer to my questions in the last version of this > patch > set. Also it would be good if I could be Cc'ed by default. I can't hunt down > all > patches. > I do not know of any kernel entity, specifically devices, that change > namespaces > on open(). > This seems like an invitation for all kinds of security bugs. > A device node belongs to one namespace only which is attached to the > underlying kobject. Opening the device should never change that. > Please look at how mqueue or shm are doing this. They don't change > namespaces on open either. > I have to say that is one of the main reasons why I disagree with that design. > > If we must return the same context when every open in proc, we can only isolate binder with mnt namespace instead of ipc namespace, what do you think, Todd? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V4] binder: ipc namespace support for android binder
Currently android's binder is not isolated by ipc namespace. Since binder
is a form of IPC and therefore should be tied to ipc namespace. With this
patch, we can run multiple instances of android container on one host.
This patch move "binder_procs" and "binder_context" into ipc_namespace,
driver will find the context from it when opening. For debugfs, binder_proc
is namespace-aware, but not for binder dead nodes, binder_stats and
binder_transaction_log_entry (we added ipc inum to trace it).
Signed-off-by: chouryzhou
Reviewed-by: Davidlohr Bueso
---
drivers/android/binder.c | 133 --
include/linux/ipc_namespace.h | 15 +
ipc/namespace.c | 10 +++-
3 files changed, 125 insertions(+), 33 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index cb30a524d16d..453265505b04 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -67,6 +67,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -80,13 +82,21 @@
#include "binder_alloc.h"
#include "binder_trace.h"
+
+#ifndef CONFIG_IPC_NS
+static struct ipc_namespace binder_ipc_ns = {
+ .ns.inum = PROC_IPC_INIT_INO,
+};
+
+#define ipcns (_ipc_ns)
+#else
+#define ipcns (current->nsproxy->ipc_ns)
+#endif
+
static HLIST_HEAD(binder_deferred_list);
static DEFINE_MUTEX(binder_deferred_lock);
static HLIST_HEAD(binder_devices);
-static HLIST_HEAD(binder_procs);
-static DEFINE_MUTEX(binder_procs_lock);
-
static HLIST_HEAD(binder_dead_nodes);
static DEFINE_SPINLOCK(binder_dead_nodes_lock);
@@ -233,6 +243,7 @@ struct binder_transaction_log_entry {
uint32_t return_error;
uint32_t return_error_param;
const char *context_name;
+ unsigned int ipc_inum;
};
struct binder_transaction_log {
atomic_t cur;
@@ -263,19 +274,66 @@ static struct binder_transaction_log_entry
*binder_transaction_log_add(
}
struct binder_context {
+ struct hlist_node hlist;
struct binder_node *binder_context_mgr_node;
struct mutex context_mgr_node_lock;
kuid_t binder_context_mgr_uid;
+ int device;
const char *name;
};
struct binder_device {
struct hlist_node hlist;
struct miscdevice miscdev;
- struct binder_context context;
};
+void binder_exit_ns(struct ipc_namespace *ns)
+{
+ struct binder_context *context;
+ struct hlist_node *tmp;
+
+ mutex_destroy(>binder_procs_lock);
+ hlist_for_each_entry_safe(context, tmp, >binder_contexts, hlist) {
+ mutex_destroy(>context_mgr_node_lock);
+ hlist_del(>hlist);
+ kfree(context);
+ }
+}
+
+int binder_init_ns(struct ipc_namespace *ns)
+{
+ int ret;
+ struct binder_device *device;
+
+ mutex_init(>binder_procs_lock);
+ INIT_HLIST_HEAD(>binder_procs);
+ INIT_HLIST_HEAD(>binder_contexts);
+
+ hlist_for_each_entry(device, _devices, hlist) {
+ struct binder_context *context;
+
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
+ if (!context) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ context->device = device->miscdev.minor;
+ context->name = device->miscdev.name;
+ context->binder_context_mgr_uid = INVALID_UID;
+ mutex_init(>context_mgr_node_lock);
+
+ hlist_add_head(>hlist, >binder_contexts);
+ }
+
+ return 0;
+err:
+ binder_exit_ns(ns);
+ return ret;
+}
+
+
/**
* struct binder_work - work enqueued on a worklist
* @entry: node enqueued on list
@@ -2728,6 +2786,7 @@ static void binder_transaction(struct binder_proc *proc,
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
e->context_name = proc->context->name;
+ e->ipc_inum = ipcns->ns.inum;
if (reply) {
binder_inner_proc_lock(proc);
@@ -4922,6 +4981,7 @@ static int binder_open(struct inode *nodp, struct file
*filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binder_context *context;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -4937,7 +4997,15 @@ static int binder_open(struct inode *nodp, struct file
*filp)
proc->default_priority = task_nice(current);
binder_dev = container_of(filp->private_data, struct binder_device,
miscdev);
- proc->context = _dev->context;
+ hlist_for_each_entry(context, >binder_contexts, hlist) {
+ if (context->device == binder_dev->miscdev.minor) {
+ proc->context = context;
+ break;
+ }
+ }
+ if (!proc->context)
+ return
Re: Re: [PATCH V3] binder: ipc namespace support for android binder(Internet mail)
> > > > If IPC_NS is disabled, "current-nsporxy->ipc_ns" will also exists, it will > > be a static > > reference of "init_ipc_ns" (in ipc/msgutil.c, not defined in binder.c by > > me) with > > no namespace-ization. You will get the same one in all processes, > > everything is > > the same as without this patch. > > except, as far as I can tell, binder_init_ns() would never have been > called on it so the mutex and list heads are not initialized so its > completely broken. Am I missing something? How do those fields get > initialized in this case? > @@ -5832,8 +5888,12 @@ static int __init binder_init(void) > goto err_init_binder_device_failed; > } > > - return ret; > + ret = binder_init_ns(_ipc_ns); > + if (ret) > + goto err_init_namespace_failed; > > + return ret; They are initialized here. - choury - ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH V3] binder: ipc namespace support for android binder(Internet mail)
> > > I still don't understand the dependencies on SYSVIPC or POSIX_MQUEUE. > > > It seems like this mechanism would work even if both are disabled -- > > > as long as IPC_NS is enabled. Seems cleaner to change init/Kconfig and > > > allow IPC_NS if CONFIG_ANDROID_BINDER_IPC and change this line to > > > "#ifndef CONFIG_IPC_NS" > > > > Let me explain it in detail. If SYSIPC and IPC_NS are both defined, > > current->nsproxy->ipc_ns will save the ipc namespace variables. We just use > > it. If SYSIPC (or POSIX_MQUEUE) is defined while IPC_NS is not set, > > current->nsproxy->ipc_ns will always refer to init_ipc_ns in ipc/msgutil.c, > > which is also fine to us. But if neither SYSIPC nor POSIX_MQUEUE is set > > (IPC_NS can't be set in this situation), there is no > > current->nsproxy->ipc_ns. > > We make a fack init_ipc_ns here and use it. > > Yes, I can read the code. I'm wondering specifically about SYSVIPC and > POSIX_MQUEUE. Even with your code changes, binder has no dependency on > these configs. Why are you creating one? The actual dependency with > your changes is on "current->nsproxy->ipc_ns" being initialized for > binder -- which is dependent on CONFIG_IPC_NS being enabled, isn't it? > > If SYSVIPC or POSIX_MQUEUE are enabled, but IPC_NS is disabled, does this > work? If IPC_NS is disabled, "current-nsporxy->ipc_ns" will also exists, it will be a static reference of "init_ipc_ns" (in ipc/msgutil.c, not defined in binder.c by me) with no namespace-ization. You will get the same one in all processes, everything is the same as without this patch. - choury - ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH V3] binder: ipc namespace support for android binder
>
> I still don't understand the dependencies on SYSVIPC or POSIX_MQUEUE.
> It seems like this mechanism would work even if both are disabled --
> as long as IPC_NS is enabled. Seems cleaner to change init/Kconfig and
> allow IPC_NS if CONFIG_ANDROID_BINDER_IPC and change this line to
> "#ifndef CONFIG_IPC_NS"
Let me explain it in detail. If SYSIPC and IPC_NS are both defined,
current->nsproxy->ipc_ns will save the ipc namespace variables. We just use
it. If SYSIPC (or POSIX_MQUEUE) is defined while IPC_NS is not set,
current->nsproxy->ipc_ns will always refer to init_ipc_ns in ipc/msgutil.c,
which is also fine to us. But if neither SYSIPC nor POSIX_MQUEUE is set
(IPC_NS can't be set in this situation), there is no current->nsproxy->ipc_ns.
We make a fack init_ipc_ns here and use it.
> why eliminate name? The string name is very useful for differentiating
> normal "framework" binder transactions vs "hal" or "vendor"
> transactions. If we just have a device number it will be hard to tell
> in the logs even which namespace it belongs to. We need to keep both
> the "name" (for which there might be multiple in each ns) and some
> indication of which namespace this is. Maybe we assign some sort of
> namespace ID during binder_init_ns().
I will remain the name of device. The inum of ipc_ns can be treated as
namespace ID in ipc_ns.
> As mentioned above, we need to retain name and probably also want a ns
> id of some sort. So context now has 3 components if IPC_NS, so maybe a
> helper function to print context like:
>
> static void binder_seq_print_context(struct seq_file *m, struct
> binder_context *context)
> {
> #ifdef CONFIG_IPC_NS
> seq_printf(m, "%d-%d-%s", context->ns_id, context->device,
> context->name);
> #else
> seq_printf(m, "%d", context->name);
> #endif
> }
>
> (same comment below everywhere context is printed)
>
> Should these debugfs nodes should be ns aware and only print debugging
> info for the context of the thread accessing the node? If so, we would
> also want to be namespace-aware when printing pids.
Nowadays, debugfs is not namespace-ized, and pid namespace is not associated
with ipc namespace. Will it be more complicated to debug this if we just print
the info for current thread? Because we will have to enter the ipc namespace
firstly. But add ipc inum should be no problem.
- choury -
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3] binder: ipc namespace support for android binder
We are working for running android in container, but we found that binder is
not isolated by ipc namespace. Since binder is a form of IPC and therefore
should
be tied to ipc namespace. With this patch, we can run more than one android
container on one host.
This patch move "binder_procs" and "binder_context" into ipc_namespace,
driver will find the context from it when opening. Although statistics in
debugfs
remain global.
Signed-off-by: chouryzhou
---
drivers/android/binder.c | 128 +++---
include/linux/ipc_namespace.h | 15 +
ipc/namespace.c | 10 +++-
3 files changed, 117 insertions(+), 36 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index cb30a524d16d..22e45bb937e6 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -68,6 +68,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -80,13 +81,18 @@
#include "binder_alloc.h"
#include "binder_trace.h"
+
+#if !defined(CONFIG_SYSVIPC) && !defined(CONFIG_POSIX_MQUEUE)
+struct ipc_namespace init_ipc_ns;
+#define ipcns (_ipc_ns)
+#else
+#define ipcns (current->nsproxy->ipc_ns)
+#endif
+
static HLIST_HEAD(binder_deferred_list);
static DEFINE_MUTEX(binder_deferred_lock);
static HLIST_HEAD(binder_devices);
-static HLIST_HEAD(binder_procs);
-static DEFINE_MUTEX(binder_procs_lock);
-
static HLIST_HEAD(binder_dead_nodes);
static DEFINE_SPINLOCK(binder_dead_nodes_lock);
@@ -232,7 +238,7 @@ struct binder_transaction_log_entry {
int return_error_line;
uint32_t return_error;
uint32_t return_error_param;
- const char *context_name;
+ int context_device;
};
struct binder_transaction_log {
atomic_t cur;
@@ -263,19 +269,64 @@ static struct binder_transaction_log_entry
*binder_transaction_log_add(
}
struct binder_context {
+ struct hlist_node hlist;
struct binder_node *binder_context_mgr_node;
struct mutex context_mgr_node_lock;
kuid_t binder_context_mgr_uid;
- const char *name;
+ intdevice;
};
struct binder_device {
struct hlist_node hlist;
struct miscdevice miscdev;
- struct binder_context context;
};
+void binder_exit_ns(struct ipc_namespace *ns)
+{
+ struct binder_context *context;
+ struct hlist_node *tmp;
+
+ mutex_destroy(>binder_procs_lock);
+ hlist_for_each_entry_safe(context, tmp, >binder_contexts, hlist) {
+ mutex_destroy(>context_mgr_node_lock);
+ hlist_del(>hlist);
+ kfree(context);
+ }
+}
+
+int binder_init_ns(struct ipc_namespace *ns)
+{
+ int ret;
+ struct binder_device *device;
+
+ mutex_init(>binder_procs_lock);
+ INIT_HLIST_HEAD(>binder_procs);
+ INIT_HLIST_HEAD(>binder_contexts);
+
+ hlist_for_each_entry(device, _devices, hlist) {
+ struct binder_context *context;
+
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
+ if (!context) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ context->device = device->miscdev.minor;
+ context->binder_context_mgr_uid = INVALID_UID;
+ mutex_init(>context_mgr_node_lock);
+
+ hlist_add_head(>hlist, >binder_contexts);
+ }
+
+ return 0;
+err:
+ binder_exit_ns(ns);
+ return ret;
+}
+
+
/**
* struct binder_work - work enqueued on a worklist
* @entry: node enqueued on list
@@ -2727,7 +2778,7 @@ static void binder_transaction(struct binder_proc *proc,
e->target_handle = tr->target.handle;
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
- e->context_name = proc->context->name;
+ e->context_device = proc->context->device;
if (reply) {
binder_inner_proc_lock(proc);
@@ -4922,6 +4973,7 @@ static int binder_open(struct inode *nodp, struct file
*filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binder_context *context;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -4937,7 +4989,15 @@ static int binder_open(struct inode *nodp, struct file
*filp)
proc->default_priority = task_nice(current);
binder_dev = container_of(filp->private_data, struct binder_device,
miscdev);
- proc->context = _dev->context;
+ hlist_for_each_entry(context, >binder_contexts, hlist) {
+ if (context->device == binder_dev->miscdev.minor) {
+ proc->context = context;
+ break;
+ }
+ }
+ if (!proc->context)
+ return -ENOENT;
+
binder_alloc_init(>alloc);
RE: [PATCH V2] binder: ipc namespace support for android binder
> From: Davidlohr Bueso
> Sent: Thursday, November 8, 2018 3:00 PM
>
> On Mon, 29 Oct 2018, chouryzhou(??) wrote:
> >@@ -63,6 +63,12 @@ struct ipc_namespace {
> >unsigned intmq_msg_default;
> >unsigned intmq_msgsize_default;
> >
> >+ /* next fields are for binder */
> >+ struct mutex binder_procs_lock;
> >+ struct hlist_head binder_procs;
> >+ struct mutex binder_contexts_lock;
> >+ struct hlist_head binder_contexts;
>
> I don't think you want a mutex here protecting the binder_contexts list.
> Afaict there is no concurrency going on: you only modify it in when doing
> namespace init and exit (for which you have no serialization); do you even
> need a lock here? Or at least I would think a more lightweight alternative
> (rcu/spinlock/rwlock) would suffice.
Yes, you're right, the binder_contexts is just modified when initing and exiting
namespace, we don't need this lock.
I will update the code you mentioned above.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2] binder: ipc namespace support for android binder
> -Original Message- > From: Andrew Morton > Sent: Thursday, November 8, 2018 6:38 AM > To: chouryzhou(周威) > Cc: gre...@linuxfoundation.org; a...@android.com; tk...@android.com; > d...@stgolabs.net; de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH V2] binder: ipc namespace support for android > binder > > On Wed, 7 Nov 2018 01:48:12 + chouryzhou(周威) > wrote: > > > > > --- a/ipc/namespace.c > > > > +++ b/ipc/namespace.c > > > > @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct > > > user_namespace *user_ns, > > > > ns->ucounts = ucounts; > > > > > > > > err = mq_init_ns(ns); > > > > + if (err) > > > > + goto fail_put; > > > > + err = binder_init_ns(ns); > > > > if (err) > > > > goto fail_put; > > > > > > > > > > Don't we need an mq_put_mnt() if binder_init_ns() fails? > > > > > > free_ipc_ns() seems to have forgotten about that too. In which case it > > > must be madly leaking mounts so probably I'm wrong. Confused. > > > > > > > mq_init_ns will do clean job if it failed, and as do binder_init_ns. > > My point is that if mq_init_ns() succeeds and binder_init_ns() fails, > we don't undo the effects of mq_init_ns()? Oh, mq_put_mnt is called in put_ipc_ns. We should invoke put_ipc_ns if binder_init_ns fails. I will update the patch soon. Thank you very much for pointing out the issue. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2] binder: ipc namespace support for android binder
> -Original Message- > From: Andrew Morton > Sent: Wednesday, November 7, 2018 8:07 AM > To: chouryzhou(周威) > Cc: gre...@linuxfoundation.org; a...@android.com; tk...@android.com; > d...@stgolabs.net; de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH V2] binder: ipc namespace support for android > binder > > On Mon, 29 Oct 2018 06:18:11 + chouryzhou(周威) > wrote: > > > We are working for running android in container, but we found that binder > is > > not isolated by ipc namespace. Since binder is a form of IPC and therefore > should > > be tied to ipc namespace. With this patch, we can run more than one android > > container on one host. > > This patch move "binder_procs" and "binder_context" into > ipc_namespace, > > driver will find the context from it when opening. Although statistics in > debugfs > > remain global. > > > > ... > > > > --- a/ipc/namespace.c > > +++ b/ipc/namespace.c > > @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct > user_namespace *user_ns, > > ns->ucounts = ucounts; > > > > err = mq_init_ns(ns); > > + if (err) > > + goto fail_put; > > + err = binder_init_ns(ns); > > if (err) > > goto fail_put; > > > > Don't we need an mq_put_mnt() if binder_init_ns() fails? > > free_ipc_ns() seems to have forgotten about that too. In which case it > must be madly leaking mounts so probably I'm wrong. Confused. > mq_init_ns will do clean job if it failed, and as do binder_init_ns. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] binder: ipc namespace support for android binder
> > > It's not obvious from this patch where this dependency comes > > > from...why is SYSVIPC required? I'd like to not have to require > > > IPC_NS either for devices. > > > > Yes, the patch is not highly dependent on SYSVIPC, but it will be > > convenient if require it. I will update it to drop dependency of it in > > V2 patch. This patch doesn't need IPC_NS set at present. > > Actually it is dependent on IPC_NS since it makes changes to ipc/namespace.c > which is compiled only if CONFIG_IPC_NS. > Actually it does not require IPC_NS, the code in ipc/namespace.c are namespace specific, and is *not needed* if ipc namespace is not supported. <-- fixed here > There are a couple more implementations similar to this one. > https://lwn.net/Articles/577957/ and some submissions to AOSP derived from > that one that introduce a generic registration function for namespace support > [1], and changes to binder to implement namespaces [2]. > > If this is really needed, then we should have a solution that works for > devices without requiring IPC_NS or SYSVIPC. Also, we should not add > binder-specific code to ipc/namespace.c or include/linux/ipc_namespace.h. > > -Todd > > [1] https://android-review.googlesource.com/c/kernel/common/+/471961 > [2] https://android-review.googlesource.com/c/kernel/common/+/471825 > If the binder will be isolated by namespace, it must put binder proc and binder context in ipc_namespace (or with something like void* as [1] did) I have sent the V2 patch, that patch does not require SYSVIPC or IPC_NS. If IPC_NS is not set, binder_init will put proc and context into init_ipc_ns. If SYSVIPC and CONFIG_POSIX_MQUEUE are both unset, I will make a fake init_ipc_ns to put them. it is marked as no static intentionally to let compile generate an error if it has defined somewhere alse. The code in ipc/namespace.c is just to notify binder to do some installationwhere namespace are creating, If no IPC_NS set, the initialization in binder_init will be enough. So please review and test the V2 patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] binder: ipc namespace support for android binder
> > > It's not obvious from this patch where this dependency comes > > > from...why is SYSVIPC required? I'd like to not have to require > > > IPC_NS either for devices. > > > > Yes, the patch is not highly dependent on SYSVIPC, but it will be > > convenient if require it. I will update it to drop dependency of it in > > V2 patch. This patch doesn't need IPC_NS set at present. > > Actually it is dependent on IPC_NS since it makes changes to ipc/namespace.c > which is compiled only if CONFIG_IPC_NS. > Actually it does not require IPC_NS, the code in ipc/namespace.c are namespace specific, and is *not needed* if ipc namespace is supported. > There are a couple more implementations similar to this one. > https://lwn.net/Articles/577957/ and some submissions to AOSP derived from > that one that introduce a generic registration function for namespace support > [1], and changes to binder to implement namespaces [2]. > > If this is really needed, then we should have a solution that works for > devices without requiring IPC_NS or SYSVIPC. Also, we should not add > binder-specific code to ipc/namespace.c or include/linux/ipc_namespace.h. > > -Todd > > [1] https://android-review.googlesource.com/c/kernel/common/+/471961 > [2] https://android-review.googlesource.com/c/kernel/common/+/471825 > If the binder will be isolated by namespace, it must put binder proc and binder context in ipc_namespace (or with something like void* as [1] did) I have sent the V2 patch, that patch does not require SYSVIPC or IPC_NS. If IPC_NS is not set, binder_init will put proc and context into init_ipc_ns. If SYSVIPC and CONFIG_POSIX_MQUEUE are both unset, I will make a fake init_ipc_ns to put them. it is marked as no static intentionally to let compile generate an error if it has defined somewhere alse. The code in ipc/namespace.c is just to notify binder to do some installationwhere namespace are creating, If no IPC_NS set, the initialization in binder_init will be enough. So please review and test the V2 patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2] binder: ipc namespace support for android binder
We are working for running android in container, but we found that binder is
not isolated by ipc namespace. Since binder is a form of IPC and therefore
should
be tied to ipc namespace. With this patch, we can run more than one android
container on one host.
This patch move "binder_procs" and "binder_context" into ipc_namespace,
driver will find the context from it when opening. Althought statistics in
debugfs
remain global.
Signed-off-by: chouryzhou
---
drivers/android/binder.c | 134 +-
include/linux/ipc_namespace.h | 14
ipc/namespace.c | 4 +
3 files changed, 118 insertions(+), 34 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index cb30a524d16d..98e815e3472d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -68,6 +68,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -80,13 +81,20 @@
#include "binder_alloc.h"
#include "binder_trace.h"
+
+// If init_ipc_ns is not defined elsewhere,
+// we make a fake one here to put our variable.
+#if !defined(CONFIG_SYSVIPC) && !defined(CONFIG_POSIX_MQUEUE)
+struct ipc_namespace init_ipc_ns;
+#define ipcns (_ipc_ns)
+#else
+#define ipcns (current->nsproxy->ipc_ns)
+#endif
+
static HLIST_HEAD(binder_deferred_list);
static DEFINE_MUTEX(binder_deferred_lock);
static HLIST_HEAD(binder_devices);
-static HLIST_HEAD(binder_procs);
-static DEFINE_MUTEX(binder_procs_lock);
-
static HLIST_HEAD(binder_dead_nodes);
static DEFINE_SPINLOCK(binder_dead_nodes_lock);
@@ -232,7 +240,7 @@ struct binder_transaction_log_entry {
int return_error_line;
uint32_t return_error;
uint32_t return_error_param;
- const char *context_name;
+ int context_device;
};
struct binder_transaction_log {
atomic_t cur;
@@ -263,19 +271,66 @@ static struct binder_transaction_log_entry
*binder_transaction_log_add(
}
struct binder_context {
+ struct hlist_node hlist;
struct binder_node *binder_context_mgr_node;
struct mutex context_mgr_node_lock;
kuid_t binder_context_mgr_uid;
- const char *name;
+ intdevice;
};
struct binder_device {
struct hlist_node hlist;
struct miscdevice miscdev;
- struct binder_context context;
};
+void binder_exit_ns(struct ipc_namespace *ns)
+{
+ struct binder_context *context;
+ struct hlist_node *tmp;
+
+ mutex_destroy(>binder_procs_lock);
+ mutex_destroy(>binder_contexts_lock);
+ hlist_for_each_entry_safe(context, tmp, >binder_contexts, hlist) {
+ mutex_destroy(>context_mgr_node_lock);
+ hlist_del(>hlist);
+ kfree(context);
+ }
+}
+
+int binder_init_ns(struct ipc_namespace *ns)
+{
+ int ret;
+ struct binder_device *device;
+
+ mutex_init(>binder_procs_lock);
+ INIT_HLIST_HEAD(>binder_procs);
+ mutex_init(>binder_contexts_lock);
+ INIT_HLIST_HEAD(>binder_contexts);
+
+ hlist_for_each_entry(device, _devices, hlist) {
+ struct binder_context *context;
+
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
+ if (!context) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ context->device = device->miscdev.minor;
+ context->binder_context_mgr_uid = INVALID_UID;
+ mutex_init(>context_mgr_node_lock);
+
+ hlist_add_head(>hlist, >binder_contexts);
+ }
+
+ return 0;
+err:
+ binder_exit_ns(ns);
+ return ret;
+}
+
+
/**
* struct binder_work - work enqueued on a worklist
* @entry: node enqueued on list
@@ -2727,7 +2782,7 @@ static void binder_transaction(struct binder_proc *proc,
e->target_handle = tr->target.handle;
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
- e->context_name = proc->context->name;
+ e->context_device = proc->context->device;
if (reply) {
binder_inner_proc_lock(proc);
@@ -4922,6 +4977,7 @@ static int binder_open(struct inode *nodp, struct file
*filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binder_context *context;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -4937,7 +4993,17 @@ static int binder_open(struct inode *nodp, struct file
*filp)
proc->default_priority = task_nice(current);
binder_dev = container_of(filp->private_data, struct binder_device,
miscdev);
- proc->context = _dev->context;
+ mutex_lock(>binder_contexts_lock);
+ hlist_for_each_entry(context, >binder_contexts, hlist) {
+ if (context->device == binder_dev->miscdev.minor) {
+ proc->context
RE: [PATCH] binder: ipc namespace support for android binder
>> Hi >> We are working for running android in container, but we found that binder >> is >> not isolated by ipc namespace. Since binder is a form of IPC and therefore >> should >> be tied to ipc namespace. With this patch, we can run more than one android >> container on one host. >> This patch move "binder_procs" and "binder_context" into ipc_namespace, >> driver will find the context from it when opening. Althought statistics in >> debugfs >> remain global. >> >> Signed-off-by: choury zhou >> --- >> drivers/android/Kconfig | 2 +- >> drivers/android/binder.c | 126 +- >> include/linux/ipc_namespace.h | 14 >> ipc/namespace.c | 4 ++ >> 4 files changed, 111 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig >> index 432e9ad77070..09883443b2da 100644 >> --- a/drivers/android/Kconfig >> +++ b/drivers/android/Kconfig >> @@ -10,7 +10,7 @@ if ANDROID >> >> config ANDROID_BINDER_IPC >> bool "Android Binder IPC Driver" >> - depends on MMU >> + depends on MMU && SYSVIPC > > NAK. We can't force SYSVIPC on for Android. The notion of running > binder in a container is reasonable, but needs to be done without > explicit requirement for SYSVIPC. binder-in-container is a topic in > the android microconf at Linux plumbers -- are you going to be at LPC? > We have no plan to going to attend LPC temporarily. > It's not obvious from this patch where this dependency comes > from...why is SYSVIPC required? I'd like to not have to require IPC_NS > either for devices. Yes, the patch is not highly dependent on SYSVIPC, but it will be convenient if require it. I will update it to drop dependency of it in V2 patch. This patch doesn't need IPC_NS set at present. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: ipc namespace support for android binder
Hi
We are working for running android in container, but we found that binder is
not isolated by ipc namespace. Since binder is a form of IPC and therefore
should
be tied to ipc namespace. With this patch, we can run more than one android
container on one host.
This patch move "binder_procs" and "binder_context" into ipc_namespace,
driver will find the context from it when opening. Althought statistics in
debugfs
remain global.
Signed-off-by: choury zhou
---
drivers/android/Kconfig | 2 +-
drivers/android/binder.c | 126 +-
include/linux/ipc_namespace.h | 14
ipc/namespace.c | 4 ++
4 files changed, 111 insertions(+), 35 deletions(-)
diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 432e9ad77070..09883443b2da 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -10,7 +10,7 @@ if ANDROID
config ANDROID_BINDER_IPC
bool "Android Binder IPC Driver"
- depends on MMU
+ depends on MMU && SYSVIPC
default n
---help---
Binder is used in Android for both communication between processes,
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d58763b6b009..e061dba9b8b3 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -68,6 +68,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -79,13 +80,12 @@
#include "binder_alloc.h"
#include "binder_trace.h"
+#define ipcns (current->nsproxy->ipc_ns)
+
static HLIST_HEAD(binder_deferred_list);
static DEFINE_MUTEX(binder_deferred_lock);
static HLIST_HEAD(binder_devices);
-static HLIST_HEAD(binder_procs);
-static DEFINE_MUTEX(binder_procs_lock);
-
static HLIST_HEAD(binder_dead_nodes);
static DEFINE_SPINLOCK(binder_dead_nodes_lock);
@@ -231,7 +231,7 @@ struct binder_transaction_log_entry {
int return_error_line;
uint32_t return_error;
uint32_t return_error_param;
- const char *context_name;
+ int context_device;
};
struct binder_transaction_log {
atomic_t cur;
@@ -262,19 +262,66 @@ static struct binder_transaction_log_entry
*binder_transaction_log_add(
}
struct binder_context {
+ struct hlist_node hlist;
struct binder_node *binder_context_mgr_node;
struct mutex context_mgr_node_lock;
kuid_t binder_context_mgr_uid;
- const char *name;
+ intdevice;
};
struct binder_device {
struct hlist_node hlist;
struct miscdevice miscdev;
- struct binder_context context;
};
+void binder_exit_ns(struct ipc_namespace *ns)
+{
+ struct binder_context *context;
+ struct hlist_node *tmp;
+
+ mutex_destroy(>binder_procs_lock);
+ mutex_destroy(>binder_contexts_lock);
+ hlist_for_each_entry_safe(context, tmp, >binder_contexts, hlist) {
+ mutex_destroy(>context_mgr_node_lock);
+ hlist_del(>hlist);
+ kfree(context);
+ }
+}
+
+int binder_init_ns(struct ipc_namespace *ns)
+{
+ int ret;
+ struct binder_device *device;
+
+ mutex_init(>binder_procs_lock);
+ INIT_HLIST_HEAD(>binder_procs);
+ mutex_init(>binder_contexts_lock);
+ INIT_HLIST_HEAD(>binder_contexts);
+
+ hlist_for_each_entry(device, _devices, hlist) {
+ struct binder_context *context;
+
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
+ if (!context) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ context->device = device->miscdev.minor;
+ context->binder_context_mgr_uid = INVALID_UID;
+ mutex_init(>context_mgr_node_lock);
+
+ hlist_add_head(>hlist, >binder_contexts);
+ }
+
+ return 0;
+err:
+ binder_exit_ns(ns);
+ return ret;
+}
+
+
/**
* struct binder_work - work enqueued on a worklist
* @entry: node enqueued on list
@@ -2748,7 +2795,7 @@ static void binder_transaction(struct binder_proc *proc,
e->target_handle = tr->target.handle;
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
- e->context_name = proc->context->name;
+ e->context_device = proc->context->device;
if (reply) {
binder_inner_proc_lock(proc);
@@ -4754,6 +4801,7 @@ static int binder_open(struct inode *nodp, struct file
*filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binder_context *context;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -4770,7 +4818,17 @@ static int binder_open(struct inode *nodp, struct file
*filp)
proc->default_priority = task_nice(current);
binder_dev = container_of(filp->private_data, struct binder_device,
miscdev);
-
