gfapi: copy the gfid-req into the xdata an open may carry past the caller's frame - #4810
Draft
ThalesBarretto wants to merge 1 commit into
Draft
ThalesBarretto wants to merge 1 commit into
ThalesBarretto wants to merge 1 commit into
Conversation
glfs_creat() and the *at() entry helper generate the gfid of a file to be created into a uuid_t on the caller's stack and store a pointer to it in xattr_req as "gfid-req" (dict_set_gfuuid(..., true): the dict keeps the caller's pointer without copying it). When the file already exists the same xattr_req is passed to the open (glfs_creat(), and glfs_openat() with O_CREAT), and an open is a fop a translator may answer first and perform later: open-behind does exactly that, doing the real open from an io-thread on the first fop that needs it, after glfs_creat() or glfs_openat() have returned. dict_to_xdr() then copies the gfid-req out of a stack frame that no longer exists. AddressSanitizer reports it as a stack-use-after-return in dict_to_xdr() on every gfapi test that creates an already existing file with open-behind on (tests/basic/gfapi/gfapi-ssl-test.t, upcall-register-api.t, glfsxmp.t); without instrumentation it puts 16 bytes of dead stack on the wire as the gfid-req of an OPEN, which the server ignores, so it has been silent. Allocate the gfid and hand it to the dict with is_static=false, so the dict owns and frees it, the way shard and gfid-access already set their gfid-req. The other gfid-req producers in api/src (create, mkdir, mknod, symlink, the resolver's lookups) feed fops whose caller stays blocked until the fop completes, so their dicts cannot outlive the frame; they are left unchanged. Fixes: gluster#4809 Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
ThalesBarretto
marked this pull request as draft
September 22, 2026 10:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gfapi: copy the gfid-req into the xdata an open may carry past the caller's frame
glfs_creat()and the*at()entry helper store a pointer to a stackuuid_tinxattr_reqas"gfid-req"(
dict_set_gfuuid(..., true)); when the file exists the same dict goes to theopen, which open-behind answers firstand performs later, from an io-thread, after the caller returned -
dict_to_xdr()then reads the gfid out of a deadstack frame (ASan
stack-use-after-returnongfapi-ssl-test.t,upcall-register-api.t,glfsxmp.twith open-behindon; 16 bytes of dead stack on the wire otherwise, ignored by the server). Details in #4809.
The change (
api/src/glfs-fops.c, +29/-5)At the two sites whose xattr_req can reach an open,
pub_glfs_creat()andsetup_entry_fopat_args()(used byglfs_openat()), allocate the gfid (GF_MALLOC(sizeof(uuid_t), gf_common_mt_uuid_t)) and set it withdict_set_gfuuid(..., false), so the dict owns and frees it - the patternfeatures/shardandfeatures/gfid-accessalready use for their
gfid-req.dict_set_gfuuid()never copies (trueborrows the caller's pointer,falsetakesownership), so the stack
uuid_tis replaced, not re-flagged; the*at()helper still fills its caller'suuid_targument. The create/mkdir/mknod/symlink and resolver producers keep the caller blocked until their fop completes and
are left unchanged.
Test
No new test: the defect is only observable with AddressSanitizer, where the existing
tests/basic/gfapi/gfapi-ssl-test.t,upcall-register-api.tandglfsxmp.talready fail before and pass after this change (open-behind on, its default).Fixes: #4809