> On 21 Jan 2022, at 00:06, David CARLIER <devnexen@gmail.com> wrote:
>
> From 7f3194c36a9788d9b98630773ab907adb110cf6f Mon Sep 17 00:00:00 2001
> From: David CARLIER <devnexen@gmail.com>
> Date: Thu, 20 Jan 2022 20:56:49 +0000
> Subject: [PATCH] process worker, enabling process tracing and core dumping on
> FreeBSD 11.x and above using the procctl native API. Checking the version is
> enough as the functions and the flag we re interested in are available right
> off the bat.
>
> Signed-off-by: David CARLIER <devnexen@gmail.com>

Can you please elaborate,
what is the purpose of the proposed change?

In FreeBSD, tracing is enabled by default by means of PROC_TRACE_CTL
(yet, it could be denied by other means, see p_candebug() impl. in
kernel sources). Thus, enabling tracing explicitly makes no sense
unless it was previously disabled, such as with proccontrol(1).
That is a P2_NOTRACE kernel process flag, it controls.
(In particular, the kern.coredump sysctl used to enable/disable
coredump systemwide has a weak connection to PROC_TRACE_CTL;
it is a distinct way to specifically deny coredump).

Note that while API indeed first appeared in FreeBSD 11,
it was merged in some form to stable/10, back to FreeBSD 10.2.
OTOH, the patch uses semantics not originally present in 11.
P_PID:0 is a shortcut added rather recently in f833ab9dd187,
MFCed to stable/13 post 13.0. So, it doesn't present in
neither of the released versions.
In particular, on 13.0 the proposed call results in [EPERM],
since procctl() naturally searches for PID 0, which doesn't match.

With these thoughts, the patch doesn't look useful.
See below for more.

> ---
> auto/os/freebsd | 11 +++++++++++
> src/os/unix/ngx_freebsd_config.h | 4 ++++
> src/os/unix/ngx_process_cycle.c | 10 ++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/auto/os/freebsd b/auto/os/freebsd
> index 870bac4b..8bb086f0 100644
> --- a/auto/os/freebsd
> +++ b/auto/os/freebsd
> @@ -103,3 +103,14 @@ if [ $version -ge 701000 ]; then
> echo " + cpuset_setaffinity() found"
> have=NGX_HAVE_CPUSET_SETAFFINITY . auto/have
> fi
> +
> +# procctl
> +
> +
> +# the procctl api and its PROC_TRACE_CTL* flags exists from
> +# FreeBSD 11.x
> +
> +if [ $version -ge 1100000 ]; then
> + echo " + procctl() found"
> + have=NGX_HAVE_PROCCTL . auto/have
> +fi
> diff --git a/src/os/unix/ngx_freebsd_config.h b/src/os/unix/ngx_freebsd_config.h
> index c641108b..04ed19ca 100644
> --- a/src/os/unix/ngx_freebsd_config.h
> +++ b/src/os/unix/ngx_freebsd_config.h
> @@ -87,6 +87,10 @@
> #include <sys/event.h>
> #endif
>
> +#if (NGX_HAVE_PROCCTL)
> +#include <sys/procctl.h>
> +#endif
> +
>
> #if (NGX_HAVE_FILE_AIO)
>
> diff --git a/src/os/unix/ngx_process_cycle.c b/src/os/unix/ngx_process_cycle.c
> index 07cd05e8..c0cf052f 100644
> --- a/src/os/unix/ngx_process_cycle.c
> +++ b/src/os/unix/ngx_process_cycle.c
> @@ -869,6 +869,16 @@ ngx_worker_process_init(ngx_cycle_t *cycle,
> ngx_int_t worker)
>
> #endif
>
> +#if (NGX_HAVE_PROCCTL)
> + /* allow the process being traceable and producing a coredump in
> FreeBSD 11.x */
> + ngx_int_t ctl = PROC_TRACE_CTL_ENABLE;
> +
> + if (procctl(P_PID, 0, PROC_TRACE_CTL, &ctl) == -1) {
> + ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
> + "procctl(PROC_TRACE_CTL_ENABLE) failed");
> + }
> +#endif
> +
> if (ccf->working_directory.len) {
> if (chdir((char *) ccf->working_directory.data) == -1) {
> ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,

Given the place for patching, I can try to guess that the actual
intention is to be on par with Linux'ish PR_SET_DUMPABLE,
that is, to allow tracing/coredump with UID/GID set.

Indeed, ability to trace a process is denied with UID/GID set.
This is controlled with a kernel process flag P_SUGID.
But, P_SUGID has a higher precedence over P2_NOTRACE.

--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org