Re: git: 851dc7f859c2 - main - jail: add jail descriptors

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