Bug#138195: why not just use symlinks correctly?
I hate to beat a dead horse here, but it seems like the "right" fix to
this problem is to use the symlink as it is intended, which is to say
calling execv on it and letting the kernel resolve it.
I understand the desire to make sure the symlink doesn't point back to
the wrapper, but you can still check that, and then just execv
X_SERVER_SYMLINK. Here's a patch to show it better:
--- xserver-wrapper.c.orig 2003-08-21 04:39:16.000000000 -0400
+++ xserver-wrapper.c 2003-08-21 04:41:16.000000000 -0400
@@ -104,7 +104,6 @@
#define VT_MAJOR_DEV 4
#define X_WRAPPER_CONFIG_FILE "/etc/X11/Xwrapper.config"
-#define X_SERVER_SYMLINK_DIR "/etc/X11"
#define X_SERVER_SYMLINK "/etc/X11/X"
#define X_SOCKET_DIR "/tmp/.X11-unix"
#define X_SOCKET_DIR_MODE (S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO)
@@ -337,16 +336,7 @@
/* DEBUG exit(0); */
- /*
- * change to the directory where the X server symlink is so that a relative
- * symlink will work and execute the X server
- */
- if (chdir(X_SERVER_SYMLINK_DIR)) {
- (void) fprintf(stderr, "X: cannot chdir() to %s (%s), aborting.\n",
- X_SERVER_SYMLINK_DIR, strerror(errno));
- exit(1);
- }
- (void) execv(xserver, argv);
+ (void) execv(X_SERVER_SYMLINK, argv);
(void) fprintf(stderr, "X: exec of %s failed\n", xserver);
exit(1);
Advantages:
- won't break anything that relies on X being started from a particular
directory
- less code
Disadvantages:
- potentially root could have a race condition in which the symlink is
OK, but then between readlink() and execv() he changes it to cause an
infinite loop . This is so ridiculous as to be irrelevant, IMHO.
As an aside, I also noticed a (probably harmless) buffer overflow. The
readlink call needs to leave a byte free for the NUL-terminator, in case
the link really *is* 1024 bytes long. Patch is below.
-Peff
--- xserver-wrapper.c.orig 2003-08-21 04:43:23.000000000 -0400
+++ xserver-wrapper.c 2003-08-21 04:43:37.000000000 -0400
@@ -232,7 +232,7 @@
exit(1);
}
- i = readlink(X_SERVER_SYMLINK, xserver, 1024);
+ i = readlink(X_SERVER_SYMLINK, xserver, sizeof(xserver) - 1);
if (i < 0) {
(void) fprintf(stderr, "X: cannot read %s symbolic link (%s), aborting.\n",
Reply to: