Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.4.0 portability regression because of openat(AT_FDCWD #701

Closed
solardiz opened this issue Jan 15, 2025 · 2 comments · Fixed by #707
Closed

3.4.0 portability regression because of openat(AT_FDCWD #701

solardiz opened this issue Jan 15, 2025 · 2 comments · Fixed by #707

Comments

@solardiz
Copy link

The 3.4.0 release blindly assumes that openat and AT_FDCWD are available when certain other things are, but this is not the case e.g. on ancient glibc. This fixes build on glibc 2.3.6:

+++ rsync-3.4.0/syscall.c	2025-01-15 03:21:50 +0000
@@ -734,7 +734,7 @@ int secure_relative_open(const char *bas
 		return -1;
 	}
 
-#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY)
+#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD)
 	// really old system, all we can do is live with the risks
 	if (!basedir) {
 		return open(relpath, flags, mode);

Just like the O_* checks here, this added check relies on AT_FDCWD being a macro (rather than e.g. part of an enum) on systems where it's present - I don't know whether that is guaranteed or perhaps not.

It's unfortunate that this fallback code keeps some of the security issues exposed - perhaps this is worth warning about somewhere beyond the source code comment? For example, maybe configure should fail on such systems by default, and require an "accept risks" option to override that.

@freakout42
Copy link

#ifdef HAVE_LINKAT has to be used here:

#else
  int dirfd = AT_FDCWD;
  if (basedir != NULL) {
    dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
    if (dirfd == -1) {
      return -1;
    }
  }
  int retfd = -1;

@ncopa
Copy link
Contributor

ncopa commented Jan 15, 2025

#ifdef HAVE_LINKAT has to be used here:

#else
  int dirfd = AT_FDCWD;
  if (basedir != NULL) {
    dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
    if (dirfd == -1) {
      return -1;
    }
  }
  int retfd = -1;

Technically there should be a HAVE_OPENAT. Using AT_FD_CWD works and is better than use HAVE_LINKAT IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants