[CDRIVER-1192] Leak in _mongoc_cursor_cursorid_init_with_reply Created: 01/Apr/16  Updated: 10/Aug/16  Resolved: 18/Apr/16

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 1.4.0
Fix Version/s: 1.4.0

Type: Bug Priority: Major - P3
Reporter: A. Jesse Jiryu Davis Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by PHPC-542 Remove dependency on mongoc private s... Closed
is depended on by PHPC-629 Upgrade libbson and libmongoc to 1.4.0 Closed
Related
related to CDRIVER-1092 mongoc_cursor_new_from_command_reply Closed

 Description   

Leak in unreleased code. Affects mongoc_cursor_new_from_command_reply and other command-to-cursor paths, possibly including "find" commands.

The problem with mongoc_cursor_new_from_command_reply goes like this:

mongoc_cursor_t *cursor;
bson_t *reply = bson_new ();
 
run_command (...., reply);
cursor = mongoc_cursor_new_from_command_reply (
   client,
   reply,
   server_id);
mongoc_cursor_destroy (cursor);

1. Create "reply" with bson_new. Its "STATIC" flag is unset to indicate this is heap-allocated.
2. Pass "reply" as an out-pointer to run_command.
3. run_command assumes reply is uninitialized. When it calls bson_copy_to to copy the command reply to the bson_t, bson_copy_to initializes reply, including setting its STATIC flag.
4. Pass reply to mongoc_cursor_new_from_command_reply.
5. When the cursor is destroyed it calls bson_destroy on the bson_t, but since its STATIC flag is set, only the bson_t's contents are freed, not the struct itself.

One solution is to stack-allocate "reply". But then the cursor must copy reply's contents in order to own them, which is a very expensive way to do every query. So in addition, we need:

/* moves src's internal buffer to dst, destroys src */
bson_copy_with_steal (bson_t *src, bson_t *dst)

This way, "reply" is stack allocated, run_command can correctly assume it's uninitialized and set its STATIC flag, and then mongoc_cursor_new_from_command_reply can take ownership of reply's contents without copying them.



 Comments   
Comment by Githook User [ 18/Apr/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1192 command cursor leaks reply
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/0e76d9b6902d79248a37023085059faca6347fe7

Comment by Githook User [ 14/Apr/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1192 leak in bson_steal()
Branch: master
https://github.com/mongodb/libbson/commit/bdc6dea923005f0a68df2ca13f8e98cb207dcde1

Comment by Githook User [ 14/Apr/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1192 use after free in bson_steal test
Branch: master
https://github.com/mongodb/libbson/commit/e95012b5605bbeac20719a9a0dfa5815646a2e10

Comment by Githook User [ 12/Apr/16 ]

Author:

{u'username': u'jmikola', u'name': u'Jeremy Mikola', u'email': u'jmikola@gmail.com'}

Message: CDRIVER-1192: Remove unused variables in bson_steal() (#157)
Branch: master
https://github.com/mongodb/libbson/commit/bab9f5615699e4bb751c96571965eda581405f64

Comment by Githook User [ 12/Apr/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1192 bson_steal()
Branch: master
https://github.com/mongodb/libbson/commit/1c3095e73aca60d5e87520e8dd386b3f008d4235

Generated at Wed Feb 07 21:11:50 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.