release: ngx-0.5.1#283
Conversation
(cherry picked from commit 1b802cd)
Add core main-conf wrapper for accessing top-level core main configuration in ngx-rust. This introduces: - CoreModuleConfExt for ngx_conf_t and ngx_cycle_t - core_main_conf / core_main_conf_mut typed helpers - NgxCoreModule convenience accessors for ngx_core_conf_t The new API mirrors the existing HTTP config helper style while centralizing the unsafe conf_ctx slot traversal in one place. Why: Modules that need core process configuration currently have to manually walk cycle->conf_ctx, index with ngx_core_module.index, and cast to ngx_core_conf_t. That is repetitive, easy to get wrong, and noisy in reviews. (cherry picked from commit 56f27a4)
ngx_resolve_name frees the argument on error, but we keep a stale
pointer in ResolveCtx and attempt to destroy it again on scope exit.
The fix is to skip the ResolveCtx drop handler for invalid context
pointer.
```
==46503==ERROR: AddressSanitizer: heap-use-after-free on address 0x770adc4e8d88 at pc 0x000000534136 bp 0x7fffb08f4ca0 sp 0x7fffb08f4c98
READ of size 8 at 0x770adc4e8d88 thread T0
#0 0x000000534135 in ngx_resolve_name_done src/core/ngx_resolver.c:522:14
#1 0x000000d6a84c in <ngx::async_::resolver::ResolverCtx as core::ops::drop::Drop>::drop src/async_/resolver.rs:267:13
#2 0x000000d5a4e1 in core::ptr::drop_in_place::<ngx::async_::resolver::ResolverCtx> /usr/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:805:1
nginx#3 0x000000d59dc3 in core::ptr::drop_in_place::<core::option::Option<ngx::async_::resolver::ResolverCtx>> /usr/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:805:1
nginx#4 0x000000d5a4ad in core::ptr::drop_in_place::<ngx::async_::resolver::Resolution> /usr/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:805:1
nginx#5 0x000000d5a05c in <allocator_api2::boxed::Box<ngx::async_::resolver::Resolution, ngx::core::pool::Pool> as core::ops::drop::Drop>::drop src/boxed.rs:1556:13
nginx#6 0x000000d5a05c in core::ptr::drop_in_place::<allocator_api2::boxed::Box<ngx::async_::resolver::Resolution, ngx::core::pool::Pool>> /usr/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:805:1
nginx#7 0x000000d59c61 in core::ptr::drop_in_place::<core::pin::Pin<allocator_api2::boxed::Box<ngx::async_::resolver::Resolution, ngx::core::pool::Pool>>> /usr/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:805:1
nginx#8 0x000000d665a0 in <ngx::async_::resolver::Resolution>::new src/async_/resolver.rs:199:5
nginx#9 0x00000096ceaf in <ngx::async_::resolver::Resolver>::resolve_name::{closure#0} src/async_/resolver.rs:121:28
0x770adc4e8d88 is located 8 bytes inside of 224-byte region [0x770adc4e8d80,0x770adc4e8e60)
freed by thread T0 here:
#0 0x0000004a7a3a in free (objs/nginx+0x4a7a3a)
#1 0x000000530b28 in ngx_resolver_free src/core/ngx_resolver.c:4189:5
#2 0x000000530b28 in ngx_resolve_name src/core/ngx_resolver.c:508:5
nginx#3 0x000000d663c9 in <ngx::async_::resolver::Resolution>::new src/async_/resolver.rs:193:28
nginx#4 0x00000096ceaf in <ngx::async_::resolver::Resolver>::resolve_name::{closure#0} src/async_/resolver.rs:121:28
previously allocated by thread T0 here:
#0 0x0000004a7cd8 in malloc (objs/nginx+0x4a7cd8)
#1 0x00000055e794 in ngx_alloc src/os/unix/ngx_alloc.c:22:9
#2 0x0000005306c4 in ngx_resolver_alloc src/core/ngx_resolver.c:4161:9
nginx#3 0x0000005306c4 in ngx_resolver_calloc src/core/ngx_resolver.c:4174:9
nginx#4 0x0000005306c4 in ngx_resolve_start src/core/ngx_resolver.c:422:11
nginx#5 0x000000d691d8 in <ngx::async_::resolver::ResolverCtx>::new src/async_/resolver.rs:279:28
nginx#6 0x000000d65d63 in <ngx::async_::resolver::Resolution>::new src/async_/resolver.rs:167:23
nginx#7 0x00000096ceaf in <ngx::async_::resolver::Resolver>::resolve_name::{closure#0} src/async_/resolver.rs:121:28
```
(cherry picked from commit cf663ff)
It's not possible to get NGX_RESOLVE codes from ngx_resolve_name. The one code we can get here is NGX_ERROR, which points to an unspecified resolution initialization error (likely allocation). Report it as "Internal error" instead of "Unknown NGX_RESOLVE error -1". (cherry picked from commit 03f6f45)
(cherry picked from commit 660af00)
There was a problem hiding this comment.
Pull request overview
Release PR for ngx v0.5.1 that adds core module main-conf access helpers and includes a couple of bugfixes (resolver UAF + log level filtering), plus some repo/CI housekeeping.
Changes:
- Add core module configuration access traits/helpers (
CoreModuleConfExt,CoreModuleMainConf,NgxCoreModule). - Fix log filtering so messages at the configured level are emitted (
<→<=), and make a log buffer test architecture-stable. - Adjust resolver start error handling to avoid a use-after-free, bump crate version to 0.5.1, update changelog, and remove FOSSA scanning config/workflow.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/log.rs |
Fixes log-level comparison and stabilizes a truncation test constant. |
src/core/mod.rs |
Wires in the new core::conf module exports. |
src/core/conf.rs |
Introduces new core main-conf access traits + unit tests. |
src/async_/resolver.rs |
Avoids UAF on ngx_resolve_name() error path; adds Error::Internal. |
Cargo.toml |
Bumps crate version to 0.5.1. |
Cargo.lock |
Updates locked dependency versions. |
CHANGELOG.md |
Adds 0.5.1 release notes. |
.github/workflows/fossa.yaml |
Removes license scanning workflow. |
.fossa.yml |
Removes FOSSA configuration. |
.dockerignore |
Stops ignoring .fossa.yml (since it’s removed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Caller must ensure that type `CoreModuleMainConf::MainConf` matches the configuration type | ||
| /// for the specified module. |
There was a problem hiding this comment.
The # Safety docs on CoreModuleMainConf currently say the caller must ensure the associated MainConf type matches the module's actual main-conf type. Since this is an unsafe trait, this invariant is primarily the responsibility of the implementer of the trait; callers of main_conf()/main_conf_mut() can't reasonably validate it. Updating the wording would make the safety contract clearer.
| /// Caller must ensure that type `CoreModuleMainConf::MainConf` matches the configuration type | |
| /// for the specified module. | |
| /// Implementers must ensure that `Self::MainConf` is the actual main-configuration type for | |
| /// the module returned by `Self::module()`. The default methods rely on this invariant when | |
| /// converting the raw configuration slot pointer into typed references. |
There was a problem hiding this comment.
Makes sense. Could be fixed along with #284, left a comment in the issue.
Features
Bugfixes
ngx::async_::resolveron resolver initialization failureI'm not planning to tag new nginx-sys at the moment, as I haven't found any suitable changes. Pretty much everything else in main is breaking, incomplete or requires higher MSRV.