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 <[email protected]>
AuthorDate: 2025-09-04 20:27:47 +0000
Commit:     Jamie Gritton <[email protected]>
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

Reply via email to