Skip to content

proc-identity fix saved-set-ID update in setreuid and setregid#206

Merged
jserv merged 1 commit into
sysprog21:mainfrom
open-sources-port:setreuid
Jul 15, 2026
Merged

proc-identity fix saved-set-ID update in setreuid and setregid#206
jserv merged 1 commit into
sysprog21:mainfrom
open-sources-port:setreuid

Conversation

@doanbaotrung

@doanbaotrung doanbaotrung commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

sys: guard saved ID update in setreuid/setregid with euid/egid != -1

Modify DEFINE_SETRE macro to ensure that the saved-set-ID is only
updated by the effective ID change check when the new effective ID
is actually being set (i.e. e != -1).

This prevents setreuid(-1, -1) and setregid(-1, -1) from spuriously
clobbering the saved-set-ID when the current effective ID differs
from the real ID, matching Linux behavior.

Fix #140


Summary by cubic

Fix saved-set-ID handling in setreuid(2) and setregid(2) to match Linux rules. The saved ID now updates from the new effective ID after applying real/effective changes, including when only the real ID is set.

Written for commit a6d01d9. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread src/syscall/proc-identity.c Outdated
* *new* effective value (eff, which may be \
* unchanged if e == -1). \
*/ \
if (r != (uint32_t) -1 || eff != old_real) \

@jserv jserv Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saved-set-ID condition drops the e != -1 guard.

This clause fires even when the effective ID is not being changed. When e == -1, eff still holds the current, unchanged effective ID; if it differs from the real ID, saved gets spuriously rewritten. So setreuid(-1, -1) on a process whose euid != ruid (e.g. ruid=1000, euid=0, suid=500 reached via a prior setresuid) clobbers the saved-set-ID even though a (-1, -1) call must be a no-op — contradicting this PR's own description table (setreuid(-1,-1) -> no change). This is privilege-relevant because uid_is_permitted/gid_is_permitted later key off emu_suid/emu_sgid.

Linux (kernel/sys.c __sys_setreuid, and the setreuid(2) man page) guards the second clause with euid != -1:

if (r != (uint32_t) -1 || (e != (uint32_t) -1 && eff != old_real))
    saved = eff;

The core intent of the PR — updating the saved-set-ID for setreuid(r, -1) — is correct; this is the one remaining defect, and it applies to setregid too via the shared macro.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Modify DEFINE_SETRE macro to ensure that the saved-set-ID is only
updated by the effective ID change check when the new effective ID
is actually being set (i.e. e != -1).

This prevents setreuid(-1, -1) and setregid(-1, -1) from spuriously
clobbering the saved-set-ID when the current effective ID differs
from the real ID, matching Linux behavior.

@jserv jserv requested a review from Max042004 July 15, 2026 03:06
…!= -1

Modify DEFINE_SETRE macro to ensure that the saved-set-ID is only
updated by the effective ID change check when the new effective ID
is actually being set (i.e. e != -1).

This prevents setreuid(-1, -1) and setregid(-1, -1) from spuriously
clobbering the saved-set-ID when the current effective ID differs
from the real ID, matching Linux behavior.

Fix sysprog21#140
@jserv jserv merged commit d2e71b3 into sysprog21:main Jul 15, 2026
9 checks passed
@jserv

jserv commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Thank @doanbaotrung for contributing!

@doanbaotrung doanbaotrung deleted the setreuid branch July 15, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setreuid / setregid Saved ID is Not Updated on Real ID Change when e == -1

2 participants