Skip to content

osdc: Update CB_ObjectOperation to use value types for patterns#64956

Merged
SrinivasaBharath merged 1 commit intoceph:mainfrom
edwinzrodriguez:ceph-wip-72403
Oct 27, 2025
Merged

osdc: Update CB_ObjectOperation to use value types for patterns#64956
SrinivasaBharath merged 1 commit intoceph:mainfrom
edwinzrodriguez:ceph-wip-72403

Conversation

@edwinzrodriguez
Copy link
Contributor

The callback object would create a local variable then save the address of the
local variable in the object which will later be referenced. This leads
to access stack memory that is no longer in scope,
possibly not even in the same thread.

Fixes: https://tracker.ceph.com/issues/72403

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@edwinzrodriguez edwinzrodriguez requested review from a team as code owners August 11, 2025 18:31
@edwinzrodriguez edwinzrodriguez changed the title osdc: Update CB_ObjectOperation_decodevals and CB_ObjectOperation_decodekeys to use value types for pattrs osdc: Update CB_ObjectOperation to use value types for patterns Aug 11, 2025
@adamemerson adamemerson removed request for a team August 11, 2025 19:55
@adamemerson
Copy link
Contributor

jenkins retest this please

struct CB_ObjectOperation_decodevals {
uint64_t max_entries;
Vals* pattrs;
Vals pattrs;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this works. The pointer is an out parameter.

if (r >= 0) {
auto p = bl.cbegin();
try {
if (pattrs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This checks whether the pointer is null and doesn't decode to it if so. You found a bug, but the right fix is to get rid of the uses of &ignore and pass in a null pointer instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be fair, the compiler found the bug :)
I simplified the use of the temp by keeping it in the object. There's probably a more elegant way to do this but it would involve a much greater change

Copy link
Contributor

Choose a reason for hiding this comment

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

The bug is in passing something on the stack to it. We still need to be able to write to an object given its address. The pointer value should be a pointer value, we just shouldn't be giving it an address to something on the stack.

@edwinzrodriguez edwinzrodriguez force-pushed the ceph-wip-72403 branch 3 times, most recently from 376b231 to 56e7f8f Compare August 18, 2025 13:53
@edwinzrodriguez edwinzrodriguez force-pushed the ceph-wip-72403 branch 3 times, most recently from ace3fe7 to 67241c3 Compare August 25, 2025 17:23
…odekeys to persist temp val

The callback object would create a local variable then save the address of the
local variable in the object which will later be referenced. This leads
to access stack memory that is no longer in scope,
possibly not even in the same thread.

Fixes: https://tracker.ceph.com/issues/72403
Signed-off-by: Edwin Rodriguez <edwin.rodriguez1@ibm.com>
@anrao19
Copy link
Contributor

anrao19 commented Oct 22, 2025

Tested from RGW side, and got approval from @ivancich , please find details in bz : https://tracker.ceph.com/issues/73576
so ACK from RGW

@ljflores
Copy link
Member

@SrinivasaBharath SrinivasaBharath merged commit df00e59 into ceph:main Oct 27, 2025
13 checks passed
@github-actions
Copy link

This is an automated message by src/script/redmine-upkeep.py.

I found one or more Fixes: tags in the commit messages in

git log df00e598d2dae0e73cc413977be2cd615eeb909c^..df00e598d2dae0e73cc413977be2cd615eeb909c

The referenced tickets are:

Those tickets do not reference this merged Pull Request. If this Pull Request merge resolves any of those tickets, please update the "Pull Request ID" field on each ticket. A future run of this script will appropriately update them.

Update Log: https://github.com/ceph/ceph/actions/runs/18838045001

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants