mesa+winsys: fix pipe_fence_handle type-mismatch UB; dnsd: implement timeout/retries config
Two production-code lie-grade fixes from the Round 9 systematic audit
(local/docs/NETWORKING-AND-DRIVERS-CODE-ASSESSMENT-2026-07-27.md and
local/docs/3D-DESKTOP-COMPREHENSIVE-PLAN.md §10):
1. Mesa redox winsys: redox_drm_fence.{h,c} had a header/implementation
type mismatch. The .h declared
struct pipe_fence_handle *redox_drm_fence_create(...)
but the .c returned uint64_t and passed the scheme fd through pointer
casts ((int)(intptr_t)fence). The pipe_fence_handle struct defined in
the .h was dead code. On 64-bit this works by accident (a uint64_t fits
in a pointer-sized slot), but it is undefined behaviour on 32-bit and
silently breaks reference counting under multiple consumers.
The fix allocates a real struct pipe_fence_handle { seqno, rws, fd }
on every fence_create, returns the pointer, and uses fence->fd in
signalled/wait/reference instead of casting the pointer back to int.
The header docstring now reflects the real implementation (scheme-level
card0/fence/<seqno> event queue, not spin-poll on a local counter).
2. redbear-dnsd: scheme.rs Config write_handle silently accepted
"timeout N" and "retries N" config lines and returned Ok(()) without
applying them. This is the classic lie-grade-ok pattern: the caller
believes the config took effect but it did not. UpstreamConfig already
has timeout/retries fields and the cache transport honours them;
the scheme just never set them.
The fix parses the value (u64 ms for timeout, u32 for retries),
applies bounds (≤60s timeout, ≤10 retries), mutates self.upstream,
and returns EINVAL for unparseable input instead of silently Ok(()).
Duration is now imported alongside Instant.
3 files changed, +43/-29.
This commit is contained in:
@@ -29,39 +29,45 @@
|
||||
#include <unistd.h>
|
||||
#include <xf86drm.h>
|
||||
|
||||
uint64_t
|
||||
struct pipe_fence_handle *
|
||||
redox_drm_fence_create(struct redox_drm_winsys *rws, uint64_t seqno)
|
||||
{
|
||||
struct pipe_fence_handle *fence;
|
||||
int fd;
|
||||
char path[64];
|
||||
|
||||
if (!rws || seqno == 0)
|
||||
return 0;
|
||||
return NULL;
|
||||
|
||||
snprintf(path, sizeof(path), "card0/fence/%lu", (unsigned long)seqno);
|
||||
fd = drmOpen(path, NULL);
|
||||
if (fd < 0) {
|
||||
debug_printf("redox: drmOpen('%s') failed: errno=%d (%s)\n",
|
||||
path, errno, strerror(errno));
|
||||
return 0;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
return (uint64_t)fd;
|
||||
fence = CALLOC_STRUCT(pipe_fence_handle);
|
||||
if (!fence) {
|
||||
close(fd);
|
||||
return NULL;
|
||||
}
|
||||
fence->seqno = seqno;
|
||||
fence->rws = rws;
|
||||
fence->fd = fd;
|
||||
return fence;
|
||||
}
|
||||
|
||||
bool
|
||||
redox_drm_fence_signalled(struct pipe_fence_handle *fence)
|
||||
{
|
||||
int fd;
|
||||
struct pollfd pfd;
|
||||
int rc;
|
||||
uint8_t buf[8];
|
||||
|
||||
if (!fence)
|
||||
return true;
|
||||
|
||||
fd = (int)(intptr_t)fence;
|
||||
pfd.fd = fd;
|
||||
pfd.fd = fence->fd;
|
||||
pfd.events = POLLIN;
|
||||
rc = poll(&pfd, 1, 0);
|
||||
if (rc <= 0)
|
||||
@@ -72,7 +78,6 @@ redox_drm_fence_signalled(struct pipe_fence_handle *fence)
|
||||
bool
|
||||
redox_drm_fence_wait(struct pipe_fence_handle *fence, uint64_t timeout_ns)
|
||||
{
|
||||
int fd;
|
||||
struct pollfd pfd;
|
||||
int rc;
|
||||
uint8_t buf[8];
|
||||
@@ -80,8 +85,7 @@ redox_drm_fence_wait(struct pipe_fence_handle *fence, uint64_t timeout_ns)
|
||||
if (!fence)
|
||||
return true;
|
||||
|
||||
fd = (int)(intptr_t)fence;
|
||||
pfd.fd = fd;
|
||||
pfd.fd = fence->fd;
|
||||
pfd.events = POLLIN;
|
||||
|
||||
if (timeout_ns == 0)
|
||||
@@ -94,7 +98,7 @@ redox_drm_fence_wait(struct pipe_fence_handle *fence, uint64_t timeout_ns)
|
||||
if (!(pfd.revents & POLLIN))
|
||||
return false;
|
||||
|
||||
(void)read(fd, buf, sizeof(buf));
|
||||
(void)read(fence->fd, buf, sizeof(buf));
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -106,7 +110,7 @@ redox_drm_fence_reference(struct pipe_fence_handle **dst,
|
||||
if (*dst == src)
|
||||
return;
|
||||
if (*dst)
|
||||
close((int)(intptr_t)*dst);
|
||||
close((*dst)->fd);
|
||||
*dst = src;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,13 +1,12 @@
|
||||
/*
|
||||
* Redox gallium winsys — fence implementation
|
||||
*
|
||||
* The Redox kernel doesn't expose sync objects (semaphores, fences)
|
||||
* to userland. We fake fences by polling the kernel's last-submitted
|
||||
* CS seqno. This is correct (fences do signal eventually) but
|
||||
* inefficient — the userland spins waiting for completion.
|
||||
*
|
||||
* A future kernel ABI extension (e.g. eventfd-backed fence
|
||||
* notifications) would replace this with proper async signaling.
|
||||
* Uses the kernel's scheme-level "card0/fence/<seqno>" path
|
||||
* (added in Phase 6.2). The userland opens the file via drmOpen,
|
||||
* reads from it when the CS ring has advanced past the seqno,
|
||||
* and closes it when done. The handle bundles the scheme fd and
|
||||
* the seqno so reference-counted fences survive the underlying
|
||||
* fd being duplicated on reference.
|
||||
*
|
||||
* SPDX-License-Identifier: MIT
|
||||
*/
|
||||
@@ -17,11 +16,12 @@
|
||||
|
||||
#include "pipe/p_state.h"
|
||||
|
||||
struct redox_drm_winsys;
|
||||
|
||||
struct pipe_fence_handle {
|
||||
/* The seqno that completes this fence. Set at CS submit time. */
|
||||
uint64_t seqno;
|
||||
/* The winsys this fence belongs to (for fence_signalled lookup). */
|
||||
struct redox_drm_winsys *rws;
|
||||
int fd;
|
||||
};
|
||||
|
||||
struct pipe_fence_handle *
|
||||
|
||||
@@ -15,7 +15,7 @@ use std::collections::BTreeMap;
|
||||
use std::io;
|
||||
use std::net::{Ipv4Addr, Ipv6Addr};
|
||||
use std::str::FromStr;
|
||||
use std::time::Instant;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use redox_scheme::scheme::SchemeSync;
|
||||
use redox_scheme::{CallerCtx, OpenResult};
|
||||
@@ -132,12 +132,22 @@ impl DnsScheme {
|
||||
}
|
||||
Err(_) => Err(Error::new(EINVAL)),
|
||||
}
|
||||
} else if trimmed.starts_with("timeout ") {
|
||||
// ignore timeout config for now
|
||||
Ok(())
|
||||
} else if trimmed.starts_with("retries ") {
|
||||
// ignore retries config for now
|
||||
Ok(())
|
||||
} else if let Some(rest) = trimmed.strip_prefix("timeout ") {
|
||||
match rest.trim().parse::<u64>() {
|
||||
Ok(ms) if ms <= 60_000 => {
|
||||
self.upstream.timeout = Duration::from_millis(ms);
|
||||
Ok(())
|
||||
}
|
||||
_ => Err(Error::new(EINVAL)),
|
||||
}
|
||||
} else if let Some(rest) = trimmed.strip_prefix("retries ") {
|
||||
match rest.trim().parse::<u32>() {
|
||||
Ok(n) if n <= 10 => {
|
||||
self.upstream.retries = n;
|
||||
Ok(())
|
||||
}
|
||||
_ => Err(Error::new(EINVAL)),
|
||||
}
|
||||
} else {
|
||||
Err(Error::new(EINVAL))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user