Skip to content

gfapi: glfs_creat()/glfs_openat() store a stack pointer as "gfid-req" in the xdata of an open; open-behind replays it after the caller returned #4809

Description

@ThalesBarretto

Description of problem

glfs_creat() and the *at() entry helper generate the gfid of the file to be created into a uuid_t on the caller's
stack and store a pointer to it in xattr_req under "gfid-req" (dict_set_gfuuid(..., true): true means the
dict keeps the caller's pointer and does not copy the 16 bytes). When the file already exists, the same xattr_req is
passed to the open instead of the create.

An open is a fop a translator may answer first and perform later. performance/open-behind does exactly that: it
unwinds the open at once and performs the real open from an io-thread on the first fop that needs it. By then
glfs_creat() has returned, and dict_to_xdr() copies the gfid-req out of a stack frame that no longer exists.

Where (devel a482a8578a)

  • api/src/glfs-fops.c:1030 pub_glfs_creat(): uuid_t gfid :1041; dict_set_gfuuid(xattr_req, "gfid-req", gfid, true)
    :1063; existing file -> syncop_open(subvol, &loc, flags, glfd->fd, xattr_req, NULL) :1142.
  • api/src/glfs-fops.c:558 setup_entry_fopat_args(): dict_set_gfuuid(*xattr_req, "gfid-req", gfid, true) :595, with
    gfid the uuid_t of the caller pub_glfs_openat() (:615, :625); existing file ->
    syncop_open(subvol, &loc, flags, glfd->fd, fop_attr, NULL) :674.
  • xlators/performance/open-behind/src/open-behind.c:465: fop_open_stub(frame, ob_open, loc, flags, fd, xdata) keeps a
    reference to that xdata; the stub is resumed later (ob_open_and_resume_fd -> ob_open_resume), reaching
    client_pre_open_v2() -> dict_to_xdr() (rpc/xdr/src/glusterfs3.h:766, the GFUUID memcpy).

How it shows

With --enable-asan, every gfapi test that creates an already existing file with open-behind on dies in the tester:

ERROR: AddressSanitizer: stack-use-after-return
READ of size 16 ... in dict_to_xdr ../../../../rpc/xdr/src/glusterfs3.h:766
    #1 client_pre_open_v2 xlators/protocol/client/src/client-common.c:402
    #2 client4_0_open ... #7 ob_open_resume ... #12 ob_writev ... #18 iot_worker
Address is located in stack of thread T0 at offset 64 in frame
    #0 pub_glfs_creat api/src/glfs-fops.c:1031
    [64, 80) 'gfid' (line 1041) <== Memory access at offset 64 is inside this variable

(tests/basic/gfapi/gfapi-ssl-test.t, upcall-register-api.t, glfsxmp.t.) Without instrumentation the OPEN carries 16
bytes of dead stack as its gfid-req; the server ignores gfid-req on open, so it has been silent. It is still a read of
freed stack on every such open, and the same xdata travels through every translator below open-behind.

The other gfid-req producers

glfs_symlink (:2987), glfs_mknod (:3134), glfs_mkdir (:3224), the handle-based creators in glfs-handleops.c
and the resolver's lookups (glfs-resolve.c:389, :444) use the same dict_set_gfuuid(..., true), but their fops
(create, mkdir, mknod, symlink, lookup) keep the caller blocked until the fop completes, so their dicts cannot outlive
the frame. Only the two open-capable sites are affected.

Proposed fix

Allocate the gfid and hand it to the dict with is_static=false, so the dict owns and frees it - the way
features/shard (shard.c:1450) and features/gfid-access (gfid-access.c:314) already set their gfid-req.
(dict_set_gfuuid() never copies: true borrows the caller's pointer, false takes ownership of it; the stack pointer
must therefore be replaced, not re-flagged.) Two sites, api/src/glfs-fops.c +29/-5. Candidate patch in PR #4810.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions