linux-kernel - Re: Re: [PATCH V3] binder: ipc namespace support for android binder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5FBCBE569E134E4CA167B91C0A77FD610198F8A217@EXMBX-SZMAIL022.tencent.com>
Date:   Sat, 10 Nov 2018 03:09:34 +0000
From:   chouryzhou(周威) <chouryzhou@...cent.com>
To:     tkjos <tkjos@...gle.com>
CC:     gregkh <gregkh@...uxfoundation.org>, arve <arve@...roid.com>,
        tkjos <tkjos@...roid.com>, akpm <akpm@...ux-foundation.org>,
        dave <dave@...olabs.net>, devel <devel@...verdev.osuosl.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        chouryzhou <chouryzhou@...il.com>
Subject: 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 -