Hi,
I was told that I have to explain race conditions I found.
I. Copy file bytes
It concerns 'e_fm_op.c.diff1'. When copying a file, the dest
file is written with bytes read from source file. It only stops
when we are at the end of the source file. The problem
is that you just have to launch a program like this:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main()
{
char buf[] = "aaaaa";
FILE* f = fopen("file.txt", "wb");
if (f)
{
while(1)
fwrite(buf, sizeof(buf), 1, f);
}
fclose(f);
}
And then right-click->copy 'file.txt' to another place to cause
e to indefinitly copy the file (as it never arrives to the end
of the file).
This point is not discutable. When you copy a file, you ONLY
must copy n bytes (got by xx.st_size) of source file. It's like
in file transfer. You get size of file, you split it in blocks and
send them, and you recalculate the size of the last block to
have sent exactly the bytes of file. Otherwise, you can flood
the server.
Quick ex:
block size -> 100 b
got file size -> 246 b
So:
copy 100 b
copy 100 b
copy 46 b
That's it. The file is copied at its original size, even if it has
changed since the copy has been started.
II. Copy file (2 race conditions)
When copying a simple file, we do:
1. check type of file with stat().
2. if dest file doesn't exist
--> 2.1 if file is a regular file, then copy.
--> 2.2 ...
3. else: ask for overwrite
There is a TOCTOU.
Betwenn 2 and 3, you just have to replace the file by a symlink
to make the copy copy the file pointed by the link.
Between 1 and 2, you just have to create a symlink to the destination,
to make the copy copy the source file in the file pointed by the symlink.
You can test:
- add 'sleep(10);' to the line 1231 of 'e_fm_op.c'
- copy a file to an empty folder
- create quickly a symlink in the dest folder with the same name of the
file which will be copied, pointing to another file
- wait... and then, check the file pointed by the symlink: its content has
been overwritten by the source file of the copy
Now, keep the sleep(), apply all patches joined to this mail, and retry.
You will see that e doesn't copy file, because it detects that file type
has been changed.
I was also told that my patches are causing problems and that there are
no race condition (although if you test, you will see that there are),
so I'm
waiting for the response of raster to know what problem(s) it causes.
Maxime.
--- e_fm/e_fm_ipc.c 2012-07-22 16:47:04.000000000 +0200
+++ e_fm/e_fm_ipc.c 2012-07-22 17:05:43.138103151 +0200
@@ -1087,7 +1087,7 @@
// printf("MOD %s %3.3f\n", path, ecore_time_unix_get());
lnk = ecore_file_readlink(path);
memset(&st, 0, sizeof(struct stat));
- if (stat(path, &st) == -1)
+ if (lstat(path, &st) == -1)
{
if ((path[0] == 0) || (lnk)) broken_lnk = 1;
else return;
--- e_fm_op.c 2012-07-22 17:02:46.000000000 +0200
+++ e_fm_op.c 2012-07-22 17:14:15.199111133 +0200
@@ -33,6 +33,8 @@
#include <errno.h>
#include <limits.h>
+#include <fcntl.h>
+
#include <Ecore.h>
#include <Ecore_File.h>
@@ -81,6 +83,7 @@
static int _e_fm_op_copy_link(E_Fm_Op_Task *task);
static int _e_fm_op_copy_fifo(E_Fm_Op_Task *task);
static int _e_fm_op_open_files(E_Fm_Op_Task *task);
+static int _e_fm_op_check_race(E_Fm_Op_Task *task);
static int _e_fm_op_copy_chunk(E_Fm_Op_Task *task);
static int _e_fm_op_copy_atom(E_Fm_Op_Task *task);
@@ -118,6 +121,7 @@
{
const char *name;
struct stat st;
+ struct stat st_fs;
} src;
struct
@@ -1228,6 +1232,8 @@
_e_fm_op_open_files(E_Fm_Op_Task *task)
{
E_Fm_Op_Copy_Data *data = task->data;
+ int fd_from;
+ int fd_to;
/* Ordinary file. */
if (!data)
@@ -1240,14 +1246,43 @@
if (!data->from)
{
- data->from = fopen(task->src.name, "rb");
+ fd_from = open(task->src.name, O_RDONLY);
+ if(fd_from == -1)
+ {
+ _E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot open file '%s' for reading: %s.", task->src.name);
+ }
+
+ data->from = fdopen(fd_from, "rb");
if (!data->from)
_E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot open file '%s' for reading: %s.", task->src.name);
+
+ if (fstat(fd_from, &task->src.st_fs) == -1)
+ {
+ _E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot fstat() file '%s'.", task->src.name);
+ }
+
+ if (_e_fm_op_check_race(task))
+ {
+ fclose(data->from);
+ data->from = NULL;
+
+ FREE(task->data);
+
+ task->finished = 1;
+
+ return 1;
+ }
}
if (!data->to)
{
- data->to = fopen(task->dst.name, "wb");
+ fd_to = open(task->dst.name, O_WRONLY | O_CREAT | O_EXCL);
+ if(fd_to == -1)
+ {
+ _E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot open file '%s' for writing: %s.", task->dst.name);
+ }
+
+ data->to = fdopen(fd_to, "wb");
if (!data->to)
_E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot open file '%s' for writing: %s.", task->dst.name);
_e_fm_op_copy_stat_info(task);
@@ -1256,6 +1291,21 @@
return 0;
}
+/* check for race condition */
+static int
+_e_fm_op_check_race(E_Fm_Op_Task *task)
+{
+ if (task->src.st.st_mode != task->src.st_fs.st_mode ||
+ task->src.st.st_ino != task->src.st_fs.st_ino ||
+ task->src.st.st_dev != task->src.st_fs.st_dev)
+ {
+ _E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Race condition detected: file '%s' has changed.", task->src.name);
+ return 1;
+ }
+
+ return 0;
+}
+
static int
_e_fm_op_copy_chunk(E_Fm_Op_Task *task)
{
@@ -1273,6 +1323,11 @@
return 1;
}
+ if (_e_fm_op_check_race(task))
+ {
+ goto quit;
+ }
+
if (task->dst.done+sizeof(buf)>task->src.st.st_size)
{
dread = fread(buf, 1, task->src.st.st_size-task->dst.done, data->from);
--- e_fm_op.c 2012-07-22 09:15:29.000000000 +0200
+++ e_fm_op.c 2012-07-22 09:20:25.000000000 +0200
@@ -1273,26 +1273,21 @@
return 1;
}
- dread = fread(buf, 1, sizeof(buf), data->from);
+ if (task->dst.done+sizeof(buf)>task->src.st.st_size)
+ {
+ dread = fread(buf, 1, task->src.st.st_size-task->dst.done, data->from);
+ }
+ else
+ {
+ dread = fread(buf, 1, sizeof(buf), data->from);
+ }
+
if (dread <= 0)
{
if (!feof(data->from))
_E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot read data from
'%s': %s.", task->dst.name);
- fclose(data->from);
- fclose(data->to);
- data->to = NULL;
- data->from = NULL;
-
- _e_fm_op_copy_stat_info(task);
-
- FREE(task->data);
-
- task->finished = 1;
-
- _e_fm_op_update_progress(task, 0, 0);
-
- return 1;
+ goto quit;
}
dwrite = fwrite(buf, 1, dread, data->to);
@@ -1301,9 +1296,32 @@
_E_FM_OP_ERROR_SEND_WORK(task, E_FM_OP_ERROR, "Cannot write data to '%s':
%s.", task->dst.name);
task->dst.done += dread;
+
+ if(task->dst.done == task->src.st.st_size)
+ {
+ goto quit;
+ }
+
_e_fm_op_update_progress(task, dwrite, 0);
return 0;
+
+quit:
+
+ fclose(data->from);
+ fclose(data->to);
+ data->to = NULL;
+ data->from = NULL;
+
+ _e_fm_op_copy_stat_info(task);
+
+ FREE(task->data);
+
+ task->finished = 1;
+
+ _e_fm_op_update_progress(task, 0, 0);
+
+ return 1;
}
/*
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
