Package: leafpad Version: 0.8.18.1-5 X-Debbugs-CC: bladeboy200...@gmail.com Severity: critical
Some mishandling of error return code in the saving function of leafpad can lead to data loss. When a file ($FILE) is saved in leafpad, it calls the functions file_save_real(file.c:180): - The content on the GtkTextBuffer is transformed in a gchar*, - Then the file $FILE is opened (file.c:220) in write mode (which truncate $FILE content) - The gchar* is written to $FILE (file.c:226) but not necessary yet flushed (on my computer there is an autoflush only if more than 4k of data), - Leafpad calls the function gtk_text_buffer_set_modified() (file.c:232) for the UI layer - Then the $FILE is closed (file.c:233) which causes the unflushed content to be finally written to disk. - Sadly the result of that fclose() (file.c:233) is not checked at all. --- 1) first problem, the "truncating side-effect" The call to gtk_text_buffer_set_modified() (file.c:232) triggers a "modified-changed" signal. There is a callback (view.c:354) that links that "modified-changed" signal to cb_modified_changed. -> the function cb_modified_changed(view.c:218) calls (view.c:224) the function get_file_basename() -> the function get_file_basename(file.c:42) calls (file.c:66) the function check_file_writable() -> the function check_file_writable(file.c:31) : - calls (file.c:35) the function fopen() on the same $FILE in APPEND mode to check if it's writable - and (file.c:36) that second handle on $FILE is closed Because in the file_save_real(file.c:180), gtk_text_buffer_set_modified() is called before the fclose() somehow 2 handles on $FILE exists at the same time, the second handle being closed before the first. On a local filesystem (ext4) it usually behave somehow without errors: when the file is fclosed any unflushed content is then written on disk. Here is the strace when trying to save 2 bytes in /tmp/file.txt (an ext filesystem) file.c:220:file_save_real() | open("/tmp/file.txt", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 13 file.c:035:check_file_writable() | open("/tmp/file.txt", O_WRONLY|O_CREAT|O_APPEND, 0666) = 14 file.c:036:check_file_writable() | close(14) = 0 file.c:233:file_save_real() | *write(13, "X!", 2) = 2* 🗸 file.c:233:file_save_real() | close(13) = 0 Until here it seems fine, but that can have a dramatic side-effect when used with gvfs over sftp, a bug happens! ( see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=680418 and https://bugs.launchpad.net/ubuntu/+source/leafpad/+bug/708829 ) gfvs+sftp don't like having concurrently 2 file pointers on the same file, and trying to write data on the oldest, after having closed the youngest ... fails and the file end up truncated. After having mounted the sftp://localhost/tmp/file.txt via gvfs, the same strace file.c:220:file_save_real() | open("/run/user/1000/gvfs/sftp:host=localhost,user=me/tmp/file.txt", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 13 file.c:035:check_file_writable() | open("/run/user/1000/gvfs/sftp:host=localhost,user=me/tmp/file.txt", O_WRONLY|O_CREAT|O_APPEND, 0666) = 14 file.c:036:check_file_writable() | close(14) = 0 file.c:233:file_save_real() | *write(13, "X!", 2) = -1 EOPNOTSUPP (Operation not supported) 🗴 *😱 file.c:233:file_save_real() | close(13) = 0 Solution proposed: As previously proposed in 2012 (and by Max Rockatanski <bladeboy200...@gmail.com> in 2016) on bug=680418, the call to fclose() should be done before the call to gtk_text_buffer_set_modified(). The data should also probably be fflushed and fsynced just after the fwrite. --- 2) second problem, the silent (No space left on device) The result of fclose() (file.c:233) being not checked at all, if there is not enough space on disk to write data when flushing, the remaining data is currently silently lost. For example try creating a ram drive of 4kb: mkdir -p /tmp/tmpfs4k && mount -o size=4k -t tmpfs none /tmp/tmpfs4k Then try to write 4097 bytes into /tmp/tmpfs4k/test (with a strace) : file.c:220:file_save_real() | open("/tmp/tmpfs4k/test", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 13 file.c:226:file_save_real() | write(13, "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"..., 4096) = 4096 file.c:035:check_file_writable() | open("/tmp/tmpfs4k/test", O_WRONLY|O_CREAT|O_APPEND, 0666) = 14 file.c:036:check_file_writable() | close(14) = 0 file.c:233:file_save_real() | *write(13, "!", 1) = -1 ENOSPC (No space left on device) 🗴 😱* file.c:233:file_save_real() | close(13) = 0 Solution proposed: The return code for fopen should be checked and a feedback should be provided to the user if an error occur. (and the same thing should be done if we add a fflush and a fsync)