Repository navigation
DAOS-17948 object: fix bugs for sgl duplicate logic - #16795
Conversation
|
Ticket title is 'Potential use-after-free for ec retry IO' |
|
Test stage Functional on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-16795/1/execution/node/1268/log |
1. When IO retries occur, a use-after-free scenario may arise during scatter-gather list (sgl) merging. This happens because retried I/O's sgl references dup_sgls, which could be prematurely freed in obj_reasb_io_fini(). To fix above issue, processed_sgls must not be freed during retry operations and should be called only once per I/O operation. 2. If both short reads and duplicated SGLs occur, the iov_len of the original SGLs must be updated properly to ensure the correct number of read data bytes is returned. 3. Since only fetched data bytes are copied back, it is unnecessary to copy the original buffer to the merged buffer before fetching. 4. A test case has been added to verify that short reads function as expected during SGL merging. Signed-off-by: Wang Shilong <shilong.wang@hpe.com>
b4b2322 to
58123b4
Compare
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-16795/2/testReport/ |
Signed-off-by: Wang Shilong <shilong.wang@hpe.com>
|
Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-16795/5/execution/node/1465/log |
| dup_buf_size -= iov->iov_buf_len; | ||
| dup_buf += iov->iov_buf_len; | ||
| D_ASSERT(dup_buf_size >= iov->iov_len); | ||
| memcpy((char *)iov->iov_buf, dup_buf, iov->iov_len); |
There was a problem hiding this comment.
Some confused here, for read case, the output lov_len depend on the read result, right? Why not copy as much data as possible as long as the output buffer is large enough? something like that:
if (iov->iov_buf_len > dup_data_len)
iov->iov_len = dup_data_len;
else
iov->iov_len = iov->iov_buf_len;
memcpy((char *)iov->iov_buf, dup_buf, iov->iov_len);
On the other hand, it the application requires only copy specified length to the given buffer which real size is large than the required length (iov->iov_len < iov->iov_buf_len), then how can we guarantee that all the fetched data can be filled into the given SGL buffer? I may miss something.
There was a problem hiding this comment.
This is what I currently follow: I think daos will not return more data than requested iov_len, iov_buf_len shall be equal or larger than iov_len, and when copying data back, real fetched data might be less than user requested iov_len. and it will not return more data then requested iov_len.
Signed-off-by: Wang Shilong <shilong.wang@hpe.com>
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-16795/8/execution/node/1379/log |
There is a failure https://daosio.atlassian.net/browse/DAOS-18011 I opened a ticket, it looks like a MD-on-SSD related bug. |
| processed_sgls_free(ctx, obj_auxi->iod_nr); | ||
| obj_auxi->rw_args.merge_ctx = NULL; | ||
| api_args = dc_task_get_args(obj_auxi->obj_task); | ||
| api_args = dc_task_get_args(obj_auxi->obj_task); |
There was a problem hiding this comment.
not from this patch, this line is a dup
When IO retries occur, a use-after-free scenario may arise during
scatter-gather list (sgl) merging. This happens because retried I/O's
sgl references dup_sgls, which could be prematurely freed in obj_reasb_io_fini().
To fix above issue, processed_sgls must not be freed during retry operations
and should be called only once per I/O operation.
If both short reads and duplicated SGLs occur, the iov_len of the original
SGLs must be updated properly to ensure the correct number of read data bytes
is returned.
A test case has been added to verify that short reads function as expected during SGL merging.
Steps for the author:
After all prior steps are complete: