[Bug libc/30737] New: fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[Bug libc/30737] New: fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[markgaleck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

            Bug ID: 30737
           Summary: fdopendir() is not robust - returns bogus DIR* instead
                    of flagging an error
           Product: glibc
           Version: 2.35
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: markgaleck at gmail dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

First, dislclaimer: I checked this issue is present on the latest Ubuntu
distribution, which has glibc version 2.35, not the latest stable 2.38.  I was
able to build 2.38 locally, but I am not able to force Ubuntu to accept it for
linking to the sample program, or to upgrade the installed glibc 2.35 to 2.38. 

If you know how to test the sample program with 2.38, please let me know, and I
will do so and I will remove this report if it does not reproduce with glibc
2.38.
---------------------------

Here is the SSCCE 
'foobar.c'

(which I obtained by necessary modifications to the sample code in man7 entry
for fdopendir() ) :


#define _GNU_SOURCE

#include <stdio.h>
#include <dirent.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>

int
main(int argc, char* argv[])
{
    DIR* d;
    struct dirent* dp;
    int dfd;

    if ((d = fdopendir(open("./dir", O_RDONLY | O_PATH | O_DIRECTORY))) ==
NULL) {
        fprintf(stderr, "Cannot open directory\n");
        exit(1);
    }

    while ((dp = readdir(d)) != NULL) {
        if (!strcmp(dp->d_name, "."))
            continue;
        if (!strcmp(dp->d_name, ".."))
            continue;
        printf("d_name %s\n", dp->d_name);
    }
    return 0;
}

-------------------------
Here is a script to show the problem:

$ mkdir dir
$ touch dir/file
$ gcc -lc foobar.c
$ ./a.out
$

When called on a descriptor opened with O_PATH , fdopendir() succeeds happily,
but then, readdir() reports nothing!

So the DIR* returned from fdopendir() is useless, but fdopendir() returns it,
instead of NULL and setting errno to EBADF , as it should INHO


If I remove | O_PATH from the above sample code, I get the expected result:

$gcc -lc foobar.c
$ ./a.out
d_name file

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[markgaleck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

--- Comment #1 from Mark Galeck <markgaleck at gmail dot com> ---
Created attachment 15049
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15049&action=edit
foobar.c - SSCCE source

put in the same directory as the sample script foobar.sh

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[markgaleck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

--- Comment #2 from Mark Galeck <markgaleck at gmail dot com> ---
Created attachment 15050
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15050&action=edit
foobar.sh - SSCCE script

run in the same directory as C source foobar.c

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

--- Comment #3 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Mark Galeck from comment #0)
> First, dislclaimer: I checked this issue is present on the latest Ubuntu
> distribution, which has glibc version 2.35, not the latest stable 2.38.  I
> was able to build 2.38 locally, but I am not able to force Ubuntu to accept
> it for linking to the sample program, or to upgrade the installed glibc 2.35
> to 2.38. 
> 
> If you know how to test the sample program with 2.38, please let me know,
> and I will do so and I will remove this report if it does not reproduce with
> glibc 2.38.
> ---------------------------
> 
> Here is the SSCCE 
> 'foobar.c'
> 
> (which I obtained by necessary modifications to the sample code in man7
> entry for fdopendir() ) :
> 
> 
> #define _GNU_SOURCE
> 
> #include <stdio.h>
> #include <dirent.h>
> #include <fcntl.h>
> #include <stdlib.h>
> #include <string.h>
> 
> int
> main(int argc, char* argv[])
> {
>     DIR* d;
>     struct dirent* dp;
>     int dfd;
> 
>     if ((d = fdopendir(open("./dir", O_RDONLY | O_PATH | O_DIRECTORY))) ==
> NULL) {
>         fprintf(stderr, "Cannot open directory\n");
>         exit(1);
>     }
> 
>     while ((dp = readdir(d)) != NULL) {
>         if (!strcmp(dp->d_name, "."))
>             continue;
>         if (!strcmp(dp->d_name, ".."))
>             continue;
>         printf("d_name %s\n", dp->d_name);
>     }
>     return 0;
> }
> 
> -------------------------
> Here is a script to show the problem:
> 
> $ mkdir dir
> $ touch dir/file
> $ gcc -lc foobar.c
> $ ./a.out
> $
> 
> When called on a descriptor opened with O_PATH , fdopendir() succeeds
> happily, but then, readdir() reports nothing!

But readdir *does* report it, if you set errno as 0 prior calling, you will see
that EBADF is correctly set (from getdents64).

> 
> So the DIR* returned from fdopendir() is useless, but fdopendir() returns
> it, instead of NULL and setting errno to EBADF , as it should INHO
> 

Although POSIX defines fdopendir EBADF for file descriptors not open for
reading, O_PATH is a Linux extension, and standard-wise not failing with it is
still conformant. However, I agree this is QoI to fail if the file descriptor
is opened with O_PATH (it fails early and Linux fdopendir already call fcntl,
so it is a free check).

> 
> If I remove | O_PATH from the above sample code, I get the expected result:
> 
> $gcc -lc foobar.c
> $ ./a.out
> d_name file

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[markgaleck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

--- Comment #4 from Mark Galeck <markgaleck at gmail dot com> ---
Right, I did not mean to say readdir() did not report error; I did not check
the error return of readdir().  

Yes so the only question is that of early failure versus later failure.  

Up to you, fine with me if you close it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

--- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Mark Galeck from comment #4)
> Right, I did not mean to say readdir() did not report error; I did not check
> the error return of readdir().  
> 
> Yes so the only question is that of early failure versus later failure.  
> 
> Up to you, fine with me if you close it.

I think it is a QoI to fail early, and it is a free check on fdopendir. I will
send a patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/30737] fdopendir() is not robust - returns bogus DIR* instead of flagging an error

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30737

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           Assignee|unassigned at sourceware dot org   |adhemerval.zanella at linaro dot o
                   |                            |rg
   Target Milestone|---                         |2.39
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #6 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Fixed on 2.39.

-- 
You are receiving this mail because:
You are on the CC list for the bug.