[CDRIVER-172] mongo_read_response: insufficient memory allocated for "out" Created: 11/Oct/12  Updated: 08/Aug/13  Resolved: 08/Aug/13

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

Type: Bug Priority: Major - P3
Reporter: Marco Assignee: Gary Murakami
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

32bit Linux
Server: http://fastdl.mongodb.org/linux/mongodb-linux-i686-2.2.0.tgz
Client: https://github.com/mongodb/mongo-c-driver/zipball/v0.6



 Description   

Running "tutorial_empty_query" from http://api.mongodb.org/c/current/tutorial.html on an empty database, I see that not enough memory is allocated for "out" in mongo_read_response (src/mongo.c). I added a printf:
printf("mongo_read_response: %d required, %d allocated\n", sizeof(mongo_reply), len);
out = ( mongo_reply * )bson_malloc( len ); // was here before

And the output I get (tutorial_empty_query on empty database):
mongo_read_response: 37 required, 106 allocated
mongo_read_response: 37 required, 36 allocated // from mongo_cursor_next( cursor )

In mongo_read_response (src/mongo.c) the check for "len" is:
len < sizeof( head )+sizeof( fields )
Seems mongo_reply.objs is missing. Wouldn't something like:
len < sizeof(mongo_reply)
be better? But this is just a check that fails to catch the problem. I guess the actual problem is that the requested "len" is too small. Not sure where that comes from.



 Comments   
Comment by Gary Murakami [ 08/Aug/13 ]

While there is no functional problem, there is a poor coding practice problem that should otherwise be fixed, except that we are moving to a new C driver that is completely rewritten. Closing won't fix.

Comment by Gary Murakami [ 08/Aug/13 ]

Marco,

I finally got around to looking at this. My guess is that the original author (Kyle) added the objs field just for convenience in the code, the cursor next case with 37 versus 36 is where there are no objects in the response. The existing code makes use of "&out->objs" and "&cursor->reply->objs". This (objs) probably could be eliminated with "&(out+1)" and "&(cursor->reply+1)" after objs is removed from mongo_reply.

So while there is no functional problem here at the moment, I agree that it is not good coding practice, that objs should be removed from mongo_reply, and that this would fix the compiler complaint appropriately. However, there is a completely rewritten C driver in the works, so we're focusing work there and minimizing work on this branch.

So I'm going to close this. When the new rewritten driver is available, I would certainly welcome similar reports like this.

Thanks,

-Gary

Comment by Marco [ 23/Jan/13 ]

Are you surprised about sizeof(mongo_reply) = 37?
I'm using Ubuntu 12.10, x86, gcc 4.4.7.
Also Red Hat 6.3, x64, gcc 4.4.6 has sizeof(mongo_reply) = 37.

Comment by Gary Murakami [ 22/Jan/13 ]

Marco, thanks for your response. What environment/compiler are you using to get this report?

Comment by Marco [ 21/Jan/13 ]

Hi Gary

The code is still doing the same. With a requested "len" of 36, both versions will malloc 36:

  • old: bson_malloc( len );
  • new: bson_malloc( sizeof(mongo_reply) - sizeof(char) + len - 16 - 20 ); // 37 - 1 + 36 - 16 - 20 = 36

I'm not sure if this addresses your concern from Jan 18 09:11, but it doesn't change the situation for me.

Thanks
Marco

Comment by Gary Murakami [ 18/Jan/13 ]

Marco, please check out the above commit and see if it addresses your issue (no more warning from your C compiler runtime).

Comment by auto [ 18/Jan/13 ]

Author:

{u'date': u'2013-01-18T21:47:41Z', u'email': u'gary.murakami@10gen.com', u'name': u'Gary J. Murakami'}

Message: CDRIVER-172 mongo_read_response: insufficient memory allocated for "out"
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/98826a1eaad779ea20a5f218f5b9e9591f30fd8b

Comment by Gary Murakami [ 18/Jan/13 ]

Marco, thanks for your response. Yes, after examining this, there could be a problem. While the message length on the wire is appropriately given, the mongo_reply struct could require more space due to differences in native type sizes versus wire type sizes and struct padding/packing. I'll look into a fix.

Comment by Marco [ 08/Jan/13 ]

I found it because I'm using a C compiler respectively a runtime that warns about pointers with allocated areas that are not big enough for the type pointed to.
Since I never encountered an intentional case where insufficient memory was allocated for a struct pointer, I didn't look at the code too close before reporting. But I guess that this is the case here (no message; no memory for the "objs" field). Interesting!
So I guess this bug can be closed unless the design isn't deliberate after all.

Comment by Gary Murakami [ 07/Jan/13 ]

What leads you to think that this is an issue? Did the tutorial program crash? Did you get some diagnostic?

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