getdir can infinite loop with rm -rf

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

getdir can infinite loop with rm -rf

John Muir-3
Hi,

I know that I should update my program and use the new *dir interfaces,
but there is a bug with the getdir, or the new mechanism if you use a
zero offset (not sure?).

Try this with a program that uses the old getdir interface:

    mkdir test ; touch test/one test/two test/three ; rm -rf test

I think what is happening is that rm -rf will list the directory, remove
the contents, rewind the directory, list the directory, remove any
remaining (newly created) files, and loop like that until the directory
is empty.

Unfortunately, inside libfuse's do_readdir, if the directory file handle
buffer is already filled, it simply returns with the old contents of the
directory, and not the updated contents which result from the removal of
the contents of that directory.

Of course, if you use the readdir implementation and accept offsets,
this problem does not occur.

The fix that I've attached will mark the buffer 'filled' to 0 when the
entire contents of the buffer have been read. This is of course still
only works if the entire directory is read, so we're still better off
using the new readdir interfaces.

The attached patch applies to 2.3.0, but it looks like the same problem
could occur in your current CVS view, but the patch would be slightly
different.

John.

--
John Muir
NORTEL
[hidden email]


diff -Nur fuse-2.3.0/lib/fuse.c.orig fuse-2.3.0/lib/fuse.c
--- fuse-2.3.0/lib/fuse.c.orig 2005-08-18 14:53:21.000000000 -0400
+++ fuse-2.3.0/lib/fuse.c 2005-08-18 14:56:53.000000000 -0400
@@ -1779,6 +1779,13 @@
                 size = dh->len - arg->offset;
             buf = dh->contents + arg->offset;
         }
+        else
+        {
+            /* Mark the directory buffer empty in case the directory
+               is kept open, and the process tries reading it again
+               as in rm -rf */
+            dh->filled = 0;
+        }
     } else {
         size = dh->len;
         buf = dh->contents;
Reply | Threaded
Open this post in threaded view
|

Re: getdir can infinite loop with rm -rf

Miklos Szeredi
> I know that I should update my program and use the new *dir interfaces,
> but there is a bug with the getdir, or the new mechanism if you use a
> zero offset (not sure?).
>
> Try this with a program that uses the old getdir interface:
>
>     mkdir test ; touch test/one test/two test/three ; rm -rf test

Works for me.

What kind of 'rm' do you use?

$ rm --version
rm (coreutils) 5.2.1

> I think what is happening is that rm -rf will list the directory, remove
> the contents, rewind the directory, list the directory, remove any
> remaining (newly created) files, and loop like that until the directory
> is empty.

This would be buggy as it is, because in some cases unlink() returning
success, doesn't necessarily mean, that the file was really deleted
(e.g. open file under NFS).

> Unfortunately, inside libfuse's do_readdir, if the directory file handle
> buffer is already filled, it simply returns with the old contents of the
> directory, and not the updated contents which result from the removal of
> the contents of that directory.

Yes, it could be a bit more clever about caching the directory
contents.  However, that would present other problems: the posisions
obtained by telldir() would no longer be valid after a directory
modification.

> Of course, if you use the readdir implementation and accept offsets,
> this problem does not occur.
>
> The fix that I've attached will mark the buffer 'filled' to 0 when the
> entire contents of the buffer have been read. This is of course still
> only works if the entire directory is read, so we're still better off
> using the new readdir interfaces.
>
> The attached patch applies to 2.3.0, but it looks like the same problem
> could occur in your current CVS view, but the patch would be slightly
> different.

I'd rather not add this until we are sure where the problem lays.

Thanks,
Miklos


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: getdir can infinite loop with rm -rf

Miklos Szeredi
> > The attached patch applies to 2.3.0, but it looks like the same problem
> > could occur in your current CVS view, but the patch would be slightly
> > different.
>
> I'd rather not add this until we are sure where the problem lays.

OK, looked at the SUSv3 standard, and it does specify the behavior on
rewinddir():

   It shall also cause the directory stream to refer to the current
   state of the corresponding directory, as a call to opendir() would
   have done.

Also offsets returned by telldir() may become stale after a
rewinddir():

   If the value of loc was not obtained from an earlier call to
   telldir(), or if a call to rewinddir() occurred between the call to
   telldir() and the call to seekdir(), the results of subsequent
   calls to readdir() are unspecified.

So let's reset dh->filled, when offset is zero (after opendir() and
rewinddir()).  That should make it conforming, and so hopefully also
fix the 'rm -rf' issue.

Can you please verify the following patch against 2.3.0?

Thanks,
Miklos

Index: lib/fuse.c
===================================================================
RCS file: /cvsroot/fuse/fuse/lib/fuse.c,v
retrieving revision 1.113
diff -u -p -r1.113 fuse.c
--- lib/fuse.c 27 May 2005 09:12:42 -0000 1.113
+++ lib/fuse.c 19 Aug 2005 11:04:53 -0000
@@ -1767,6 +1767,11 @@ static void do_readdir(struct fuse *f, s
     unsigned char *buf = NULL;
 
     pthread_mutex_lock(&dh->lock);
+    /* According to SUS, directory contents need to be refreshed on
+       rewinddir() */
+    if (!arg->offset)
+        dh->filled = 0;
+
     if (!dh->filled) {
         err = readdir_fill(f, in, arg, dh);
         if (err)



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: getdir can infinite loop with rm -rf

John Muir-3
Miklos,

Sorry for the long delay in my reply. Other priorities.

I have tested your patch, and it works just fine.

Thanks!

John.


Miklos Szeredi wrote:

>>>The attached patch applies to 2.3.0, but it looks like the same problem
>>>could occur in your current CVS view, but the patch would be slightly
>>>different.
>>>      
>>>
>>I'd rather not add this until we are sure where the problem lays.
>>    
>>
>
>OK, looked at the SUSv3 standard, and it does specify the behavior on
>rewinddir():
>
>   It shall also cause the directory stream to refer to the current
>   state of the corresponding directory, as a call to opendir() would
>   have done.
>
>Also offsets returned by telldir() may become stale after a
>rewinddir():
>
>   If the value of loc was not obtained from an earlier call to
>   telldir(), or if a call to rewinddir() occurred between the call to
>   telldir() and the call to seekdir(), the results of subsequent
>   calls to readdir() are unspecified.
>
>So let's reset dh->filled, when offset is zero (after opendir() and
>rewinddir()).  That should make it conforming, and so hopefully also
>fix the 'rm -rf' issue.
>
>Can you please verify the following patch against 2.3.0?
>
>Thanks,
>Miklos
>
>Index: lib/fuse.c
>===================================================================
>RCS file: /cvsroot/fuse/fuse/lib/fuse.c,v
>retrieving revision 1.113
>diff -u -p -r1.113 fuse.c
>--- lib/fuse.c 27 May 2005 09:12:42 -0000 1.113
>+++ lib/fuse.c 19 Aug 2005 11:04:53 -0000
>@@ -1767,6 +1767,11 @@ static void do_readdir(struct fuse *f, s
>     unsigned char *buf = NULL;
>
>     pthread_mutex_lock(&dh->lock);
>+    /* According to SUS, directory contents need to be refreshed on
>+       rewinddir() */
>+    if (!arg->offset)
>+        dh->filled = 0;
>+
>     if (!dh->filled) {
>         err = readdir_fill(f, in, arg, dh);
>         if (err)
>
>
>
>-------------------------------------------------------
>SF.Net email is Sponsored by the Better Software Conference & EXPO
>September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
>Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
>Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
>_______________________________________________
>fuse-devel mailing list
>[hidden email]
>https://lists.sourceforge.net/lists/listinfo/fuse-devel
>
>  
>


--
John Muir
NORTEL
[hidden email]



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel