Date: Wed, 10 Jun 2015 01:43:59 +0900 From: <kikuchan@uranus.dti.ne.jp> To: Mateusz Guzik <mjguzik@gmail.com> Cc: <freebsd-jail@freebsd.org>, <freebsd-stable@freebsd.org> Subject: Re: [patch] separate SysV IPC namespace for jail Message-ID: <e4fac688781e0b491d328e632eaa6322@imap.cm.dream.jp> In-Reply-To: <20150608171702.GA15516@dft-labs.eu> References: <CAG40kxFFnfvbLbqVprPC0oZ%2BnbKDYGxdvgd-vxWXFfN%2B3NQ0_A@mail.gmail.com> <20150605235348.GA9965@dft-labs.eu> <CAG40kxEaOAmcOCwb7p6NF6sgox-KysKh2RJgG7og1fi0WL0-Sg@mail.gmail.com> <20150607013929.GA9182@dft-labs.eu> <CAG40kxFaD%2BTS3Asb7ZiRW67XLtMOe6ChDEVgkSnt1Ji3013j4w@mail.gmail.com> <20150607083734.GB9182@dft-labs.eu> <CAG40kxGFvgP0Zhoseo%2BDi2Zk2J6kf0jA8isZD5UDOoqnWdkqYQ@mail.gmail.com> <20150608171702.GA15516@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Mateusz, Thank you for your reply. (and sorry about my ISP mail service jammed...) I still want to resolve sysv ipc confilicts between jails somehow. (It's welcome if someone else do, but I couldn't find any working example yet...) > I only briefly looked at the patch. The fact that you perform outside of > ipcperm looks suspicious but may be harmless, so at best it's a bad > style. If you need ipc mechanism-specifc functions, make them call > ipcperm instead. Sorry, I guess EACCES misled you. I should have chosen other value and/or concealed information for each jail completely. I intended to demonstrate it's enought to achieve IPC key_t space separation (to PostgreSQL work) for each jails without having shmid_kernel struct for each jails. > The jail check is too simplistic. Jails higher in the hierarchy should be > able to access whatever lower jails produced. Yes I thought so, however, I realized that a single key_t space per jail is enough, otherwise user get confused. Hmm, but yes, probably you're right. I can't find any technical reason to forbid the behavior you mentioned. (I think it needs to modify ipcs(1) so that can display JID to avoid confusing) > Well, as I said in my first mail the idea is to make ipc code look at > structures assigned to given jail, so that we can have multiple jails > with only their own objects. No "well, this id is used by other jail", > unless the namespace is explicitly shared. Ok, now I've understood what the idea is, and maybe it's done by Nick once before on https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=48471 I'm reading the code (shm so far) and kernel deeply now, and I'm so sorry about confusing you due to less knowledge of kernel code. There are two identifier, ID and KEY (shmid and shmkey for SHM) on SysV IPC object. The KEY space must be separated for each jails I think, but about the ID, is determined by kernel and userland users don't care its value, do you really think it should be managed separately in kernel? I agree that "multiple jails with only their own objects" is good design basically, but especially, if you want to support hierarchical jails, the objects will be referenced by multiple jails. If ID space separation is not so important, separating internal namespace for each jail is too complicate for simple KEY space separation, I think. I really should have implemented to conceal information instead of returning EACCES, sorry. ;/ Before jumping to the conclusion, I want to know that *current* code relative to SHM whether have any problems on sharing underlaying vm page between processes that jailed to different jails each other, especially on fork and jail_attach. multithreaded process perhaps? I'm also trying to port Nick's code to 10/stable, then I've found potentially dangerous behavior happens if a process do jail_attach to other jail, due to the separate namespaces have separate ID space for each jail. I guess it was not happned on 4.8 because lack of jail_attach. For example, a process attached to shmid = 65535 on jid=1, then the process changes its jail to jid=2, and if shmid = 65535 exists on jid=2, the process refers wrong vm mapping unless maintain shmmap_state data for the process every jail_attach. Maybe this behavior is something relative to the race that you mentioned before? > For instance back then I could not find any reliable mechanism to tell > me whether given process has a shared address space. There is only a > vm_refcnt counter in vmspace which is modified on various occasions, Hmm, sorry I can't understand what the problem is here... I'm not good at kernel internals yet, so I don't know details of when the processes share the address space, and I have no idea why you want to know whether the process has a shared address space or not... > It is easy to implement it for "private purposes" (i.e. > disregarding possible attacks with jailing processes). The real work is > making the whole business safe. I agree. Is there any project ongoing for this sysvipc issue? If any, what is needed to be done? P.S. I'm really sorry if you feel bad, due to my bad English skill... Regards, Kikuchan On Mon, 8 Jun 2015 19:17:03 +0200, Mateusz Guzik <mjguzik@gmail.com> wrote: > On Mon, Jun 08, 2015 at 01:42:21AM +0900, kikuchan wrote: >> From my curiosity, is my patch a technically bad? >> Is there race condition in it? Or, enabling key_t separation for jail >> could trigger race condition, perhaps? >> > > I only briefly looked at the patch. The fact that you perform outside of > ipcperm looks suspicious but may be harmless, so at best it's a bad > style. If you need ipc mechanism-specifc functions, make them call > ipcperm instead. > > The jail check is too simplistic. Jails higher in the hierarchy should be > able to access whatever lower jails produced. > >> >> > I do see great benefit in having jail-aware ipcs. >> > >> > I do not believe the way to achieve it is to add jail-aware permission >> > checks. Support in question should provide support for separate >> > namespaces. The are several upsides, including lack of conflict between >> > jails and plugged infoleaks. >> >> Sorry but I might misunderstand what your "separate namespaces" means. >> What namespace are you going to separate? key_t, shmid, kernel >> structure of shm, or others? >> What features do your "jail-aware ipcs" provide? >> > > Well, as I said in my first mail the idea is to make ipc code look at > structures assigned to given jail, so that we can have multiple jails > with only their own objects. No "well, this id is used by other jail", > unless the namespace is explicitly shared. > > I did have a patch with a meh implementation doing this, but I lost it > along the way. It is easy to implement it for "private purposes" (i.e. > disregarding possible attacks with jailing processes). The real work is > making the whole business safe. > > For instance back then I could not find any reliable mechanism to tell > me whether given process has a shared address space. There is only a > vm_refcnt counter in vmspace which is modified on various occasions, > thus is not suitable. Adding a separate counter sucks and adding a "once > set, never cleared flag" sucks as well. Maybe there is a good method.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?e4fac688781e0b491d328e632eaa6322>