From a124c8d59b81630855909df1223017c9ea98602f Mon Sep 17 00:00:00 2001 From: Trung Date: Mon, 13 Jul 2026 16:03:50 +0700 Subject: [PATCH] chown-overlay fix ID-space mismatch in set and apply guards chown_overlay_set receives cur_uid/cur_gid from chown_result where they were taken directly from host_st->st_uid/st_gid (the raw macOS stat values). new_owner/new_group are guest-virtual UIDs/GIDs. These two live in different ID namespaces, so comparing them: next_uid = new_owner == -1 ? cur_uid : new_owner; if (next_uid == cur_uid && next_gid == cur_gid) goto out; produces wrong answers whenever host UID != GUEST_UID (e.g. host user has UID 501, guest runs as GUEST_UID 1000), or whenever the file already has an overlay entry that virtualises its owner to a different value. Two complementary fixes: fs.c - chown_result: Apply the existing overlay to a temporary stat copy before extracting cur_uid/cur_gid. This makes the values passed to chown_overlay_set reflect what the guest currently sees for the file rather than the physical host owner. chown-overlay.c - chown_overlay_set / chown_overlay_apply: Initialise new overlay entries with the sentinel (uint32_t)-1 ("no override for this field") instead of cur_uid/cur_gid. Resolve sentinels to cur before the stale-entry removal check so both sides of the comparison use guest-virtual IDs. Skip sentinel fields in chown_overlay_apply so a partial chown does not corrupt the untouched field with a stale value. fix #136 --- src/syscall/chown-overlay.c | 12 ++++++++++++ src/syscall/fs.c | 32 ++++++++++++++++++++++++-------- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/syscall/chown-overlay.c b/src/syscall/chown-overlay.c index dd94beb..5b1b7b1 100644 --- a/src/syscall/chown-overlay.c +++ b/src/syscall/chown-overlay.c @@ -117,6 +117,11 @@ int chown_overlay_set(uint64_t dev, } e->dev = dev; e->ino = ino; + /* Initialise with the sentinel (uint32_t)-1, meaning "no override + * recorded for this field yet". The apply path skips sentinel + * fields, so a partial chown that only touches one of uid/gid does + * not corrupt the other field with a stale physical host value. + */ e->uid = (uint32_t) -1; e->gid = (uint32_t) -1; unsigned int idx = bucket_index(dev, ino); @@ -131,6 +136,10 @@ int chown_overlay_set(uint64_t dev, if (new_group != (uint32_t) -1) e->gid = new_group; + /* A sentinel field means "keep whatever cur reports". Resolve before + * comparing so the no-op check is evaluated in the guest-virtual ID + * space on both sides. + */ uint32_t effective_uid = e->uid == (uint32_t) -1 ? cur_uid : e->uid; uint32_t effective_gid = e->gid == (uint32_t) -1 ? cur_gid : e->gid; if (effective_uid == cur_uid && effective_gid == cur_gid) @@ -166,6 +175,9 @@ void chown_overlay_apply(struct stat *st) overlay_entry_t *e = find_locked((uint64_t) st->st_dev, (uint64_t) st->st_ino); if (e) { + /* Skip sentinel fields: (uint32_t)-1 means "no override recorded" + * for that field, so leave the host value untouched. + */ if (e->uid != (uint32_t) -1) st->st_uid = e->uid; if (e->gid != (uint32_t) -1) diff --git a/src/syscall/fs.c b/src/syscall/fs.c index ef17592..5ab93d6 100644 --- a/src/syscall/fs.c +++ b/src/syscall/fs.c @@ -2458,15 +2458,31 @@ static int64_t chown_result(int host_rc, { if (host_rc < 0 && errno != EPERM) return linux_errno(); - if (host_st && chown_overlay_set((uint64_t) host_st->st_dev, - (uint64_t) host_st->st_ino, owner, group, - host_st->st_uid, host_st->st_gid) < 0) { - /* Override allocation failed; reporting success would lie about the - * post-call stat round-trip. Linux's chown(2) lists ENOMEM among its - * possible errors for related allocation paths, so surface that to the - * guest instead. + if (host_st) { + /* cur_uid/cur_gid must be the owner the guest *currently sees*, not + * the raw host stat values. The two differ on macOS where the host + * user UID (e.g. 501) does not match GUEST_UID (1000), and whenever + * the file already has an overlay entry that virtualises its owner. + * chown_overlay_set uses cur_uid/cur_gid for both the early-exit + * no-op guard and the stale-entry removal guard; passing the physical + * UID evaluates those guards in the wrong ID namespace. + * + * Apply the existing overlay to a temporary copy of the host stat to + * obtain the guest-visible owner before forwarding to + * chown_overlay_set. */ - return -LINUX_ENOMEM; + struct stat guest_st = *host_st; + chown_overlay_apply(&guest_st); + if (chown_overlay_set((uint64_t) host_st->st_dev, + (uint64_t) host_st->st_ino, owner, group, + guest_st.st_uid, guest_st.st_gid) < 0) { + /* Override allocation failed; reporting success would lie about + * the post-call stat round-trip. Linux's chown(2) lists ENOMEM + * among its possible errors for related allocation paths, so + * surface that to the guest instead. + */ + return -LINUX_ENOMEM; + } } return 0; }