minijail_from_fd functional bug report & some other feedback

[Kostya Kortchinsky]

Hey,

The following applies to minijail when using the preload library (also keep in mind that I might be completely wrong on everything that I am saying, and completely miss the point).

We faced some issues where `minijail_from_fd` was failing in what appeared non-deterministic fashion & without some straight way to reproduce.
I tracked it down (I think?) to the fact that the "atomicity" of the read on pipes is only guaranteed for <= PIPE_BUF bytes (https://man7.org/linux/man-pages/man7/pipe.7.html), meaning that based on parent/child scheduling, for larger marshalled structures (usually > 4096), the child could fail to read the expected size and error out (while really the whole thing is there, it just has to try harder).
We fixed it by implementing a `read_exactly` that mirrors `write_exactly` and loops to read the whole expected size as opposed to trying a single read.

Couple of other (related) hurdles we had to solve:
- `--logging=stderr` is not passed to the child, so in environments without a /dev/log, you can't see the preload outputs. We fixed this by passing an env var from the parent that would help doing an appropriate init_logging in the child. Alternatively, the logging structure could be part of the marshalled one?
- the unmarshalling (including minijail_from_fd) is basically log-less, which makes it real hard to know where things fail when they do :P (solved by adding logs for each error condition); to be fair, it was really only needed until the problem was solved.
- the `RET_ERRNO` syscalls (as defined in a policy) are not logged. I think it's fair to expect that allowing logging (-L) would warrant `SECCOMP_FILTER_FLAG_LOG` on the filter. We solved that by adding said flag to `sys_seccomp` if `j->flags.seccomp_filter_logging` and use the branch in that case (not sure it's the right way tbh).

Anyway, that would be great to get all of those upstream. I can't really tell if you accept outside contributions, if so I can send you PRs, otherwise I am pretty sure you can prompt any coding model with that email and make that happen in a jiffy :)

Cheers!

Kostya

[Jorge Lucangeli Obes]

Hey Kostya,

On Thu, Oct 9, 2025 at 12:00 PM 'Kostya Kortchinsky' via minijail <mini...@chromium.org> wrote:

Hey,

The following applies to minijail when using the preload library (also keep in mind that I might be completely wrong on everything that I am saying, and completely miss the point).

We faced some issues where `minijail_from_fd` was failing in what appeared non-deterministic fashion & without some straight way to reproduce.
I tracked it down (I think?) to the fact that the "atomicity" of the read on pipes is only guaranteed for <= PIPE_BUF bytes (https://man7.org/linux/man-pages/man7/pipe.7.html), meaning that based on parent/child scheduling, for larger marshalled structures (usually > 4096), the child could fail to read the expected size and error out (while really the whole thing is there, it just has to try harder).
We fixed it by implementing a `read_exactly` that mirrors `write_exactly` and loops to read the whole expected size as opposed to trying a single read.

This sounds reasonable.

 

Couple of other (related) hurdles we had to solve:
- `--logging=stderr` is not passed to the child, so in environments without a /dev/log, you can't see the preload outputs. We fixed this by passing an env var from the parent that would help doing an appropriate init_logging in the child. Alternatively, the logging structure could be part of the marshalled one?
- the unmarshalling (including minijail_from_fd) is basically log-less, which makes it real hard to know where things fail when they do :P (solved by adding logs for each error condition); to be fair, it was really only needed until the problem was solved.
- the `RET_ERRNO` syscalls (as defined in a policy) are not logged. I think it's fair to expect that allowing logging (-L) would warrant `SECCOMP_FILTER_FLAG_LOG` on the filter. We solved that by adding said flag to `sys_seccomp` if `j->flags.seccomp_filter_logging` and use the branch in that case (not sure it's the right way tbh).

Hmm good point, I can see the argument for logging RET_ERRNO calls. I would need to take a quick look at that part of the code to see if it makes the most sense to do it that way.

 

Anyway, that would be great to get all of those upstream. I can't really tell if you accept outside contributions, if so I can send you PRs, otherwise I am pretty sure you can prompt any coding model with that email and make that happen in a jiffy :)

We do accept external contributions! Upstream Minijail is currently on the Chromium git server https://chromium.googlesource.com/chromiumos/platform/minijail/, and there is a Gerrit code review instance that can be used to review patches. I believe a company CLA might be required for us to be able to accept patches though.

Regards,

Jorge

 

Cheers!

Kostya

--
You received this message because you are subscribed to the Google Groups "minijail" group.
To unsubscribe from this group and stop receiving emails from it, send an email to minijail+u...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/minijail/CALXKMC5%3DiLYe0VkrwNZNGwLeH6ChjTuqiWPVN1Q74WfaJWOe-Q%40mail.gmail.com.