After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 730223 - Data loss: housekeeping plugin deletes files that aren't in /tmp
Data loss: housekeeping plugin deletes files that aren't in /tmp
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gio
2.40.x
Other Linux
: Normal major
: ---
Assigned To: gtkdev
gtkdev
: 737079 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-05-15 23:05 UTC by Tim Cuthbertson
Modified: 2018-05-24 16:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
housekeeping: Don't follow symlinks to subdirectories (3.54 KB, patch)
2014-05-19 11:07 UTC, Bastien Nocera
committed Details | Review

Description Tim Cuthbertson 2014-05-15 23:05:54 UTC
Thankfully, in my case most files were under source control so I only lost a small amount of data due to this bug. But needless to say, this bug could be **catastrophic** in some circumstances.

I'm running fedora 20, gnome-settings-daemon-3.10.2-3.fc20.x86_64

To reproduce:

$ mkdir ~/Desktop/some_files
$ touch ~/Desktop/some_files/important.txt
$ ln -s ~/Desktop/some_files/ /tmp/some_files

$ ll ~/Desktop/some_files/
-rw-r--r-- 1 tim tim 0 May 16 08:52 important.txt

$ dbus-send --session --dest=org.gnome.SettingsDaemon --print-reply /org/gnome/SettingsDaemon/Housekeeping org.gnome.SettingsDaemon.Housekeeping.RemoveTempFiles
method return sender=:1.10 -> dest=:1.192 reply_serial=2
                                                                                $ ll ~/Desktop/some_files/
# (all gone)


Why did I do this?

I was testing some development stuff, in which I was copying files into a location under /tmp. Then I decided to just use a symlink, so I didn't need to keep the files in sync. Some time later (after a periodic task kicked in, I assume) `git status` shows that a startling number of files have simply disappeared from my workspace. Not all of them (for some reason the .git directory itself did not get removed), but enough to terrify me.
Comment 1 Bastien Nocera 2014-05-16 16:32:45 UTC
I believe that this might be related to /tmp being on tmpfs. I'd need to check again, but I don't think I can reproduce this when using /var/tmp instead.

commit c296a73cec417752e3ff538c6719a74348fcb6e7
Author: Bastien Nocera <hadess@hadess.net>
Date:   Fri May 16 18:30:06 2014 +0200

    housekeeping: Fix warnings when info cannot be gathered

commit 82c91ff6f929a534e26cb70dbd35d344847feb44
Author: Bastien Nocera <hadess@hadess.net>
Date:   Fri May 16 16:39:26 2014 +0200

    housekeeping: Create test program for bug 730223
    
    $ mkdir /tmp/gsd-purge-temp-test
    $ mkdir /tmp/important/
    $ touch /tmp/important/dont-delete.txt
    $ ln -s /tmp/important/ /tmp/gsd-purge-temp-test/
    $ ls -l /tmp/gsd-purge-temp-test/
    0 lrwxrwxrwx. 1 hadess hadess 15 May 16 16:41 important -> /tmp/important/
    $ ./gsd-purge-temp-test
    <ctrl+c>
    $ ls -l /tmp/gsd-purge-temp-test/
    total 0
    $ ls -l /tmp/important/
    total 0

commit 56dc72152d3e4f64a77419c8e5fcc52c78793702
Author: Bastien Nocera <hadess@hadess.net>
Date:   Fri May 16 16:38:57 2014 +0200

    housekeeping: Make some functions public to allow debugging
Comment 2 Bastien Nocera 2014-05-19 10:59:58 UTC
Using the same directory structure as in the commit mentioned above, we get with this test program:
** Message: important is of type: 3

Which means that we detect the symlink as G_FILE_TYPE_DIRECTORY even though it links to a sub-directory. This smells of a GIO bug.

#include <gio/gio.h>

int main (int argc, char **argv)
{
        GFileEnumerator *e; 
        GFile *file;
        GError *error = NULL;
        GFileInfo *info;

        file = g_file_new_for_path ("/tmp/gsd-purge-temp-test/");
        e = g_file_enumerate_children (file,
                                       G_FILE_ATTRIBUTE_STANDARD_NAME "," 
                                       G_FILE_ATTRIBUTE_STANDARD_TYPE,
                                       G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
                                       NULL,
                                       &error);
        if (!e) {
                g_warning ("Couldn't enum children: %s", error->message);
                return 1;
        }

        while ((info = g_file_enumerator_next_file (e, NULL, NULL)) != NULL) {
                GFileType type = g_file_info_get_file_type (info);

                g_message ("%s is of type: %d", g_file_info_get_name (info), type);
        }

        return 0;
}
Comment 3 Bastien Nocera 2014-05-19 11:07:25 UTC
Created attachment 276762 [details] [review]
housekeeping: Don't follow symlinks to subdirectories

Check whether a leaf node is a symlink before processing it, and if it
is, check whether we should delete the link itself.

This works around a possible bug in GIO where GFileEnumerators will get
the filetype of the linked-to item, instead of detecting that it is a
symlink.
Comment 4 Rui Matos 2014-05-19 13:18:15 UTC
Review of attachment 276762 [details] [review]:

This seems like it should work but I don't like the structure of the code. We're calling g_file_delete() in two places already and this adds a third.

The equivalent of what nautilus does would be calling g_file_delete() right at delete_recursively_by_age() and then just recurse if the deletion fails with G_IO_ERROR_NOT_EMPTY . The rest of callbacks would only do the recurse itself with the actual deletion happening only on this one place.
Comment 5 Rui Matos 2014-05-19 15:04:24 UTC
Review of attachment 276762 [details] [review]:

Ok, my patch wasn't going to get simpler than this after all, so let's go with this one.

::: plugins/housekeeping/gsd-disk-space.c
@@ +446,3 @@
+        } else if (type == G_FILE_TYPE_SYMBOLIC_LINK) {
+                if (should_purge_file (data->file, data->cancellable, data->old)) {
+                        if (!data->dry_run) {

Please add the same g_debug() that is used in other places here.
Comment 6 Bastien Nocera 2014-05-19 15:07:48 UTC
Comment on attachment 276762 [details] [review]
housekeeping: Don't follow symlinks to subdirectories

Committed to gnome-3-8, gnome-3-10, gnome-3-12 and master.
Comment 7 Bastien Nocera 2014-05-19 15:08:35 UTC
Reassigning to gio to get the bug mentioned in comment 2.
Comment 8 Ray Strode [halfline] 2014-05-19 15:31:58 UTC
Does the problem still happen if you change "/tmp/gsd-purge-temp-test/" to "/tmp/gsd-purge-temp-test" ?

I think "/tmp/gsd-purge-temp-test/" is shorthand for "/tmp/gsd-purge-temp-test/." not an alternative way to write "/tmp/gsd-purge-temp-test"
Comment 9 Thomas Wendt 2014-05-19 22:17:16 UTC
Yikes! Looks like I was hit hard by this one. Now I at least know what happened. 

But due to this bug I discovered another bug. Something seems to leak file descriptors. I have thousands of lines in my journal about "Failed to enumerate children of /tmp/wine/dosdevices/z:/xyz: Too many open files" (small excerpt http://pastebin.com/raw.php?i=RhNkXfHh). I don't know if this is a bug in the housekeeping plugin or gio. So I'm just leaving a comment here.
z: was a symlink to / (yay!) so there are also lots of error messages about missing permission and also many 'file not found' errors.

I was lucky to hit the open files limit :). It could have caused way more damage otherwise. It's also quite bizarre because the housekeeping plugin seems to leave empty folders everywhere. 

I'm happy to provide more information.
Comment 10 Bastien Nocera 2014-05-20 07:19:53 UTC
(In reply to comment #8)
> Does the problem still happen if you change "/tmp/gsd-purge-temp-test/" to
> "/tmp/gsd-purge-temp-test" ?

The local handling code in GIO uses ->d_type, which might, or might not be set correctly depending on the filesystem. See a work-around here:
http://cgit.freedesktop.org/systemd/systemd/tree/src/shared/util.c#n4443

Compared to the naive version in GIO:
https://git.gnome.org/browse/glib/tree/gio/glocalfileenumerator.c#n328
Comment 11 Allison Karlitskaya (desrt) 2014-06-23 13:31:20 UTC
(In reply to comment #2)
> Using the same directory structure as in the commit mentioned above, we get
> with this test program:
> ** Message: important is of type: 3
> 
> Which means that we detect the symlink as G_FILE_TYPE_DIRECTORY even though it
> links to a sub-directory. This smells of a GIO bug.

3 is actually G_FILE_TYPE_SYMBOLIC_LINK.  2 is directory.

Check gvfs-ls -a standard::type -n /tmp/gsd-purge-temp-test

This shows it properly reported as a symbolic link.  I don't think that this is a GIO bug.
Comment 12 Rui Matos 2014-09-22 16:08:33 UTC
*** Bug 737079 has been marked as a duplicate of this bug. ***
Comment 13 GNOME Infrastructure Team 2018-05-24 16:32:45 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/877.