Converting to 'openat' style functions

gnashley · 11-25-2007, 01:50 PM

I maintain a few programs which need to be updated for use with newer versions of glibc. Is there some general reference that will help me to convert programs to use openat-style functions? How does using the new syntax affect compatibility with solaris and bsd systems?

osor · 11-27-2007, 01:13 PM

Quote:

Originally Posted by gnashley

I maintain a few programs which need to be updated for use with newer versions of glibc. Is there some general reference that will help me to convert programs to use openat-style functions?

Of course you don’t have to convert all programs to use the *at() functions. The old-style functions still work just as well (except for the possible race conditions ;). In fact, they are translated (at kernel-level I believe) from something like this:

Code:

fd = open("/foo/bar/baz", 0);

to this:

Code:

dfd = open(".", 0);
fd  = openat(dfd, "/foo/bar/baz", 0);
close(dfd);

The statements in green are not actually executed, since the kernel will keep an open file descriptor to the current working directory for any given task. Of course this is how you would generally want to do it (i.e., using the new-style calls to their fullest potential):

Code:

dfd = open("/foo/bar", 0);
fd  = openat(dfd, "baz", 0);
close(dfd);

Of course it depends on the context in which you are using these functions (e.g., if you happen to have already opendired the parent directory, just use dirfd() instead).

Quote:

Originally Posted by gnashley

How does using the new syntax affect compatibility with solaris and bsd systems?

Well, Solaris already implements them, and the BSDs should not be far behind. In fact, the OpenGroup has already announced that the newest specification of POSIX will include the following new “interfaces” (i.e., functions):

Code:

faccessat
fchmodat
fchownat
fstatat
futimesat
linkat
mkdirat
mknodat
openat
readlinkat
renameat
symlinkat
unlinkat

gnashley · 11-28-2007, 08:50 AM

Thanks very much for responding to the posting -I was afraid nobody was going to provide any help at all...

Since I've gotten your interest, let me explain a little better. First, I am not really a C programmer. But I maintain over 500 programs on my website, so I've become a master of cut-n-paste fixes for many things and have managed a few hacks myself with the little I know about C. I am trying to learn more, but my plate is pretty full. I am the author of a unique software packager called src2pkg whci is written in BASH, but I use the installwatch library to track the file creation.

I'm sure you've heard of installwatch/checkinstall, so you'll know that it is pretty important to lots of folks. Trouble is that the maintainer is extremely inactive -the last code updates to installwatch were made in 2002 and they were from contributors. The last additions added some nifty, but complicated functionality and not all the bugs were ironed out. I have collected quite a few patches for the last version which seem to fix most of the original problems. But, meanwhile glibc-2.5 (with 'at' functions) has come along and seems to be causing some new problems.

As you may know, installwatch works by preloading a small library which wraps several of the main glibc function calls which have to do with file creation, linking and so forth. The latest problems seem to stem from usage with the latest version of coreutils which are apparently now starting to use 'at' functions. So, I would like to find a way to cover these new functions using a modified installwatch (The last word from the author/maintainer was that he was working on this -but that was quite a while back and there is no CVS available to get a leg-up on whatever work he has done.)

The last big code change was from a contributor and it added filesystem translation which works like a virtual chroot or jail and redirects the paths to somewhere besides the root directory. So, the changes with openat are probably going to cause real weirdness with this code. From what you have replied and what I have read in the UNIX/SUN man-pages for openat, it would seem that one can fairly easily wrap any of the at-style functions just as you have shown in your reply.

I won't ask you to do the work for me -though I won't refuse any help you can give as this all still quite murky to me. I've been busy wrapping up some other changes to the code so I haven't settled down to really have a look and see if I can figure out how to do this. But, over the next few days I intend to do so and I'd appreciate any pointers you might give. I might need some help to figure out the basic pattern of how to wrap these functions -maybe a couple of examples for 'openat' itself and 'symlinkat'. Most of the function calls are pretty much repetitive code anyway, so I shouldn't need that much help to understand how to proceed.

I intend to make the modified code available to others who rely on the installwatch functionality, so your help would be greatly appreciated. if you are really interested and want to see the whole code I'll make that available to you. At any rate, I'll get back to you in a few days to see if I am on the right track.
Thanks!

osor · 11-28-2007, 02:30 PM

I see. So you’re trying to intercept calls to *at() and stay sane

. Well for the majority of the cases, you’ll just pass the control over to the real function as intended. These are the two examples of “the majority of cases” which a “client” program might have:

Code:

int dfd = open("/foo/bar", 0);
int fd  = openat(dfd, "baz", 0);
close(dfd);

Code:

DIR *dir = opendir("/foo/bar");
int dfd = dirfd(dir);
int fd  = openat(dfd, "baz", 0);
close(dfd);

In both these cases, the “name translation” occurs in the code in red. By the time you get to openat(), the majority of the name has already been translated. A file descriptor for the directory with respect to which you want to open your file has already been given. You can’t translate file descriptors (at least not the same way you translate paths), but in this case you don’t have to (since dfd is a “real” file descriptor to the “fake” path).

Now I’ll show a few of the gotchas (which might rarely occur in real life). The first is based on an innocent pattern of code:

Code:

int dfd = open("/usr/bin", 0);
…
int fd  = openat(dfd, "../lib/foo.so", O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
…
close(dfd);

So somewhere in the execution of the program, the path /usr/bin was opened. Later on, the program wants to open /usr/lib/foo.so for writing. Instead of giving the full path through open or the path relative to /usr/lib, the program decides to give a path relative to the already-open file /usr/bin. So far, the naïve intercepted function will succeed in this case. On the initial open, the path will be translated from /usr/bin to /my/path/usr/bin. Then, the relative open will result in /my/path/usr/bin/../lib/foo.so being opened (which was the intent). The problem occurs when the “parent directory” is referenced too often in a chain:

Code:

int dfd = open("/usr/bin", 0);
…
int fd  = openat(dfd, "../../../usr/lib/foo.so", O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
…
close(dfd);

As you probably know, by convention the root directory is its own parent. So the above will work as intended when run without your translator, but when run with your translator it will fail miserably. Unfortunately this is the hardest gotcha to detect, since you’ll have to use /proc/$PID/fd/dfd to get original the path of the directory’s file descriptor. Then, strip out any extraneous parent directory references. Unfortunately you are in userspace, and will be defeating much possible race conditions the *at() functions were designed to circumvent.

The second gotcha is when a programmer does something like this:

Code:

int fd  = openat(AT_FDCWD, "/foo/bar/baz", 0);

The above code accomplishes the same thing as

Code:

int fd  = open("/foo/bar/baz", 0);

so there is little reason to use such verbosity. Nevertheless, there are a few situations where the openat() version will actually occur in real life (for example, a switch statement in which the default case is to set the file descriptor to the current working directory).

In any case, AT_FDCWD is not a “real” file descriptor, but more of a psuedo-file descriptor (it’s a macro which holds a unique value which tells the kernel to lookup a path relative to a thread’s current working directory). So you’ll have to make it a “real” file descriptor to the fake directory before passing it to the real openat().

Stay tuned for some code… CNP

gnashley · 11-28-2007, 03:56 PM

Here's an example which may be helpful, showing the definition, the wrapped libchandle, and then the full function for rename -one of the main 'at' functions which is being used in several programs included in coreutils.

Code:

static int (*true_rename)(const char *, const char *);


true_rename      = dlsym(libc_handle, "rename");


int rename(const char *oldpath, const char *newpath) {
	int result;
	instw_t oldinstw;
	instw_t newinstw;

	REFCOUNT;
	
	if (!libc_handle)
		initialize();

#if DEBUG
	debug(2,"rename(\"%s\",\"%s\")\n",oldpath,newpath);	
#endif

	  /* We were asked to work in "real" mode */
	if( !(__instw.gstatus & INSTW_INITIALIZED) ||
	    !(__instw.gstatus & INSTW_OKWRAP) ) {
		result=true_rename(oldpath,newpath);
		return result;
	}

	instw_new(&oldinstw);
	instw_new(&newinstw);
	instw_setpath(&oldinstw,oldpath);
	instw_setpath(&newinstw,newpath);

#if DEBUG
	instw_print(&oldinstw);
	instw_print(&newinstw);
#endif

	backup(oldinstw.truepath);
	instw_apply(&oldinstw);
	instw_apply(&newinstw);

	result=true_rename(oldinstw.translpath,newinstw.translpath);
	logg("%d\trename\t%s\t%s\t#%s\n",result,
	    oldinstw.reslvpath,newinstw.reslvpath,error(result));

	instw_delete(&oldinstw);
	instw_delete(&newinstw);

	return result;
}

Of course:
result=true_rename(oldpath,newpath);
is doing the real renaming in the true path '/', when neither backup nor file translation are being used.

Files which are about to be overwritten are backed up with:
backup(oldinstw.truepath);
when the backup capability is being used.

And this is the real hocus-pocus which translates the real paths into a tranlsation path:
instw_apply(&oldinstw);
instw_apply(&newinstw);

result=true_rename(oldinstw.translpath,newinstw.translpath);

A third path is built first holding meta-info about the files being created and their status -whether they exist in the real path or not, etc.

Most of the wrapper functions follow this same pattern. This one (rename) and symlink are the most complicated since they have to keep track of four or more path descriptions. I'll probably have to show you the code which performs the translation and meta translation also so you get an idea of the most reliable way to implement the changes. I think that the new 'at' functions could probably be used efficiently to replace the current translation code which is rather complicated. However that would require an extensive rewrite and so is probably not the way to go. I've started a list of the 'at' functions which seem to be needed by the current wrapped functions -it looks like there are only 8-10 which are needed. As I said, the functions are mostly constructed after the same pattern as above -the open and fopen (64) ones are a little different and getcwd probably needs some special handling as well. But, hopefully you'll see a way that this can be done simplay without having to re-write everything. Wrapping the wrappers with another level is what this means(??) so that the underlying code mostly ignores the difference?

Thanks for your interest and patience....

osor · 11-28-2007, 07:34 PM

Hello again.

First, let me give you a gist of what I was saying before with some code:

Code:

int new_openat(int dfd, const char *pathname, int flags, ...)
{
	mode_t mode = 0;
	va_list arg;
	if(flags & O_CREAT) {
		va_start(arg, flags);
		mode = va_arg(arg, mode_t);
		va_end (arg);
	}

	if(dfd == AT_FDCWD || *pathname == '/')
		return new_open(pathname, flags, mode);

	if(strstr(pathname, "..")) {
		char *real_pathname;
		int retval;

		if(!(real_pathname = simplify(dfd, pathname)))
			return -1; /* errno will be appropriate */
		if(*real_pathname == '/')
			retval = new_open(real_pathname, flags, mode);
		else
			retval = true_openat(dfd, real_pathname, flags, mode);
		free(real_pathname);
		return retval;
	}

	return true_openat(dfd, pathname, flags, mode);
}

I have called the intercepting openat() function new_openat(). I make references to new_open() (which is the intercepting open() function) and true_openat() (which from your code snippet seems like a function pointer to the “original” openat()). There are a few constructs in your code snippet which don’t make immediate sense without some context. I’ve chosen to ignore the majority of them for now. In any case, the above code probably won’t need all that since it won’t do any path translation itself. If there is any path translation at all, it will occur in a call to the new_open() function or to a currently-unspecified function named simplify() (see later).

The code in blue is not really important to our discussion. It basically makes some assurances about our optional third argument which controls the “mode” of a newly-created file. If you are new to C, it might not make too much sense, but that’s okay since it shouldn’t need to be changed.

If we forget about the other two blocks of code for a moment (and only consider blue and black) we see that at the end of our function is the fallthrough to the original openat(). Such a function would most likely work for over 90% of all client uses of openat(). For the other 10%, we’ll have to do some error checking.

The code in green is for the second of the two gotchas described in my other post. Basically, it will check to see if at least one of two conditions exist:

The client code tried to use the psuedo-fd for its current working directory.
The pathname is absolute (i.e., the first character is a slash).

In each of these cases, the outcome of the call would be the same as if the client had called open() instead of openat(). So, that’s what we do (we make our code use the equivalent of a call to the fake open()).

Now for the code in red. This code should address the first (and rarer) of the two gotchas described in my other post. If you want to get a headstart on implementing the function, I suggest you initially comment out the red section. There are only two possibilities I can think of where the code would be necessary:

The client is braindead.
The client is nefarious.

Hopefully neither will be the case of any test clients you use for initial debugging

Anyhow, let me explain what the red section actually does. This code will go into effect only when we detect a parent directory designator (“..”) as a substring of our path. If there happen to be such substrings in the code, there are two possibilities:

The client used the parent directory specifiers correctly, and the requested path resolves to a sub-path of the translated root.
The client did not use the parent specifiers correctly, in which case the requested path is resolves to an absolute path.

So there are a few ways to deal with this problem. My solution is to have a function declared as:

Code:

char *simplify(int dfd, const char *path);

This function will return a malloc()ed string (which will subsequently need to be free()ed) containing a sanitized version of the original path. If the original path with respect to dfd refers to a location outside the translated root, the returned string will be an absolute path. Otherwise, it will be semantically equivalent to the original path (but not necessarily exactly equivalent—e.g., simplify(dfd, "../foo/bar/../baz") might return a string holding "../foo/baz")). If an error occurs, NULL will be returned and errno will be set appropriately.

I could have made it easier by making simplify always return an absolute path to the intended function (so I would always be able to use new_open()), but that would re-introduce any race conditions that were being avoided in the first place.

Anyhow, I haven’t yet thought concretely about the implementation of simplify() (no doubt it requires knowledge of the implementation of your translating functions). I’ll leave that for yet another post…

gnashley · 11-29-2007, 11:44 AM

Osor, I've uploaded the complete sources I am working with to here:
http://distro.ibiblio.org/pub/linux/...watch-0.6.6.3/

The tarball installwatch-0.6.3.3.tar.bz2 is an archive of all the other files in there. The sources should compile fine and they include the installwatch wrapper script. Or you can just download the single C file to see the whole code. It's around 3,000 lines of code. The original installwatch lib was only around 1,000 lines, but a single patch which added the filesystem translation doubled that and I've applied other patches which add features for wrapping additional libc functions. They are pretty much all the same. The main code which is problematic is in the first half of the file.

I'm pretty sick the last couple of days so I'm not in much of a mood to venture any guesses at how to handle this. But, I've made a list of the 'at' style functions which seem to be used in the latest coreutils:
openat
linkat
unlinkat
fstatat
chownat
fchownat
lchownat
chmodat
fchmodat
futimesat
mkdirat

There are a couple of others, but they appear to be sub-functions of these main calls.

My initial impression is that the wrappers could be something like this:

Code:

int openat(int dfd, const char *pathname, int flags, ...)
{

	if(dfd == AT_FDCWD || *pathname == '/')
		return open(pathname, flags, mode);

	if(strstr(pathname, "..")) {
		char *real_pathname;
		int retval;

		if(!(real_pathname = simplify(dfd, pathname)))
			return -1; /* errno will be appropriate */
		if(*real_pathname == '/')
			retval = open(real_pathname, flags, mode);
		else
			retval = true_openat(dfd, real_pathname, flags, mode);
		free(real_pathname);
		return retval;
	}

	return true_openat(dfd, pathname, flags, mode);
}

I think that race conditions may not be much of a problem, since these functions seem to effectively block further execution until the true_*function is called. Apparently the meta-translation code is doing something quite similar to what the 'at' functions and probably using 'at' functions instead might be more elegant and perhaps more dependable. However, I'm concerned about keeping (or making) the code usable with libc versions which do not have the 'at' functions at all (uClibc and earlier glibc versions). So the more simply this could be implemented the better and leave the existing translation code to do the work. This should also make it easier for me to add wrappers for more functions later -though the current code already contains nearly everything needed in order to effectively 'trap' all function calls which would create, delete or alter existing files.

Actually, my first idea of how this might work is more like what's below.
Could something as simple as this accomplish what I want? That is I want it to completely bypass the 'at' function and leave all the work to the normal function:

Code:

char *simplify(int dfd, const char *path);

int openat(int dfd, const char *pathname, int flags, ...)
{

if(strstr(pathname, "..")) {
		char *real_pathname;
		int retval;

		if(!(real_pathname = simplify(dfd, pathname)))
			return -1; /* errno will be appropriate */
		if(*real_pathname == '/')
			retval = open(real_pathname, flags, mode);
		else
			retval = -1

		free(real_pathname);
		return retval;
	}
}

osor · 11-29-2007, 02:16 PM

Quote:

Originally Posted by gnashley

Osor, I've uploaded the complete sources I am working with to here:
http://distro.ibiblio.org/pub/linux/...watch-0.6.6.3/

Will look at it.

osor · 11-30-2007, 02:55 PM

Quote:

Originally Posted by osor

Will look at it.

Hi gilbert. I took a look and found out why the fallthrough implementation would not work—path translation occurs selectively. When a file is opened for reading only, the descriptor returned is for the real file (not the fake file as I had thought before). It is only when a file is opened for writing that that path is translated. Since directories can be opened only in read-only mode, it makes sense that most of what I said before would not apply (since any dfd is untranslated).

There are now two solutions I can see: the easy one and the hard one. The easy one will use /proc/self/fd/dfd to get the full path of the intended file and the “normal” (i.e., non-at) function should be called always. The hard solution will somehow try to preserve using the at function. The problem with the easy solution is that it effectively breaks any advantage the client would have gained by using the at-style function (except pathname shortening). The problem with the hard solution is that it seems impossible to implement except in the case where you are doing only read functions (in which case translation is unnecessary).

I will look into implementing this (most likely the easy solution) if I get some time over the weekend.

gnashley · 12-01-2007, 01:54 AM

So meanwhile, yesterday I did a lot of sytudying, looking at the unistd.h file for glibc-2.5 and source and header files for coreutils-6.9+, along with the openat man-page and your code. I seem to have gotten openat working "sort of" -that is I think some of it is redundant. Finally last night I was working on writing a test-case to include in the test-installwatch program. Unfortunely I found that some feature or fix that I had added from other peoples patches had already broken the test-installwatch program. So I backed up to a less-patched version to get the test program runnign again. I think I've nearly gotten it going, but I'm sure you'll see what's wrong with it.
I'll post the fragments that I've added below, but first let me explain a little and re-iterate. The easy case is what seems to be the best bet, at least for now. The reason is that I'd like to keep open the possibility of using the library on system which do *not* support the 'at' fucntions. Also, this will allow the existing code to do its' job 'as is'. Let me explain what it is supposed to do.

installwatch intercepts most calls which have to do with file creation, removal or modification.
1. If the 'backup' feature is being used, it creates a copy of the original file under the 'BACKUP' path before overwriting it by passing the glibc function along with the original path as given.
2. When the filesystem translation feature is being used, installwatch adds the 'TRANLS' path to the front of the original path before passing the call onto the real glibc function.
3. If neither of the above is being used the original path given is passed to the real function. Also, when the backup feature is being used if the new file would not replace an existing file, the path is not changed.

So, the existing code seems to do a pretty god job -I do see that the new 'at' functions could do this more elegantly, but still the translation would have to be done to 'adjust' the paths before passing the call on. There isn't really much concern about the efficiency of the code. Rather the accuracy, dependability and scope is more important -for instance being able to catch calls to sed which alter files in-place, or creation of files using echo or touch -these are areas that may not be fully covered yet -also chmod, chown and mknod.

Again, all I really want it to do is intercept the calls to the 'at' functions, convert the path to an absolute path if necessary and pass the instruction to normal installwatch function -not straight to the true_* function. The installwatch function hopefully takes care of the translation properly and, critically, logs the information. The information in the log is parsed to extend the functionality of the library with external programs -like with checkinstall or my program src2pkg. There should be no true_*at function at all by doing this??

BTW, my program was just reviewed on linux.com. Perhaps you'd like to read the review:
http://www.linux.com/feature/121499

I see you use LFS, so you could even give src2pkg a whirl with no trouble...

Now for the snippets, this part is in installwatc.c:

Code:

int openat(int dfd, const char *pathname, int flags, ...);
char *simplify(int dfd, const char *path);

int openat(int dfd, const char *pathname, int flags, ...)
{
	mode_t mode = 0;
	va_list arg;
	int result;
	
        if(flags & O_CREAT) {
                va_start(arg, flags);
                mode = va_arg(arg, mode_t);
                va_end (arg);
        }
	
        if(dfd == AT_FDCWD || *pathname == '/')
                return open(pathname, flags, mode);

	if(strstr(pathname, "..")) {
		
		char *real_pathname;
                int retval;
		
                if(!(real_pathname = simplify(dfd, pathname)))
                        return -1; /* errno will be appropriate */
                if(*real_pathname == '/')
                        retval = open(real_pathname, flags, mode);
                
#if DEBUG
		debug(2,"openat(%s,%d,mode)\n",pathname,flags);
		log("%d\topenat\t%s\t#%s\n",result,pathname,error(result));
#endif

		
		free(real_pathname);
                return retval;
        }
	
	return open(pathname, flags, mode);

}

I'm pretty sure that most of that is not need at all, and I don't think I've handled the drop-through and error case exactly right, but I think it illustrates what I'm trying to do.

Then this is in test-installwatch:

Code:

void test_openat(void) {
	int fd;
	int dfd;

	dfd = open(".", 0);
	fd = openat(dfd, TESTFILE, O_CREAT, O_RDWR, 0700);
	close(dfd);
	unlink(TESTFILE);
}

# this is in test-installwatch main:
do_test("openat", test_openat, 3);

I don't think this is exactly right either, but I at least got it showing up in the debug log to look further at what is going on.

Cheers! Don't let my problems foul up your weekend!

osor · 12-01-2007, 02:21 PM

Hello again.

Quote:

Originally Posted by gnashley

Unfortunely I found that some feature or fix that I had added from other peoples patches had already broken the test-installwatch program. So I backed up to a less-patched version to get the test program runnign again.

Yes I found this as well, but instead of going backwards, I patched up the Makefile to make it work.

Quote:

Originally Posted by gnashley

I think I've nearly gotten it going, but I'm sure you'll see what's wrong with it.
I'll post the fragments that I've added below, but first let me explain a little and re-iterate. The easy case is what seems to be the best bet, at least for now. The reason is that I'd like to keep open the possibility of using the library on system which do *not* support the 'at' fucntions. Also, this will allow the existing code to do its' job 'as is'.

This can be accomplished with conditional compilation (if a suitable glibc, is found, you can include code for the relevant functions at compile-time).

Quote:

Originally Posted by gnashley

Let me explain what it is supposed to do.

installwatch intercepts most calls which have to do with file creation, removal or modification.
1. If the 'backup' feature is being used, it creates a copy of the original file under the 'BACKUP' path before overwriting it by passing the glibc function along with the original path as given.
2. When the filesystem translation feature is being used, installwatch adds the 'TRANLS' path to the front of the original path before passing the call onto the real glibc function.
3. If neither of the above is being used the original path given is passed to the real function. Also, when the backup feature is being used if the new file would not replace an existing file, the path is not changed.

Yes, but additionally, if a file is opened for reading, and even if the translation feature is being used, path translation does not occur. That was the point I was trying to make from the last post (since directories can never be opened for “writing”).

Quote:

Originally Posted by gnashley

Again, all I really want it to do is intercept the calls to the 'at' functions, convert the path to an absolute path if necessary and pass the instruction to normal installwatch function -not straight to the true_* function. The installwatch function hopefully takes care of the translation properly and, critically, logs the information. The information in the log is parsed to extend the functionality of the library with external programs -like with checkinstall or my program src2pkg. There should be no true_*at function at all by doing this??

I think it might be more consistent if the true_*at functions were there only when a suitable glibc existed. That way, you can still use installwatch with an older glibc, yet you don’t employ much unnecessary trickery when using it with newer a glibc.

Quote:

Originally Posted by gnashley

BTW, my program was just reviewed on linux.com. Perhaps you'd like to read the review:
http://www.linux.com/feature/121499

I see you use LFS, so you could even give src2pkg a whirl with no trouble...

I actually read that yesterday. As for LFS, I don’t really use a package manager, per se, but I keep track of file permissions and “ownership” by employing a package user scheme (i.e., UIDs and GIDs whose sole purpose is to tell me what package or sub-package “owns” a particular file).

Quote:

Originally Posted by gnashley

Now for the snippets, this part is in installwatc.c:

Code:

int openat(int dfd, const char *pathname, int flags, ...);
char *simplify(int dfd, const char *path);

int openat(int dfd, const char *pathname, int flags, ...)
{
	mode_t mode = 0;
	va_list arg;
	int result;
	
        if(flags & O_CREAT) {
                va_start(arg, flags);
                mode = va_arg(arg, mode_t);
                va_end (arg);
        }
	
        if(dfd == AT_FDCWD || *pathname == '/')
                return open(pathname, flags, mode);

	if(strstr(pathname, "..")) {
		
		char *real_pathname;
                int retval;
		
                if(!(real_pathname = simplify(dfd, pathname)))
                        return -1; /* errno will be appropriate */
                if(*real_pathname == '/')
                        retval = open(real_pathname, flags, mode);
                
#if DEBUG
		debug(2,"openat(%s,%d,mode)\n",pathname,flags);
		log("%d\topenat\t%s\t#%s\n",result,pathname,error(result));
#endif

		
		free(real_pathname);
                return retval;
        }
	
	return open(pathname, flags, mode);

}

I'm pretty sure that most of that is not need at all, and I don't think I've handled the drop-through and error case exactly right, but I think it illustrates what I'm trying to do.

The problem is that the old method (with the fallthrough) won’t work. Having a fallthrough with just open(pathname, flags, mode) won’t work for the majority of cases where pathname is relative to a directory different from the cwd, but pathname contains no “..” characters.

Quote:

Originally Posted by gnashley

Then this is in test-installwatch:

Code:

void test_openat(void) {
	int fd;
	int dfd;

	dfd = open(".", 0);
	fd = openat(dfd, TESTFILE, O_CREAT, O_RDWR, 0700);
	close(dfd);
	unlink(TESTFILE);
}

# this is in test-installwatch main:
do_test("openat", test_openat, 3);

I don't think this is exactly right either, but I at least got it showing up in the debug log to look further at what is going on.

The problem with the way test-installwatch is setup is that it doesn’t test many of the possible scenarios. It seems (at least presently) to consist of sanity checks ensuring that the functions are indeed being intercepted. This is a very good thing to test, but it doesn’t tell us if our functions work the way we want them to.

Quote:

Originally Posted by gnashley

Cheers! Don't let my problems foul up your weekend!

I need some sort of distraction from my own work. That’s why I joined LQ in the first place.

osor · 12-01-2007, 02:29 PM

Okay, I’ve got a preliminary working openat. I guess since we’re having this discussion here, I’ll post the patches here. Instead of giving you one big patch, I’ve split it up into a few logical patches. All these are against the original from http://distro.ibiblio.org/pub/linux/....6.6.3.tar.bz2.
In particular, I did not make use of any patch reversions to which you alluded in your previous post (mostly because I didn’t have them).

Anyway, here are the patches:

01.patch

Code:

    Provide minor versions for glibc 2.6 and 2.7

--- a/create-localdecls
+++ b/create-localdecls
@@ -74,6 +74,12 @@ if test "$VERSION" = 'libc.so.6' ; then
 		5)
 			echo '#define GLIBC_MINOR 5' >> $OUTFILE
 			SUBVERSION='glibc-2.5' ;;
+		6)
+			echo '#define GLIBC_MINOR 6' >> $OUTFILE
+			SUBVERSION='glibc-2.6' ;;
+		7)
+			echo '#define GLIBC_MINOR 7' >> $OUTFILE
+			SUBVERSION='glibc-2.7' ;;
 		*)
 			echo 'Treated as glibc >= 2.1 (finger crossed)'
 			echo '#define GLIBC_MINOR 1' >> $OUTFILE

02.patch

Code:

    Must’ve been a typo

--- a/installwatch
+++ b/installwatch
@@ -3,7 +3,7 @@
 
 #set -x
 
-PREFIX={PREFIX:-#PREFIX#}
+PREFIX=${PREFIX:-#PREFIX#}
 
 HACK="PREFIX"
 HACK="#$HACK#";

03.patch

Code:

    test-installwatch should not be compiled as a shared lib and should be linked
    to libc (at least that’s what it takes to make it work on my machine)

--- a/Makefile
+++ b/Makefile
@@ -51,7 +51,7 @@ tarball: clean
 	tar -czvC .. -f ../installwatch-$(VERSION).tar.gz installwatch-$(VERSION)
 
 test: install
-	$(CC) $(ALL_CFLAGS) $(LDFLAGS) -o test-installwatch test-installwatch.c $(LIBS)
+	$(CC) $(ALL_CFLAGS) -o test-installwatch test-installwatch.c $(LIBS)
 	PREFIX=$(DESTDIR)$(PREFIX) $(DESTDIR)$(PREFIX)/bin/installwatch ./test-installwatch

04.patch

Code:

    Adds an implemenation of openat(). In the process, a new function was
    introduced: instw_setpathrel(). The new function tries to keep the spirit of
    instw_setpath(), except it is for paths relative to a directory file descriptor.
    As such, it is pretty much a drop-in replacement for instw_setpath() when
    making wrappers for any of the *at() functions. The *at() function declarations
    and definitions should be armored by conditional preprocessing to check if the
    GLIBC_MINOR version is greater than 4 (since those functions only exist for
    >=glibc-2.4).
    
    Overall, the goal was (for good or ill) to keep the spirit of the other wrapped
    functions.

--- a/installwatch.c
+++ b/installwatch.c
@@ -113,6 +113,12 @@ static int (*true_truncate64)(const char *, __off64_t);
 
 #endif
 
+#if (GLIBC_MINOR >= 4)
+
+static int (*true_openat)(int, const char *, int, ...);
+
+#endif
+
 #if defined __GNUC__ && __GNUC__>=2
 	#define inline inline
 #else
@@ -240,6 +246,7 @@ static int instw_delete(instw_t *);
 static int instw_setmetatransl(instw_t *);
 
 static int instw_setpath(instw_t *,const char *);
+static int instw_setpathrel(instw_t *, int, const char *);
 static int instw_getstatus(instw_t *,int *);
 static int instw_apply(instw_t *);
 static int instw_filldirls(instw_t *);
@@ -338,6 +345,12 @@ static void initialize(void) {
 	true_execve      = dlsym(libc_handle, "execve");
 #endif
 
+#if (GLIBC_MINOR >= 4)
+
+	true_openat      = dlsym(libc_handle, "openat");
+
+#endif
+
 	if(instw_init()) exit(-1);
 }
 
@@ -1504,6 +1517,57 @@ static int instw_setpath(instw_t *instw,const char *path) {
 }
 
 /*
+ * procedure = / rc:=instw_setpathrel(instw,dirfd,relpath) /
+ *
+ * task      = /   sets the 'instw->path' field and updates all the fields that
+ *               can be deduced from 'path', such as 'instw->translpath'. Much
+ *               like instw_setpath, except for paths relative to a dirfd. /
+ *
+ * inputs    = / dirfd              An open file descriptor to a directory
+ *               relpath            The given path relative to dirfd, as is
+ * outputs   = / instw->path        The full absolute (non-relative) path
+ *               instw->truepath    The given path, canonicalized
+ *               instw->translpath  The real translated path 
+ *               instw->mtranslpath The translation status path  /
+ *
+ * returns   = /  0 ok. path set
+ *               -1 failed. cf errno /
+ */
+static int instw_setpathrel(instw_t *instw, int dirfd, const char *relpath) {
+
+/* This constant should be large enough to make a string that holds
+ * /proc/self/fd/xxxxx  if you have an open fd with more than five digits,
+ * something is seriously messed up.
+ */
+#define PROC_PATH_LEN 20
+	
+	debug(2,"instw_setpathrel(%p,%d,%s)\n",instw,dirfd,relpath);
+	int retval = -1, l;
+	char *newpath;
+	char proc_path[PROC_PATH_LEN];
+	struct stat s;
+
+	snprintf(proc_path, PROC_PATH_LEN, "/proc/self/fd/%d", dirfd);
+	if(stat(proc_path, &s) == -1)
+		goto out;
+	if(!(newpath = malloc(s.st_size+strlen(relpath)+2)))
+		goto out;
+	if((l = readlink(proc_path, newpath, s.st_size)) == -1)
+		goto free_out;
+	newpath[l] = '/';
+	strcpy(newpath + l + 1, relpath);
+	
+	retval = instw_setpath(instw, newpath);
+
+free_out:
+	free(newpath);
+out:
+	return retval;
+
+#undef PROC_PATH_LEN
+}
+
+/*
  * procedure = / rc:=instw_getstatus(instw,status) /
  *
  * outputs   = / status  instw->path flags field status in the translated fs
@@ -3650,3 +3714,62 @@ int truncate64(const char *path, __off64_t length) {
 
 #endif /* GLIBC_MINOR >= 1 */
 
+#if (GLIBC_MINOR >= 4)
+
+int openat (int dirfd, const char *path, int flags, ...) {
+	mode_t mode = 0;
+	va_list arg;
+	if(flags & O_CREAT) {
+		va_start(arg, flags);
+		mode = va_arg(arg, mode_t);
+		va_end (arg);
+	}
+	int result, status;
+	instw_t instw;
+
+	/* If all we are doing is normal open, forgo refcounting, etc. */
+        if(dirfd == AT_FDCWD || *path == '/')
+                return open(path, flags, mode);
+
+	REFCOUNT;
+
+	if (!libc_handle)
+		initialize();
+
+	debug(2, "openat(%d, %s, 0x%x, 0%o)\n", dirfd, path, flags, mode);
+
+	  /* We were asked to work in "real" mode */
+	if(!(__instw.gstatus & INSTW_INITIALIZED) ||
+	   !(__instw.gstatus & INSTW_OKWRAP))
+		return true_openat(dirfd,path,flags,mode);
+
+	instw_new(&instw);
+	instw_setpathrel(&instw,dirfd,path);
+	
+#if DEBUG
+	instw_print(&instw);
+#endif
+
+	if(flags & (O_WRONLY | O_RDWR)) {
+		backup(instw.truepath);
+		instw_apply(&instw);
+	}
+
+	instw_getstatus(&instw,&status);
+
+	if(status&INSTW_TRANSLATED) 
+		result=true_open(instw.translpath,flags,mode);
+	else
+		result=true_open(instw.path,flags,mode);
+	
+	if(flags & (O_WRONLY | O_RDWR)) 
+		logg("%d\topenat\t%d\t%s\t0x%x\t0%o\t#%s\n", result, dirfd,
+		    path, flags, mode, error(result));
+
+	instw_delete(&instw);
+
+	return result;
+}
+
+#endif /* GLIBC_MINOR >= 4 */
+

The most useful function here is instw_setpathrel(). The intent is for this function to be used almost as a drop-in replacement for instw_setpath() when writing the intercepting *at() functions.

I’ll try to get a good test program soon (not an incremental patch against test-installwatch.c, but a whole different program).

osor · 12-01-2007, 02:33 PM

As an aside, it seems that the codebase for installwatch is (in general) very fragile and hackish. I think it needs a great amount of refactoring before I’d trust it for production use.

osor · 12-02-2007, 01:01 PM

Hello again gilbert. Here is a small test program (which checks for seven separate cases):

Code:

#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <stdio.h>
#include <unistd.h>

#define OLD_DIR "/usr/bin"
#define NEW_DIR "/usr/foo"
#define READ_FILE "gcc"
#define WRITE_FILE "bar"
#define REL_READ_FILE "../lib/libssl.so"
#define REL_WRITE_FILE "../lib/baz.so"

int old_dir_read_file(void)
{
	printf("Checking if you can read a file in an old dir...\n");

	int dfd, fd, retval = 0;
	static char buf[5];
	if((dfd = open(OLD_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, READ_FILE, O_RDONLY)) == -1)
		goto close_out;
	if(read(fd, buf, sizeof(buf) - 1) != sizeof(buf) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tthe next three letters should be ELF: %s.\n", buf+1);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int old_dir_write_file(void)
{
	printf("Checking if you can write a file in an old dir...\n");

	int dfd, fd, retval = 0;
	static char message[] = "Look at this message!";
	if((dfd = open(OLD_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, WRITE_FILE, O_WRONLY | O_CREAT, 0664)) == -1)
		goto close_out;
	if(write(fd, message, sizeof(message) - 1) != sizeof(message) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tyou should now have the message \"%s\" in %s/%s.\n", message, OLD_DIR, WRITE_FILE);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int old_dir_rel_read_file(void)
{
	printf("Checking if you can read a relative file in an old dir...\n");

	int dfd, fd, retval = 0;
	static char buf[5];
	if((dfd = open(OLD_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, REL_READ_FILE, O_RDONLY)) == -1)
		goto close_out;
	if(read(fd, buf, sizeof(buf) - 1) != sizeof(buf) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tthe next three letters should be ELF: %s.\n", buf+1);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int old_dir_rel_write_file(void)
{
	printf("Checking if you can write a relative file in an old dir...\n");

	int dfd, fd, retval = 0;
	static char message[] = "Look at this message!";
	if((dfd = open(OLD_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, REL_WRITE_FILE, O_WRONLY | O_CREAT, 0664)) == -1)
		goto close_out;
	if(write(fd, message, sizeof(message) - 1) != sizeof(message) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tyou should now have the message \"%s\" in %s/%s.\n", message, OLD_DIR, REL_WRITE_FILE);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int new_dir_write_file(void)
{
	printf("Checking if you can write a file in an new dir...\n");

	int dfd, fd, retval = 0;
	static char message[] = "Look at this message!";
	if((dfd = open(NEW_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, WRITE_FILE, O_WRONLY | O_CREAT, 0664)) == -1)
		goto close_out;
	if(write(fd, message, sizeof(message) - 1) != sizeof(message) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tyou should now have the message \"%s\" in %s/%s.\n", message, NEW_DIR, WRITE_FILE);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int new_dir_rel_read_file(void)
{
	printf("Checking if you can read a relative file in an new dir...\n");

	int dfd, fd, retval = 0;
	static char buf[5];
	if((dfd = open(NEW_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, REL_READ_FILE, O_RDONLY)) == -1)
		goto close_out;
	if(read(fd, buf, sizeof(buf) - 1) != sizeof(buf) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tthe next three letters should be ELF: %s.\n", buf+1);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int new_dir_rel_write_file(void)
{
	printf("Checking if you can write a relative file in an new dir...\n");

	int dfd, fd, retval = 0;
	static char message[] = "Look at this message!";
	if((dfd = open(NEW_DIR, O_RDONLY)) == -1)
		goto out;
	if((fd = openat(dfd, REL_WRITE_FILE, O_WRONLY | O_CREAT, 0664)) == -1)
		goto close_out;
	if(write(fd, message, sizeof(message) - 1) != sizeof(message) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tyou should now have the message \"%s\" in %s/%s.\n", message, NEW_DIR, REL_WRITE_FILE);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int (*tests[])(void) = {
	old_dir_read_file,
	old_dir_write_file,
	old_dir_rel_read_file,
	old_dir_rel_write_file,
	new_dir_write_file,
	new_dir_rel_read_file,
	new_dir_rel_write_file
};


int main(int argc, char *argv[])
{
	int i, count = 0, total = sizeof(tests) / sizeof(void(*)(void));

	printf("Tests for openat functionality.\n\n");
	printf("Before you run this, make sure that %s, %1$s/%s, and %1$s/%s exist.\n",
			OLD_DIR, READ_FILE, REL_READ_FILE);
	printf("Also make sure that %s, %4$s/%s, and %4$s/%s do *not* exist.\n\n",
			NEW_DIR, WRITE_FILE, REL_WRITE_FILE, OLD_DIR);
	printf("Attempting to create " NEW_DIR "...\n");
	if(mkdir(NEW_DIR, 0755) == -1) {
		perror(NEW_DIR " could not be created");
		fprintf(stderr, "Some tests will fail.\n");
	}
	else
		printf("\t" NEW_DIR " successfully created.\n");

	printf("\nStarting testing...\n");
	for(i = 0; i < total; i++)
		count += tests[i]();
	printf("%d successes out of %d total.\n", count, total);

	return 0;
}

Here’s how this program tests (using the patches to installwatch from my last post):

Code:

$ sudo rm -rf /usr/foo /usr/bin/bar /usr/bin/../lib/baz.so
$ ./test-openat
Tests for openat functionality.

Before you run this, make sure that /usr/bin, /usr/bin/gcc, and /usr/bin/../lib/libssl.so exist.
Also make sure that /usr/foo, /usr/bin/bar, and /usr/bin/../lib/baz.so do *not* exist.

Attempting to create /usr/foo...
/usr/foo could not be created: Permission denied
Some tests will fail.

Starting testing...
Checking if you can read a file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a file in an old dir...
old_dir_write_file: Permission denied
Checking if you can read a relative file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an old dir...
old_dir_rel_write_file: Permission denied
Checking if you can write a file in an new dir...
new_dir_write_file: No such file or directory
Checking if you can read a relative file in an new dir...
new_dir_rel_read_file: No such file or directory
Checking if you can write a relative file in an new dir...
new_dir_rel_write_file: No such file or directory
2 successes out of 7 total.
$ sudo rm -rf /usr/foo /usr/bin/bar /usr/bin/../lib/baz.so
$ sudo ./test-openat
Tests for openat functionality.

Before you run this, make sure that /usr/bin, /usr/bin/gcc, and /usr/bin/../lib/libssl.so exist.
Also make sure that /usr/foo, /usr/bin/bar, and /usr/bin/../lib/baz.so do *not* exist.

Attempting to create /usr/foo...
/usr/foo successfully created.

Starting testing...
Checking if you can read a file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/bar.
Checking if you can read a relative file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/../lib/baz.so.
Checking if you can write a file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/bar.
Checking if you can read a relative file in an new dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/../lib/baz.so.
7 successes out of 7 total.
$ sudo rm -rf /usr/foo /usr/bin/bar /usr/bin/../lib/baz.so
$ sudo installwatch ./test-openat

INFO : Using a default root directory : /tmp/BMVfJHNFeULUrjmJkSPq

Tests for openat functionality.

Before you run this, make sure that /usr/bin, /usr/bin/gcc, and /usr/bin/../lib/libssl.so exist.
Also make sure that /usr/foo, /usr/bin/bar, and /usr/bin/../lib/baz.so do *not* exist.

Attempting to create /usr/foo...
/usr/foo successfully created.

Starting testing...
Checking if you can read a file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/bar.
Checking if you can read a relative file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/../lib/baz.so.
Checking if you can write a file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/bar.
Checking if you can read a relative file in an new dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/../lib/baz.so.
7 successes out of 7 total.
$ sudo rm -rf /usr/foo /usr/bin/bar /usr/bin/../lib/baz.so
$ installwatch -t ./test-openat

INFO : Using a default root directory : /tmp/FKdrWeJZChbYSeklJhNT

Tests for openat functionality.

Before you run this, make sure that /usr/bin, /usr/bin/gcc, and /usr/bin/../lib/libssl.so exist.
Also make sure that /usr/foo, /usr/bin/bar, and /usr/bin/../lib/baz.so do *not* exist.

Attempting to create /usr/foo...
/usr/foo successfully created.

Starting testing...
Checking if you can read a file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/bar.
Checking if you can read a relative file in an old dir...
the next three letters should be ELF: ELF.
Checking if you can write a relative file in an old dir...
you should now have the message "Look at this message!" in /usr/bin/../lib/baz.so.
Checking if you can write a file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/bar.
Checking if you can read a relative file in an new dir...
new_dir_rel_read_file: No such file or directory
Checking if you can write a relative file in an new dir...
you should now have the message "Look at this message!" in /usr/foo/../lib/baz.so.
6 successes out of 7 total.

As you can see, there seems to be a bug in installwatch in translation mode. In particular, the bug has to do with failure in reverse translation of a created-directory when it is used as the starting point for relative-path translation. Curiously enough, when used in write mode, the bug does not appear (since COW seems to kick in).

osor · 12-02-2007, 01:22 PM

It turns out the bug mentioned in my previous post is older than any changes introduced by my patching. For example, take this program:

Code:

#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <stdio.h>
#include <unistd.h>

#define NEW_DIR "/usr/foo"
#define REL_READ_FILE "../lib/libssl.so"

int new_dir_rel_read_file(void)
{
	printf("Checking if you can read a relative file in an new dir...\n");

	int dfd, fd, retval = 0;
	static char buf[5];
	if((dfd = open(NEW_DIR, O_RDONLY)) == -1)
		goto out;
	if(fchdir(dfd) == -1)
		goto close_out;
	if((fd = open(REL_READ_FILE, O_RDONLY)) == -1)
		goto close_out;
	if(read(fd, buf, sizeof(buf) - 1) != sizeof(buf) - 1)
		goto close_close_out;
	retval = 1;
	printf("\tthe next three letters should be ELF: %s.\n", buf+1);

close_close_out:
	close(fd);
close_out:
	close(dfd);
out:
	if(!retval)
		perror(__func__);
	return retval;
}

int main(int argc, char *argv[])
{
	printf("Attempting to create " NEW_DIR "...\n");
	if(mkdir(NEW_DIR, 0755) == -1) {
		perror(NEW_DIR " could not be created");
		return -1;
	}
	else
		printf("\t" NEW_DIR " successfully created.\n");

	new_dir_rel_read_file();

	return 0;
}

One thing you might notice is that openat() is never called! So any changes I introduced should not affect the program. Now, for the tests:

Code:

$ sudo rm -rf /usr/foo
$ sudo ./test-relpath
Attempting to create /usr/foo...
        /usr/foo successfully created.
Checking if you can read a relative file in an new dir...
        the next three letters should be ELF: ELF.
$ sudo rm -rf /usr/foo
$ installwatch -t ./test-relpath

INFO : Using a default root directory : /tmp/DcYXAhnDfJeXhEEDOHSl

Attempting to create /usr/foo...
        /usr/foo successfully created.
Checking if you can read a relative file in an new dir...
new_dir_rel_read_file: No such file or directory