[SERVER-35769] Pointer to freed OperationContext is passed to Pipeline::dispose() Created: 23/Jun/18  Updated: 27/Oct/23  Resolved: 15/Feb/19

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Ian Boros Assignee: Anton Korshunov
Resolution: Gone away Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-37453 Delete PlanStage::dispose() Closed
Operating System: ALL
Sprint: Query 2019-02-25
Participants:

 Description   

ClusterClientCursors have a ClusterClientCursorParams:

Which has a PipelineDeleter:

https://github.com/mongodb/mongo/blob/f9f33683b6ec5995091ee214c81d75c0ab163fe4/src/mongo/s/query/cluster_client_cursor_params.h#L126-L127

Which has an OperationContext
https://github.com/mongodb/mongo/blob/f9f33683b6ec5995091ee214c81d75c0ab163fe4/src/mongo/db/pipeline/pipeline.h#L428

The _opCtx is not updated when the ClusterClientCursor is detached or reattached to an operation context.

https://github.com/mongodb/mongo/blob/f9f33683b6ec5995091ee214c81d75c0ab163fe4/src/mongo/s/query/cluster_client_cursor_impl.cpp#L124-L132

This could definitely be a use-after-free bug.



 Comments   
Comment by Anton Korshunov [ 15/Feb/19 ]

Given that:

  1. The original problem has gone away after the merge pipeline was removed from ClusterClientCursorParams in SERVER-33323.
  2. Access to the dangling pointer is protected by the dismissed flag, when the dismiss disposal mechanism is correctly used.

The consensus was that the problem does not require a fix.

Comment by Anton Korshunov [ 14/Feb/19 ]

Since  SERVER-37453 was closed as "Won't fix", we need to fix this issue with a dangling pointer, which is really a minor one.

We would never use this dangling pointer as it can only become a dangling one when we do a dismiss disposal on the deleter. However, in this case the dismissed flag would be set to true and we would never enter the block where the opCtx is passed to Pipeline::dispose(). The only potential issue is that we would have an invariant on a dangling pointer, but it would have no impact on the dispose logic (the invariant is before the if (_dismissed) block).

If the deleter is not dismissed, then the operation context cannot be changed in this codepath and we should be ok in this case.

That said, it does make sense to set the opCtx back to null whenever we dismiss disposal, to correctly maintain the invariant of the deleter.

Also be advised that the deleter has gone from ClusterClientCursorParams, as the mergePipeline field has been removed from the struct.

Comment by David Storch [ 07/Feb/19 ]

anton.korshunov, I believe that this issue will go away after your work for SERVER-37453. If so, then please close this ticket as "Gone Away" once the changes for SERVER-37453 land in master.

Comment by Ian Boros [ 25/Jun/18 ]

After looking into this some more, it seems like dispose() when called on mongos does not seem to actually use the OperationContext. We're passing around a pointer to garbage, but don't seem to be accessing it. This is way less urgent than I thought!

Comment by Ian Boros [ 25/Jun/18 ]

david.storch Looks like it affects 3.6 and 4.0, as they both store a pipeline in the ClusterClientCursorParams.

Comment by David Storch [ 25/Jun/18 ]

ian.boros what versions does this affect? Was this introduced during 4.0 development?

Generated at Thu Feb 08 04:40:54 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.