0

I'm learning syscalls like stat and readlink. I try to stat the root directory in two different ways:

  1. Just stat the path to root. Simple.
  2. Then, a roundabout way, fopen "/", make a file descriptor path from the fd I get and readlink it to get "/". Then stat.

What I don't understand is this works the way I expect and the inode numbers are the same, except when I introduce more code AFTERWARDS, namely getcwd, the inodes are strangely not the same and test 2 fails. If you run both versions, with and without the part it says to cut out, the printf of p2 also changes and you can see there is garbage after /. What am I doing wrong here or what's going on here? How is code after those initial statements causing a change earlier in the code?

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <stdio.h>
#include <limits.h>
#include <stdlib.h>
#include <string.h>

int main()
{
    /* Stat root this way */
    struct stat st1;
    char *p1 = "/";
    if (!stat(p1, &st1))
        printf("PASS\n");
    else
        printf("FAIL - %m\n");

    /* Stat root another way */
    struct stat st2;
    char fdpath[100], p2[100];
    FILE *fp = fopen("/", "r");
    sprintf(fdpath, "/proc/self/fd/%d", fileno(fp));
    readlink(fdpath, p2, sizeof(p2));
    if (!stat(p2, &st2))
        printf("PASS\n");
    else
        printf("FAIL - %m\n");

    printf("p2 = %s\n", p2);

    /* Check inodes are the same */
    printf("    st1.st_ino = %ld\n", st1.st_ino);
    printf("    st2.st_ino = %ld\n", st2.st_ino);

    /* TRY WITHOUT THIS - Adding this makes the inodes different! Take it out and they're the same */
    char cwd_buf[100];
    char *cwd_ret = getcwd(cwd_buf, sizeof(cwd_buf));
    if (cwd_ret == NULL)
        printf("getcwd failed - %m\n");
    printf("cwd_ret = %s\n", cwd_ret);

    return 0;
}
Andrew Cina
  • 57
  • 9
  • @LHLaurini, am I note checking for success on 0 with ```!```? – Andrew Cina Jul 21 '20 at 23:51
  • You are correct. Oops. – LHLaurini Jul 21 '20 at 23:53
  • What happens if you `fclose(fp)` in an appropriate place? (Like you should as an upstanding software citizen anyway.) – Code-Apprentice Jul 22 '20 at 00:02
  • Please do not use the `%m` in your calls to `printf()` – user3629249 Jul 22 '20 at 16:46
  • regarding: `if (cwd_ret == NULL) printf("getcwd failed\n"); printf("cwd_ret = %s\n", cwd_ret);` when the call to `getcwd()` fails, do not try to print the results of the call (which is a NULL pointer) – user3629249 Jul 22 '20 at 16:48
  • OT: for ease of readability and understanding by us humans (the compiler doesn't care) 1) please consistently indent the code. Indent after every opening brace (even it you did not actually insert the brace) '{'. Unindent before every closing brace '}'. Suggest each indent level be 4 spaces. 2) Please follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* 3) separate code blocks: `for` `if` `else` `while` `do...while` `switch` `case` `default` via a single blank line – user3629249 Jul 22 '20 at 16:50
  • OT regarding: `FILE *fp = fopen("/", "r");` and similar C library statements. always check the returned value (in this case (!=NULL) to assure the operation was successful. – user3629249 Jul 22 '20 at 16:55
  • OT: for ease of readability and understanding by us humans (the compiler doesn't care) 1) Please insert an appropriate space: inside parens, inside braces, inside brackets, after commas, after semicolons, around C operators – user3629249 Jul 22 '20 at 16:58

2 Answers2

2

From the manpage:

readlink() places the contents of the symbolic link pathname in the buffer buf, which has size bufsiz. readlink() does not append a null byte to buf. It will (silently) truncate the contents (to a length of bufsiz characters), in case the buffer is too small to hold all of the contents.

So you need to do it manually if you want to use p2 as a string:

int size = readlink(fdpath, p2, sizeof(p2) - 1);
if (size < 0)
{
    // error
}
p2[size] = 0;

Notice the -1: that should reserve a byte for a null terminator, even if the string is truncated.

LHLaurini
  • 822
  • 10
  • 24
  • @AndrewCina That's undefined behavior, so it varies. On my computer, the byte following the `'/'` character was `0x03` with the extra code and `0x00` without it. So that's why it worked before (a "fake" null terminator). Since the compiler doesn't initialize local arrays, there could be any data there. – LHLaurini Jul 22 '20 at 00:17
2

readlink does not null-terminate the string it returns, so p2 may contain arbitrary garbage following /.

You should do

ssize_t len = readlink(fdpath, p2, sizeof(p2));
if (len == -1) { /* error */ }
if (len == sizeof(p2)) { /* truncated */ }
p2[len] = '\0';
if (!stat(p2, &st2)) // ...

You should do error checking in many other places, too.

Adding and removing the unrelated code at the end probably changed the stack layout (because of the additional local variables), meaning that p2 contained different garbage with and without it. It could even be that in some cases, the garbage was null bytes, so that the string happened to be properly terminated after all. This sort of thing happens all the time with bugs in C code and isn't particularly meaningful; it's undefined behavior in the language of the C standard. Don't think too hard about why it happens; just try to find and fix the bug and don't write the same bug again :-)

Nate Eldredge
  • 24,174
  • 2
  • 31
  • 43