bug#10305: coreutils-8.14, "rm -r" fails with EBADF

[Joachim Schmitz]

> From: Paul Eggert [mailto:address@hidden
> Sent: Wednesday, December 21, 2011 7:42 PM
> To: Eric Blake
> Cc: Jim Meyering; address@hidden; Joachim Schmitz; bug-
> address@hidden
> Subject: Re: bug#10305: coreutils-8.14, "rm -r" fails with EBADF
> 
> On 12/21/11 08:27, Eric Blake wrote:
> > maybe we should wrap opendir() so that the gnulib rpl_opendir() always
> > opens a directory at the same time
> 
> That sounds a bit drastic, but it may be necessary.
> 
> How about this idea instead?  Use the following patch, so that fts_build does 
> not
> assume that dirfd works.
> 
> POSIX does not require dirfd to work, and NonStop is within its rights to not
> support dirfd, and in this particular case I think the code will work without 
> it
> (albeit less reliably in the presence of very large file trees).
> 
> Joachim, does this patch fix "rm" for you?  If not, what problems does it run
> into?  (I have not audited the code for all uses of dirfd, just for this one.)

And this seems to be the problem, dirfd() gets called 3 times:

1st time 
12 dirfd() dirfd.c:28 0x7002dbe2        
11 rpl_opendir() opendir.c:136 0x70065000       
10 opendir_safer() opendir-safer.c:33 0x70054450        
9 fd_clone_opendir() fdopendir.c:159 0x70032650 
8 fdopendir_with_dup() fdopendir.c:119 0x70032000       
7 fdopendir() fdopendir.c:68 0x700319d0 
6 opendirat() fts.c:310 0x70033a30      
5 fts_build() fts.c:1305 0x7003dcf0     
4 fts_read() fts.c:902 0x70039b10       
3 rm() remove.c:598 0x7001ad10  
2 main() rm.c:343 0x70015910    
1 _MAIN() ATOM.$RLSE.T8432H03.CPLMAINC:50 0x70012d40    
2nd time 
11 dirfd() dirfd.c:28 0x7002dbe2        
10 opendir_safer() opendir-safer.c:37 0x700544d0        
9 fd_clone_opendir() fdopendir.c:159 0x70032650 
8 fdopendir_with_dup() fdopendir.c:119 0x70032000       
7 fdopendir() fdopendir.c:68 0x700319d0 
6 opendirat() fts.c:310 0x70033a30      
5 fts_build() fts.c:1305 0x7003dcf0     
4 fts_read() fts.c:902 0x70039b10       
3 rm() remove.c:598 0x7001ad10  
2 main() rm.c:343 0x70015910    
1 _MAIN() ATOM.$RLSE.T8432H03.CPLMAINC:50 0x70012d40    

3rd time:
7 dirfd() dirfd.c:28 0x7002dbe2 
6 rpl_closedir() closedir.c:43 0x7002da90       
5 fts_build() fts.c:1397 0x7003e7d0     
4 fts_read() fts.c:902 0x70039b10       
3 rm() remove.c:598 0x7001ad10  
2 main() rm.c:343 0x70015910    
1 _MAIN() ATOM.$RLSE.T8432H03.CPLMAINC:50 0x70012d40    

The code path you modified is not touched at all.

> diff --git a/lib/fts.c b/lib/fts.c
> index ccd1980..4dc6809 100644
> --- a/lib/fts.c
> +++ b/lib/fts.c
> @@ -1283,7 +1283,7 @@ fts_build (register FTS *sp, int type)
>          FTSENT *cur = sp->fts_cur;
>          bool continue_readdir = !!cur->fts_dirp;
> 
> -        /* When cur->fts_dirp is non-NULL, that means we should
> +        /* If cur->fts_dirp is non-NULL and dirfd is supported,
>             continue calling readdir on that existing DIR* pointer
>             rather than opening a new one.  */
>          if (continue_readdir)
> @@ -1292,16 +1292,20 @@ fts_build (register FTS *sp, int type)
>              dir_fd = dirfd (dp);
>              if (dir_fd < 0)
>                {
> +                int dirfd_errno = errno;
>                  closedir_and_clear (cur->fts_dirp);
>                  if (type == BREAD)
>                    {
>                      cur->fts_info = FTS_DNR;
>                      cur->fts_errno = errno;
>                    }
> -                return NULL;
> +                if (dirfd_errno != ENOTSUP)
> +                  return NULL;
> +                continue_readdir = false;
>                }
>            }
> -        else
> +
> +        if (! continue_readdir)
>            {
>              /* Open the directory for reading.  If this fails, we're done.
>                 If being called from fts_read, set the fts_info field. */