Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#193 closed defect (fixed)

Hash collision when files are dynamically deleted and created again

Reported by: c960657 Owned by: moo
Priority: minor Milestone: 1.3.0
Component: cacher Version: 1.2.1
Keywords: Cc: xcache.lighttpd.net@…
Application: PHP Version:
Other Exts: SAPI: apache2handler
Probability: Always Blocked By:
Blocking:

Description

If a file is deleted and another file is created with another name immediately after, it looks as if the new file sometimes inherits the old file's inode number.

This behaviour is illustrated by the following commands. On my system the two ls calls output identical values:

rm -f file1 file2 ; touch file1 ; ls -i file1 ; rm file1 ; touch file2 ; ls -i file2

This causes a problem with XCache that (AFAICT) uses the inode number as a cache key. Thus, if a script dynamically deletes a file, oldfile.php, and subsequently creates another, newfile.php, "include 'newfile.php';" may lead to the inclusion of the code in oldfile.php. This happens e.g. with Smarty that compiles templates to PHP file.

Here is a test case in PHP:

<?php
sleep(2);
for ($i = 0; $i < 2; $i++) {
    $file = '/tmp/file' . $i . '.php';
    file_put_contents($file, '<?php var_dump("This is ' . $file . '"); ?' . '>');
    include $file;
    $stat = stat($file);
    var_dump('Now including ' . $file . ', inode=' . $stat['ino']);
    unlink($file);
}
?>

Expected result:

string(22) "This is /tmp/file0.php"
string(43) "Now including /tmp/file0.php, inode=2277518"
string(22) "This is /tmp/file1.php"
string(43) "Now including /tmp/file1.php, inode=2277518"

Actual result:

string(22) "This is /tmp/file0.php"
string(43) "Now including /tmp/file0.php, inode=2277518"
string(22) "This is /tmp/file0.php"
string(43) "Now including /tmp/file1.php, inode=2277518"

If you remove the initial "sleep(2)" it works as expected. Without the sleep() it looks as the files are never cached (they don't show up on the XCache Administration page).

Change History (4)

comment:1 Changed 6 years ago by moo

the file is created in the same second and inode is recycled by fs when file is removed so it is same time and same inode.

this is a problem unless we hash on realpath instead of inode. i'll try

comment:2 Changed 6 years ago by c960657

  • Cc xcache.lighttpd.net@… added

I cannot claim to understand much of the code, but wouldn't it help doing something like this (untested proof of concept):

-        delta = XG(request_time) - pbuf->st_mtime;
-        if (abs(delta) < 2 && !xc_test) {
+        if (XG(request_time) - pbuf->st_mtime < 2 && pbuf->st_mtime < time(NULL)) && !xc_test) {

This would prevent caching of files created later then 2 seconds before the beginning of the current request, while still allowing caching of files with mtime in the future.

If a file has mtime < now, another file cannot appear at the same inode with the same mtime (unless the mtime of the new file is created with an mtime explicitly set to sometime in the past). Or did I miss something?

comment:3 Changed 6 years ago by moo

  • Status changed from new to accepted

XG(request_time) is a fast and stale cache of time(NULL). u may not understand this ticket, sleep(2) is not a problem, it's just a must workaround for the trick in XCache.

"XG(request_time) - pbuf->st_mtime < 2" is a trick when someone use scp or something to upload file, the file is created/overwrited but still being transfered (incompleted). 2 seconds is just a guess. and abs() is for those have system clock screwed (e.g. without cmos/hardware clock) but file time is correct (e.g.: by rsync)

-if (p->type == XC_TYPE_VAR || /* PHP */ p->mtime == xce->mtime) {
+if (p->type == XC_TYPE_VAR || /* PHP */ p->size == xce->size && p->mtime == xce->mtime) {

is a simple but not good workaround, it does not fix your reproducable script but should help if you files is not in same size. the 2 seconds check may be removed if we have p->size == xce->size check

comment:4 Changed 5 years ago by moo

  • Milestone changed from 1.3.1 to 1.3.0
  • Resolution set to fixed
  • Status changed from accepted to closed

fixed as:

if (p->type == XC_TYPE_VAR || /* PHP */ (p->mtime == xce->mtime && p->data.php->sourcesize == xce->data.php->sourcesize)) {
Note: See TracTickets for help on using tickets.