{"thread":{"id":"61670","subject":"[PATCH] object-file: fix leak on conversion failure","startedAt":"2024-06-22T04:36:54Z","lastAt":"2024-06-24T16:08:07Z","messageCount":3,"participants":["Eric Wong","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"497534","messageId":"20240622043648.M78681@dcvr","threadId":"61670","inReplyTo":null,"subject":"[PATCH] object-file: fix leak on conversion failure","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2024-06-22T04:36:48Z","receivedAt":"2024-06-22T04:36:54Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"I'm not sure exactly how to trigger the leak, but it seems fairly\nobvious that the `content' buffer should be freed even if\nconvert_object_file() fails.  Noticed while working in this area\non unrelated things.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n object-file.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex d3cf4b8b2e..00c8f1039b 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1711,9 +1711,9 @@ static int oid_object_info_convert(struct repository *r,\n \t\t\tret = convert_object_file(&outbuf,\n \t\t\t\t\t\t  the_hash_algo, input_algo,\n \t\t\t\t\t\t  content, size, type, !do_die);\n+\t\t\tfree(content);\n \t\t\tif (ret == -1)\n \t\t\t\treturn -1;\n-\t\t\tfree(content);\n \t\t\tsize = outbuf.len;\n \t\t\tcontent = strbuf_detach(&outbuf, NULL);\n \t\t}\n"},{"id":"497549","messageId":"1f536f91-1dce-4156-98f2-4059cd5235ff@gmail.com","threadId":"61670","inReplyTo":"20240622043648.M78681@dcvr","subject":"Re: [PATCH] object-file: fix leak on conversion failure","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-06-23T16:34:51Z","receivedAt":"2024-06-23T16:34:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/22/24 12:36 AM, Eric Wong wrote:\n> I'm not sure exactly how to trigger the leak, but it seems fairly\n> obvious that the `content' buffer should be freed even if\n> convert_object_file() fails.  Noticed while working in this area\n> on unrelated things.\n\nDefinitely a good thing to include, even if it is unlikely to\nbe hit frequently in common scenarios.\n\n>   \t\t\tret = convert_object_file(&outbuf,\n>   \t\t\t\t\t\t  the_hash_algo, input_algo,\n>   \t\t\t\t\t\t  content, size, type, !do_die);\n> +\t\t\tfree(content);\n>   \t\t\tif (ret == -1)\n>   \t\t\t\treturn -1;\n> -\t\t\tfree(content);\n\nI looked at the context of this function to see that 'content'\nwas local to the method, so was not \"owned\" by something outside\nof the method that might expect to reuse the buffer on failure.\n\nThanks,\n-Stolee\n"},{"id":"497575","messageId":"xmqq8qyue330.fsf@gitster.g","threadId":"61670","inReplyTo":"1f536f91-1dce-4156-98f2-4059cd5235ff@gmail.com","subject":"Re: [PATCH] object-file: fix leak on conversion failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-24T16:08:03Z","receivedAt":"2024-06-24T16:08:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 6/22/24 12:36 AM, Eric Wong wrote:\n>> I'm not sure exactly how to trigger the leak, but it seems fairly\n>> obvious that the `content' buffer should be freed even if\n>> convert_object_file() fails.  Noticed while working in this area\n>> on unrelated things.\n>\n> Definitely a good thing to include, even if it is unlikely to\n> be hit frequently in common scenarios.\n>\n>>   \t\t\tret = convert_object_file(&outbuf,\n>>   \t\t\t\t\t\t  the_hash_algo, input_algo,\n>>   \t\t\t\t\t\t  content, size, type, !do_die);\n>> +\t\t\tfree(content);\n>>   \t\t\tif (ret == -1)\n>>   \t\t\t\treturn -1;\n>> -\t\t\tfree(content);\n>\n> I looked at the context of this function to see that 'content'\n> was local to the method, so was not \"owned\" by something outside\n> of the method that might expect to reuse the buffer on failure.\n\nThanks, both.\n\nWill queue.\n\n"}]}