shimming the c open function on linux and logging its usage

Question

I'm currently trying to implement a c shim which sits between the open function from the c standard library and a program.

the shim should transparently write all file paths being opened to a log within a directory defined by the environment variable IO_SHIM_PREFIX

I have this working relatively well, but in order to achieve it I had to fake the fcntl.h header guards, and I think that there must be a better way.

If I include fcntl.h directly I get the following error

gcc -shared -fPIC -o shim.so src/shim.c -ldl
src/shim.c:47:5: error: conflicting types for ‘open’
   47 | int open(const char *pathname, int flags){
      |     ^~~~
In file included from src/shim.c:16:
/usr/include/fcntl.h:168:12: note: previous declaration of ‘open’ was here
  168 | extern int open (const char *__file, int __oflag, ...) __nonnull ((1));
      |            ^~~~
make: *** [Makefile:3: all] Error 1

I'm guessing it has something to do with the __nonnull ((1)) part at the end but I'm not much of a c programmer and I dont understand it.

Makefile

CC=gcc
all: src/shim.c
    $(CC) -shared -fPIC -o shim.so src/shim.c -ldl

src/shim.c

// required for RTLD_NEXT
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <dlfcn.h>
#include <unistd.h>
#include <math.h>
#include <string.h>

// this bypasses the header guard in bits/fcntl.h
// its a terrible idea, but I don't know the right way to do this.
#define _FCNTL_H
#include <bits/fcntl.h>
#undef _FCNTL_H

typedef int (*open_fn_ptr)(const char*, int);


void write_to_log(const char* prefix, const char* pathname, open_fn_ptr original_open){
    pid_t pid = getpid();
    pid_t tid = gettid();

    char* pattern;

    if (prefix[strlen(prefix) -1] == '/'){
        pattern = "%s%d_%d.log";
    }
    else{
        pattern = "%s/%d_%d.log";
    }
    
    
    int path_len = snprintf(NULL, 0, pattern, prefix, pid, tid);
    char* log_filepath = (char*) malloc(sizeof(char) * path_len);
    sprintf(log_filepath, pattern, prefix, pid, tid);


    int fd = original_open(log_filepath, O_WRONLY | O_APPEND | O_CREAT);
    write(fd, pathname, strlen(pathname));
    write(fd, "\n", sizeof(char));      
    close(fd);

    free(log_filepath);
}

int open(const char *pathname, int flags){
        // acquire a pointer to the original implementation of open.
        open_fn_ptr original_open = dlsym(RTLD_NEXT, "open");

    const char* prefix = getenv("IO_SHIM_PREFIX");
    if (prefix != NULL) {
        write_to_log(prefix, pathname, original_open);
    }
        return original_open(pathname, flags);
}

Usage example

IO_SHIM_PREFIX=`realpath .` LD_PRELOAD=./shim.so brave-browser

Answer 1

    char* log_filepath = (char*) malloc(sizeof(char) * path_len);

malloc() returns a void*, which in C converts to any object-pointer type (unlike in C++, if you're used to that). So the cast is unnecessary (it's slightly harmful, in that it distracts attention from more dangerous casts). Also, because char is the unit of size, sizeof (char) can only be 1, so the multiplication is pointless. That line should be simply

    char* log_filepath = malloc(path_len + 1);

Note the +1 there - the code had a bug because we forgot to allocate space for the null character that ends the string.

There are other uses of sizeof (char) where plain old 1 would be more appropriate and easier to read.

---

Here, we assign a string literal to a char*:

        pattern = "%s%d_%d.log";

That's poor practice, as writes to *pattern are undefined behaviour. The best fix is to declare pattern to point to const char:

 const char *pattern;

I don't think it's necessary to choose between the two patterns - the filesystem interface will ignore consecutive directory separators, so it's safe to always use "%s/%d_%d.log".

---

// this bypasses the header guard in bits/fcntl.h
// its a terrible idea, but I don't know the right way to do this.
#define _FCNTL_H
#include <bits/fcntl.h>
#undef _FCNTL_H

Your intuition here is correct; we are relying on the compiler/platform innards here rather than the public interface. You should define open_fn_ptr to have the same signature as the library does:

int open(const char *pathname, int flags, ...)
{

Then we can simply include the documented, supported POSIX header:

#include <fcntl.h>

---

    // acquire a pointer to the original implementation of open.
    open_fn_ptr original_open = dlsym(RTLD_NEXT, "open");

That assignment is an invalid conversion in C - although void* can be assigned to any object pointer, that's not true for function pointers. It might be worth drawing attention here using an explicit cast, though GCC will still emit a warning - see Casting when using dlsym().