hurd: all 5 defects patched, hurd-0004 closed as clean scan, add hurd-0005 patch
This commit is contained in:
parent
80a81910cf
commit
6050e1b741
12 changed files with 401 additions and 33 deletions
|
|
@ -1,32 +1,20 @@
|
|||
# hurd-0004 — CWE-407: check_owner → check_uid — O(nuids²) double UID scan
|
||||
# hurd-0004 — CLEAN SCAN: check_owner / check_uid — no O(N²) pattern
|
||||
|
||||
**Severity:** LOW-MEDIUM
|
||||
**Files:** `proc/info.c` lines ~47–67, `proc/mgt.c` lines ~84–92
|
||||
**Status:** CLEAN — no UNDF ID assigned
|
||||
**Files inspected:** `proc/info.c`, `proc/mgt.c`
|
||||
**Functions:** `check_owner`, `check_uid`
|
||||
**CWE:** CWE-407 Algorithmic Complexity
|
||||
|
||||
## Defect
|
||||
## Finding
|
||||
|
||||
`check_owner(proc1, proc2)` verifies proc1 holds every UID proc2 holds. Outer loop over proc2's UIDs, inner `check_uid()` linearly scans proc1's UIDs.
|
||||
Ticket was written speculatively. Actual `check_owner` (proc/info.c:41-47) does NOT loop over proc2's UIDs — it checks `proc2->p_owner`, a single UID, by calling `check_uid()` once. Total cost: O(nuids_proc1), which is O(N) for N ≤ 64.
|
||||
|
||||
```c
|
||||
// check_owner — proc/info.c ~65
|
||||
for (size_t i = 0; i < proc2->p_id->i_nuids; i++)
|
||||
if (!check_uid(proc1, proc2->p_id->i_uids[i])) // O(k) linear scan
|
||||
return 0;
|
||||
|
||||
// check_uid — proc/mgt.c ~88
|
||||
for (i = 0; i < p->p_id->i_nuids; i++)
|
||||
if (p->p_id->i_uids[i] == uid || p->p_id->i_uids[i] == 0)
|
||||
return 1;
|
||||
// Actual check_owner — O(N), not O(N²)
|
||||
int check_owner (struct proc *proc1, struct proc *proc2) {
|
||||
return proc2->p_noowner
|
||||
? check_uid (proc1, 0) || proc1->p_login == proc2->p_login
|
||||
: check_uid (proc1, proc2->p_owner); // single call, not a loop
|
||||
}
|
||||
```
|
||||
|
||||
**Complexity:** O(nuids_proc2 × nuids_proc1). With N = total UIDs per process, O(N²).
|
||||
|
||||
## Scale
|
||||
|
||||
N is small in practice (≤64 UIDs), but called on every `proc_pid2task`, `proc_pid2proc`, `proc_proc2task` — every `kill()`, `waitpid()`, `ptrace()`, and task port request.
|
||||
|
||||
## Fix
|
||||
|
||||
Sort proc1's UID array once and use binary search for O(log N) lookup per check, giving O(N log N) total. Or maintain a small bitset for O(1) lookup when UID range is bounded.
|
||||
No O(N×k) amplifier exists. Not a CWE-407 defect. No patch, no UNDF ID.
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
# hurd-0005 — CWE-407: _ports_bucket_class_iterate — O(total_ports) global scan
|
||||
# hurd-0005 — CWE-407: _ports_bucket_class_iterate — O(total_ports) global scan [PATCHED]
|
||||
|
||||
**Severity:** MEDIUM
|
||||
**File:** `libports/bucket-iterate.c` lines ~41–80
|
||||
|
|
@ -31,3 +31,14 @@ Total ports in a busy Hurd system: potentially thousands (one per open file desc
|
|||
## Fix
|
||||
|
||||
Maintain a per-class linked list or per-class hash table in `port_class`. `ports_class_iterate` iterates only its own members in O(class_size) rather than O(total_ports). This is what the source comment already asks for.
|
||||
|
||||
## Patch
|
||||
|
||||
`patch/0003-CWE-407-hurd-0005-per-class-port-list.patch`
|
||||
|
||||
- `port_info`: added `class_next` / `class_prevp` intrusive list fields
|
||||
- `port_class`: added `class_ports` head pointer
|
||||
- `create-internal.c` + `import-port.c`: prepend to class list under `_ports_htable_lock` wrlock
|
||||
- `complete-deallocate.c`: splice out of class list under same wrlock
|
||||
- `bucket-iterate.c`: class-path walks `class->class_ports` in O(class_size); NULL-class (bucket iterate) still uses `HURD_IHASH_ITERATE`
|
||||
- Upper-bound malloc uses `ht->nr_items` (always ≥ class size) to avoid race with `class->count` (updated under separate `_ports_lock` after htable_lock released)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
# UNDF: UNDF-2026-000001244
|
||||
From c05586601073629e48ed886fbdaa12b9c4c80313 Mon Sep 17 00:00:00 2001
|
||||
From: "russell@unturf.com" <russell@unturf.com>
|
||||
Date: Sat, 4 Apr 2026 20:31:12 -0400
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
# UNDF: UNDF-2026-000001245
|
||||
From 25aa731e690c799837c21c09db0d047b811e79c3 Mon Sep 17 00:00:00 2001
|
||||
From: "russell@unturf.com" <russell@unturf.com>
|
||||
Date: Sat, 4 Apr 2026 20:31:12 -0400
|
||||
|
|
|
|||
|
|
@ -0,0 +1,184 @@
|
|||
From f5f781fa2e85e407ac6742df85e001ba3e5e7342 Mon Sep 17 00:00:00 2001
|
||||
From: "russell@unturf.com" <russell@unturf.com>
|
||||
Date: Sun, 5 Apr 2026 15:56:05 -0400
|
||||
Subject: [PATCH] CWE-407 (hurd-0005): per-class port list for O(class_size)
|
||||
class iteration
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
ports_class_iterate previously called _ports_bucket_class_iterate which
|
||||
scanned the entire _ports_htable filtering by class pointer — O(total_ports)
|
||||
even when only a small class subset was needed. The source code itself
|
||||
carried the comment: "This is obscenely inefficient."
|
||||
|
||||
Fix: add a per-class intrusive linked list (class_ports head in port_class,
|
||||
class_next/class_prevp in port_info) protected by _ports_htable_lock.
|
||||
create-internal.c and import-port.c prepend to this list under the existing
|
||||
_ports_htable_lock wrlock. complete-deallocate.c removes from it under the
|
||||
same wrlock. _ports_bucket_class_iterate now walks class->class_ports
|
||||
directly when class != NULL — O(class_size) — instead of scanning all ports.
|
||||
|
||||
The malloc upper-bound uses ht->nr_items (total ports, always >= class size)
|
||||
rather than class->count to avoid a race: class->count is incremented under
|
||||
_ports_lock after _ports_htable_lock is released, so it may lag one insert
|
||||
behind the list. The existing realloc trims unused slots.
|
||||
---
|
||||
libports/bucket-iterate.c | 32 ++++++++++++++++++++++----------
|
||||
libports/complete-deallocate.c | 6 ++++++
|
||||
libports/create-internal.c | 9 +++++++++
|
||||
libports/import-port.c | 8 ++++++++
|
||||
libports/ports.h | 8 ++++++++
|
||||
5 files changed, 53 insertions(+), 10 deletions(-)
|
||||
|
||||
diff --git a/libports/bucket-iterate.c b/libports/bucket-iterate.c
|
||||
index b021b99e..eea8d3c4 100644
|
||||
--- a/libports/bucket-iterate.c
|
||||
+++ b/libports/bucket-iterate.c
|
||||
@@ -29,21 +29,24 @@ _ports_bucket_class_iterate (struct hurd_ihash *ht,
|
||||
struct port_class *class,
|
||||
error_t (*fun)(void *))
|
||||
{
|
||||
- /* This is obscenely ineffecient. ihash and ports need to cooperate
|
||||
- more closely to do it efficiently. */
|
||||
void **p;
|
||||
size_t i, n, nr_items;
|
||||
error_t err;
|
||||
|
||||
pthread_rwlock_rdlock (&_ports_htable_lock);
|
||||
|
||||
- if (ht->nr_items == 0)
|
||||
+ /* Upper bound for malloc: total ports in the htable is always >= any
|
||||
+ class subset. class->count is updated under _ports_lock (separate
|
||||
+ from _ports_htable_lock), so it may lag slightly; using ht->nr_items
|
||||
+ avoids a potential overflow. The realloc below trims unused slots. */
|
||||
+ nr_items = ht->nr_items;
|
||||
+
|
||||
+ if (nr_items == 0)
|
||||
{
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
return 0;
|
||||
}
|
||||
|
||||
- nr_items = ht->nr_items;
|
||||
p = malloc (nr_items * sizeof *p);
|
||||
if (p == NULL)
|
||||
{
|
||||
@@ -52,17 +55,26 @@ _ports_bucket_class_iterate (struct hurd_ihash *ht,
|
||||
}
|
||||
|
||||
n = 0;
|
||||
- HURD_IHASH_ITERATE (ht, arg)
|
||||
+ if (class != NULL)
|
||||
{
|
||||
- struct port_info *const pi = arg;
|
||||
-
|
||||
- if (class == 0 || pi->class == class)
|
||||
+ /* CWE-407 fix (hurd-0005): O(class_size) iteration via per-class
|
||||
+ member list instead of O(total_ports) global hash scan. */
|
||||
+ for (struct port_info *pi = class->class_ports; pi; pi = pi->class_next)
|
||||
+ {
|
||||
+ refcounts_ref (&pi->refcounts, NULL);
|
||||
+ p[n++] = pi;
|
||||
+ }
|
||||
+ }
|
||||
+ else
|
||||
+ {
|
||||
+ HURD_IHASH_ITERATE (ht, arg)
|
||||
{
|
||||
+ struct port_info *const pi = arg;
|
||||
refcounts_ref (&pi->refcounts, NULL);
|
||||
- p[n] = pi;
|
||||
- n++;
|
||||
+ p[n++] = pi;
|
||||
}
|
||||
}
|
||||
+
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
|
||||
if (n != 0 && n != nr_items)
|
||||
diff --git a/libports/complete-deallocate.c b/libports/complete-deallocate.c
|
||||
index e3123b26..0e932cc8 100644
|
||||
--- a/libports/complete-deallocate.c
|
||||
+++ b/libports/complete-deallocate.c
|
||||
@@ -45,6 +45,12 @@ _ports_complete_deallocate (struct port_info *pi)
|
||||
|
||||
hurd_ihash_locp_remove (&_ports_htable, pi->ports_htable_entry);
|
||||
hurd_ihash_locp_remove (&pi->bucket->htable, pi->hentry);
|
||||
+
|
||||
+ /* CWE-407 fix (hurd-0005): remove from per-class member list. */
|
||||
+ if (pi->class_next)
|
||||
+ pi->class_next->class_prevp = pi->class_prevp;
|
||||
+ *pi->class_prevp = pi->class_next;
|
||||
+
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
|
||||
mach_port_mod_refs (mach_task_self (), pi->port_right,
|
||||
diff --git a/libports/create-internal.c b/libports/create-internal.c
|
||||
index d79dc78f..14cc636a 100644
|
||||
--- a/libports/create-internal.c
|
||||
+++ b/libports/create-internal.c
|
||||
@@ -94,6 +94,15 @@ _ports_create_port_internal (struct port_class *class,
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
goto lose;
|
||||
}
|
||||
+
|
||||
+ /* CWE-407 fix (hurd-0005): prepend to per-class member list so
|
||||
+ ports_class_iterate can walk O(class_size) instead of O(total_ports). */
|
||||
+ pi->class_next = class->class_ports;
|
||||
+ pi->class_prevp = &class->class_ports;
|
||||
+ if (class->class_ports)
|
||||
+ class->class_ports->class_prevp = &pi->class_next;
|
||||
+ class->class_ports = pi;
|
||||
+
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
|
||||
bucket->count++;
|
||||
diff --git a/libports/import-port.c b/libports/import-port.c
|
||||
index 2c638e18..b391240c 100644
|
||||
--- a/libports/import-port.c
|
||||
+++ b/libports/import-port.c
|
||||
@@ -88,6 +88,14 @@ ports_import_port (struct port_class *class, struct port_bucket *bucket,
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
goto lose;
|
||||
}
|
||||
+
|
||||
+ /* CWE-407 fix (hurd-0005): prepend to per-class member list. */
|
||||
+ pi->class_next = class->class_ports;
|
||||
+ pi->class_prevp = &class->class_ports;
|
||||
+ if (class->class_ports)
|
||||
+ class->class_ports->class_prevp = &pi->class_next;
|
||||
+ class->class_ports = pi;
|
||||
+
|
||||
pthread_rwlock_unlock (&_ports_htable_lock);
|
||||
|
||||
bucket->count++;
|
||||
diff --git a/libports/ports.h b/libports/ports.h
|
||||
index 1b6705e3..6942b4ab 100644
|
||||
--- a/libports/ports.h
|
||||
+++ b/libports/ports.h
|
||||
@@ -57,6 +57,11 @@ struct port_info
|
||||
struct port_bucket *bucket;
|
||||
hurd_ihash_locp_t hentry;
|
||||
hurd_ihash_locp_t ports_htable_entry;
|
||||
+ /* CWE-407 fix (hurd-0005): per-class member list for O(class_size)
|
||||
+ class iteration instead of O(total_ports) global scan.
|
||||
+ Protected by _ports_htable_lock. */
|
||||
+ struct port_info *class_next;
|
||||
+ struct port_info **class_prevp;
|
||||
};
|
||||
typedef struct port_info *port_info_t;
|
||||
|
||||
@@ -90,6 +95,9 @@ struct port_class
|
||||
int rpcs;
|
||||
int count;
|
||||
void (*clean_routine) (void *);
|
||||
+ /* CWE-407 fix (hurd-0005): head of per-class member list.
|
||||
+ Protected by _ports_htable_lock. */
|
||||
+ struct port_info *class_ports;
|
||||
void (*dropweak_routine) (void *);
|
||||
struct ports_msg_id_range *uninhibitable_rpcs;
|
||||
};
|
||||
--
|
||||
2.43.0
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue