Date: Thu, 04 Sep 2025 21:43:13 -0700 From: James Gritton <jamie@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 851dc7f859c2 - main - jail: add jail descriptors Message-ID: <da6b56365c188ce55bb4e878636bc911@freebsd.org> In-Reply-To: <aLokHDP-EMa1LR0D@kib.kiev.ua> References: <202509042031.584KVpxY000408@gitrepo.freebsd.org> <aLokHDP-EMa1LR0D@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2025-09-04 16:43, Konstantin Belousov wrote: > On Thu, Sep 04, 2025 at 08:31:51PM +0000, Jamie Gritton wrote: >> The branch main has been updated by jamie: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=851dc7f859c23cab09a348bca03ab655534fb7e0 >> >> commit 851dc7f859c23cab09a348bca03ab655534fb7e0 >> Author: Jamie Gritton <jamie@FreeBSD.org> >> AuthorDate: 2025-09-04 20:27:47 +0000 >> Commit: Jamie Gritton <jamie@FreeBSD.org> >> CommitDate: 2025-09-04 20:27:47 +0000 >> >> jail: add jail descriptors >> >> Similar to process descriptors, jail desriptors are allow jail >> administration using the file descriptor interface instead of >> JIDs. >> They come from and can be used by jail_set(2) and jail_get(2), >> and there are two new system calls, jail_attach_jd(2) and >> jail_remove_jd(2). >> >> Reviewed by: bz, brooks > > The code is from jaildesc_alloc(): > > jd = malloc(sizeof(*jd), M_JAILDESC, M_WAITOK | M_ZERO); > error = falloc_caps(td, &fp, fdp, 0, NULL); > finit(fp, priv_check_cred(fp->f_cred, PRIV_JAIL_SET) == 0 > ? FREAD | FWRITE : FREAD, DTYPE_JAILDESC, jd, &jaildesc_ops); > ^^^^^^^^^^^ '?' should be placed on the previous line I wasn't aware of this requirement; style(9) is silent on it. > if (error != 0) { > free(jd, M_JAILDESC); > return (error); > } > If falloc_caps() returned error, fp does not point to a valid file. > Then finit() operates on random memory. I'll file a fix for that. The error check just needs to be moved up. > Generated files should have been committed as a follow-up, not in the > same commit as written code. The FreeBSD Wiki explicitly allows it in the same commit. > jaildesc_find() returns EBADF when passed file type is not DTYPE_JAIL. > Normally EBADF means that the object underlying the file is > invalidated, > like vnode is reclaimed, tty is revoked, etc. For the wrong type, > EINVAL > should be returned. That's part of the code that I lifted from process descriptors, nearly identical to procdesc_find. A check of other c_type checks shows EBADF isn't uncommon. > jaildesc_close() does > finit(fp, 0, DTYPE_NONE, NULL, &badfileops); > that is not needed, same as cleaning f_data. Yes, that's appears to be overkill, considering it should only be called when the descript is about to be deallocated anyway. I'll remove that. > There are fo_chown/fo_chmod methods that are semantically applied to > the > jail files, instead of the underlying object. This is quite strange, > files > do not have concept of owner. True, it is strange. But jails don't have owners either, and this seemed a good way to control how the descriptors could be used. I see the jail descriptor as an intermediate object between the jail and the file descriptors, like there's a portal to the jail that is owned by its creator, and the file descriptor returned is merely the access to that portal. It's roughly equivalent to a temp file that doesn't exist in the filesystem directory space after its creation, yet is still a thing with ownership and permissions. I could remove this if it's too far out of mainstream practice, but I hope not to have to, since it provides a handy to allow some to (for instance) attach to a prison, but not alter or remove it. Such things are perhaps better left to Capsicum, but I don't have that support in place yet. - Jamie
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?da6b56365c188ce55bb4e878636bc911>