More array abstraction use in ptp2 #1040
Conversation
To be able to use the array macros in PTP and GP error "contexts", this makes translate_ptp_result() tolerate both "types" of errors. I added a comment regarding the use of the GP_ERROR_NO_MEMORY use. This is related to gphoto#1008.
…leaks ptp_free_object was leaking the memory of the mtp_props member and ptp_ptp_free_devicepropvalue was leaking array memory of certain types.
This prepares the merging of the dpd_cache and the canon_props members of PTPParams.
Now dpd_cache and canon_props have the same type and can be merged.
This reduces code bloat and hopefully helps with not forgetting to call the destructor of freed objects before freeing the array.
This also fixes a likely memcopy out of bounds error, see TODO comment.
Make it more obvious that array_extend (now array_extend_capacity) only increases the allocated memory but does not change the len parameter.
| /* FIXME: GP_ERROR_NO_MEMORY is used to communicate two completely differnt | ||
| * things, which have nothing to do with each other: | ||
| * - the camera went out of memory because the storage space ran out, this | ||
| * is totally "normal" | ||
| * - a malloc on the host computer failed: this will completely interrupt | ||
| * the functionality and likely crash the process soonish | ||
| */ |
There was a problem hiding this comment.
Perhaps it could be fixed alongside other changes?
|
This causes problems found in fuzzing. ==2477413==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf1d03bf4 at pc 0xf2529aff bp 0xffff3f38 sp 0xffff3f2c 0xf1d03bf4 is located 0 bytes after 500-byte region [0xf1d03a00,0xf1d03bf4) SUMMARY: AddressSanitizer: heap-buffer-overflow ptp2/library.c:8034 in find_child |
|
At a first glance, I assume this is caused by the fact that I'll look into this soon. Can you maybe give me a hint on how to reproduce your fuzzing? |
|
I debugged a bit, and it might have been there before already. The ptp_object_want call ptp_getobjectinfo and that fails in the fuzzer run, and that causes the object to be removed from the list in ptp_object_want (instead of the assumption of always adding to the list). I think this removal is risky. perhaps better mark it as "no longer present" object or so (or one that can reappear). |
|
Reproducer: in libgphoto2/libgphoto2_port/vusb/vcamera.h change: -#undef FUZZING unzip crash.zip examples/.libs/sample-afl crash The object array is initially 8 entries, but the find_child loop shrinks it to 4 and then crashes. I kind of feel the loop tries to remove TWO entries at once somehow, so the ENTRY.val+ENTRY.len!=PTR end of array check does not trigger and it overreads the list. |
That was exactly my thinking as well. And I thought about splitting |
seems to "workaround" it. |
More use of the new array abstraction in the ptp2 camlib and some minor cleanups along the way. The PTPParams struct is now completely converted to use the array macros. Also more preparation steps towards merging
dpd_cacheandcanon_props.There is a new
TODOand aFIXMEcomment related to old/potential bugs.