GNOME Bugzilla – Bug 773587
[PATCH] recent-manager: Add a limit to the list's size
Last modified: 2016-11-14 19:28:08 UTC
Created attachment 338615 [details] master patch This fixes a DOS where any app can cause all running gtk apps to use arbitrary amounts of memory. Originally reported against mate-panel, where running a big slideshow in eye-of-mate caused increasing RAM usage in mate-panel. This affects all active gtk versions. I'm attaching patches for master, gtk-3-22, and gtk-2-24 branches.
Created attachment 338616 [details] [review] gtk2 patch
Created attachment 338617 [details] [review] gtk3 patch
We discussed this on IRC, so here's a summary: * a new GtkSettings is problematic on API/feature frozen branches (2.24, 3.22) * GtkSettings are also not a good way to control potential DoS scenarios * the DoS is still going to be a problem until applications are untrusted by default, and executed into a sandbox, so adding a limit is just a temporary mitigation * a hard-coded limit to an arbitrary large number of items when saving the list is perfectly acceptable; 10000 items seems to be a good ballpark figure in order to keep the parsing fast * any consumer of the recently used files list should use its own limit to match the UI — e.g. a menu should only load 10/15 items * any tool that adds files to the recently used files list should also exercise some caution when adding items to the list; automatically adding thousands or more items in a single session would make the list completely useless anyway
Created attachment 338627 [details] [review] gtk2 patch Posting v2 with a hardcoded 1k value, as discussed.
Created attachment 338628 [details] [review] gtk3 patch
Created attachment 338629 [details] [review] master patch
ebassi requested memory information; it's not strictly related to this bug, but writing it here so that it's permanently saved somewhere. * mate-panel uses a gtkRecentChooserMenu, without setting a limit * it's this menu that's responsible for most of the RAM waste * with 2940 images in recent files, the menu used 7.15mb of RAM * that makes 2552 bytes per entry * 1024 bytes of that is in the mime type icon For some reason, the mime type icons were not shared, each jpg image had its own copy in memory. This is then multiplied for each active gtk app using the menu. At 10k items, it would be 24.3mb per application. IMHO that is too much, at 1k it'd be 2.4mb per app, a more palatable number.
This is in addition to the 104-byte-sized _GtkRecentInfo, which at 10k would be 1mb and at 1k 100kb.
Adding info on the dynamic allocations of GtkRecentInfo. From the same run as the above: * 930602 bytes were in the GtkRecentInfo allocations (including themselves at 104 bytes/piece) * that's ~316 bytes per entry So to wrap up, every gtk application using recent files would take 3mb at 10k, or 300kb at 1k. If they also use the recent chooser menu, add the numbers from two comments above.
Weekly ping.
plesae don't do that. it just adds to the noise
Sorry. How do you prefer to get reminders so this doesn't get forgotten and bitrot?
The commit: https://git.gnome.org/browse/gtk+/commit/?h=gtk-3-22&id=bf560369f2401e2cdd16f962f248e98c890835d0 introduced a new memory leak: https://git.gnome.org/browse/gtk+/tree/gtk/gtkrecentmanager.c?h=gtk-3-22#n1481 the early return code path skips the g_strfreev at the end of the function. Another memory leak dismissing filechoosers is here: https://git.gnome.org/browse/gtk+/tree/gtk/gtkfilesystemmodel.c?h=gtk-3-22#n1285 when error matches CANCELLED error is not freed. Another one is here: https://git.gnome.org/browse/gtk+/tree/gtk/gtkpathbar.c?h=gtk-3-22#n1670 'file_info' is not '..finished' when g_error_matches returns TRUE
Thanks for noticing! I'm attaching patches for that. Should I open a new bug for these? The others weren't by me, these only fix the one I caused.
Created attachment 339598 [details] [review] gtk2: Fix the memory leak
Created attachment 339599 [details] [review] gtk3: Fix the memory leak
Created attachment 339600 [details] [review] master: Fix the memory leak
Review of attachment 339598 [details] [review]: ok
Review of attachment 339599 [details] [review]: ok
Review of attachment 339600 [details] [review]: ok