DAOS-13563 vos: fix engine crash on iod_nr==0 fetch of flat-dkey objects#18710
DAOS-13563 vos: fix engine crash on iod_nr==0 fetch of flat-dkey objects#18710hgichon wants to merge 1 commit into
Conversation
|
Ticket title is 'Flat-KV object ' |
|
@jolivier23 would you be able to review this? It's a small regression fix in the flat-dkey fetch path you introduced in ae105cb ("DAOS-13563 vos: Implement flat dkey for array and flat KV object types", #13175). Context: Originally hit via versitygw's native DAOS backend (an S3 HEAD / conditional-PUT issues a One thing I'd appreciate your eye on: the |
88a5be4 to
45e7fe7
Compare
|
Update: I dropped the draft On actually running it in The fix itself is verified independently: the two guarded derefs are confirmed in the compiled If you'd like a unit test in this PR, I'm happy to add one — could you point me at the preferred way to stand up a flat-dkey (array / flat-KV) object in |
A CHECK_EXISTENCE / SET_TS_ONLY fetch carries no iods (iod_nr == 0 is
permitted for those flags by vos_ioc_create()). For a flat-dkey object
(array / flat KV on a pool with VOS_POOL_FEAT_FLAT_DKEY) dkey_fetch()
dereferenced ioc->ic_iods[0] without an iod_nr guard, crashing the engine:
- the SUBTR_EVT computation: ic_iods[0].iod_type -> SIGSEGV (NULL deref)
- the KREC_BF_NO_AKEY branch: iod_set_cursor(ioc, 0) trips
D_ASSERT(sgl_at < ic_iod_nr) -> SIGABRT, and ic_iods[0] is then
dereferenced NULL in fetch_value()
The non-flat path is naturally guarded by the "for (i = 0; i < ic_iod_nr;
i++)" loop, and object/key existence is already established by
key_tree_prepare() + key_ilog_check(), so the value fetch is simply skipped
when there are no iods. Guard both flat-path ic_iods[0] accesses with
ic_iod_nr > 0.
Regression introduced by commit ae105cb ("DAOS-13563 vos: Implement flat
dkey for array and flat KV object types", daos-stack#13175). Originally observed via
versitygw's native DAOS backend: an S3 HEAD/PUT existence probe issues a
CHECK_EXISTENCE fetch (iod_nr == 0) against a flat DFS object, crashing the
target engine (surfaced to the client as dc_rw_cb DER_TIMEDOUT).
Add VOS283 reproducing the crash under the io test group: it updates a
flat-dkey array object (DAOS_OT_ARRAY_BYTE, uint64 dkey) and then issues the
iod_nr == 0 CHECK_EXISTENCE fetch. Without the fix vos_tests SIGSEGVs in
dkey_fetch(); with it the fetch returns cleanly (DER_NONEXIST is acceptable
here - with no iods the flat-array subtree type is unknown to the probe, so
reaching a clean rc rather than a crash is what the test asserts). A
teardown restores the shared arg->oid so a failure cannot perturb the rest
of the io suite. Verified on real hardware: the same vos_tests binary
crashes against an unfixed libvos and passes against the fixed one (io group
21/21), and the crash reproduces on pure upstream master as well.
Signed-off-by: hgichon <hgichon@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
45e7fe7 to
a3cf556
Compare
|
Re-added the Corrected test (
Verified on hardware (md-on-SSD build,
I also physically reproduced the SIGSEGV on pure upstream master (the merge-base commit, none of my local changes) with the same test, confirming this is an upstream regression from ae105cb rather than a local artifact. |
Summary
dkey_fetch()crashes the engine on aCHECK_EXISTENCE/SET_TS_ONLYfetch (iod_nr == 0) of a flat-dkey object (array / flat KV on a pool withVOS_POOL_FEAT_FLAT_DKEY).vos_ioc_create()permitsiod_nr == 0for those flags, leavingioc->ic_iods == NULL, but the flat path indkey_fetch()dereferencesic_iods[0]without aniod_nrguard:ic_iods[0].iod_type→ SIGSEGV (NULL + 0x18)KREC_BF_NO_AKEYbranch —iod_set_cursor(ioc, 0)tripsD_ASSERT(sgl_at < ic_iod_nr)→ SIGABRT, andic_iods[0]is then dereferenced NULL infetch_value()Regression introduced by ae105cb ("DAOS-13563 vos: Implement flat dkey for array and flat KV object types", #13175). Present on master and any branch carrying that commit.
Fix
Guard both flat-path
ic_iods[0]accesses withic_iod_nr > 0. The non-flat path is already guarded by itsfor (i = 0; i < ic_iod_nr; i++)loop; key existence is established earlier bykey_tree_prepare()+key_ilog_check(), so the value fetch is correctly skipped when there are no iods.How it was found
versitygw's native DAOS backend issues an S3 HEAD / conditional-PUT existence probe as a
CHECK_EXISTENCEfetch (iod_nr == 0) against a flat DFS object. This crashed the target engine (SIGSEGV/SIGABRT), surfacing to the S3 client asdc_rw_cb ... DER_TIMEDOUT. Observed backtrace (release build):Test
Adds
VOS283tosrc/vos/tests/vts_io.c(io test group), reproducing the crash:DAOS_OT_ARRAY_BYTE, uint64 dkey) and updates it, so the object/dkey exists. The earlier draft used a string dkey, which an array object rejects with-DER_INVALinio_test_obj_update()before the fetch under test was reached — that was the "key format" issue, not a fault in the fix.iod_nr == 0VOS_OF_FETCH_CHECK_EXISTENCEfetch — the crash trigger.rc == 0 || rc == -DER_NONEXIST. With no iods the flat-array dkey's subtree type is unknown to the probe, so a cleanDER_NONEXISTis legitimate; reaching a clean rc rather than a SIGSEGV/SIGABRT is what the test verifies.teardownrestores the sharedarg->oidso a failure can't perturb the rest of the io suite.Verified on hardware (md-on-SSD build,
PMEMOBJ_CONF=sds.at_create=0), samevos_testsbinary with onlylibvos.soswapped:libvos.so→VOS283 ... Segmentation fault(11)(exit 1)libvos.so→VOS283 OK, io group 21/21 PASSAlso reproduced the SIGSEGV on pure upstream master (the merge-base commit, no local changes) with the same test, confirming this is the upstream regression rather than a local artifact.
Signed-off-by: hgichon hgichon@gmail.com