Created attachment 917784 [details] valgrind trace Description of problem: cups-browsed leaks a lot of memory (similar to #1027317 Version-Release number of selected component (if applicable): cups-filters-1.0.53-5.fc20.x86_64 How reproducible: always Steps to Reproduce: 1. run cups-browsed service 2. watch the memory use grow 3. Actual results: Expected results: Additional info: I've attached valgrind trace of cups-browse running overnight
(In reply to Jan Vesely from comment #0) > I've attached valgrind trace of cups-browse running overnight Thanks Jan, there seems to be missing debuginfo for cups-filters package - see the '??? (in /usr/sbin/cups-browsed)' in the trace. I don't see any leaks in my instance of cups-browsed so I need you to install it with 'debuginfo-install cups-filters' and try to run it again. Thank you.
Created attachment 918234 [details] valgrind trace this trace has all the symbol information, hope it helps
Created attachment 918340 [details] cups-browsed-memory-leak.patch Thanks. Fix sent upstream.
Created attachment 944317 [details] another fix (In reply to Tim Waugh from comment #3) > Created attachment 918340 [details] > cups-browsed-memory-leak.patch > > Thanks. > > Fix sent upstream. Hi, is there a way to track this? I could not find the patch on cups-filters ML, or fedora pkg-git (maybe i need to look harder). One question about the patch. Wouldn't ippDelete on line 749 lead to a possible use-after-free? if 'printer-name' is not in the first sequence of IPP_TAG_PRINTER attributes (and the outer most for loop does another iteration) I don't know whether such response format is possible.
Oh, yes, it would have lead to a use-after-free, thanks for spotting it. I think this patch should be enough to fix 1.0.60: --- cups-filters-1.0.60/utils/cups-browsed.c~ 2014-10-01 19:25:42.000000000 +0100 +++ cups-filters-1.0.60/utils/cups-browsed.c 2014-10-08 11:39:24.754004293 +0100 @@ -761,6 +761,7 @@ gboolean handle_cups_queues(gpointer unu httpClose(http); /* Schedule the removal of the queue for later */ p->timeout = current_time + TIMEOUT_RETRY; + ippDelete(response); break; } if (response) Basically we just need to make sure to ippDelete(response) once the printer-name attribute has been evaluated. What do you think?
CCing Till for review.
Created attachment 944944 [details] memory-leak-fix.bundle Here's that change as a bzr bundle.
Tim, thank you for the patch. It is absolutely correct. I have applied it upstream on the BZR repository (rev. 7297) now, so the change will be part of the 1.0.61 release.
Thanks.