Skip to content

gh-145335: Fix crash when passing -1 as fd in os.pathconf#145390

Open
aisk wants to merge 4 commits intopython:mainfrom
aisk:pathconf-with-negative-one
Open

gh-145335: Fix crash when passing -1 as fd in os.pathconf#145390
aisk wants to merge 4 commits intopython:mainfrom
aisk:pathconf-with-negative-one

Conversation

@aisk
Copy link
Member

@aisk aisk commented Mar 1, 2026

We currently test whether path_t.fd is -1 to decide if the argument should be treated as an fd or as a path, and then call fpathconf() or pathconf() according to it.

In the original issue, we passing -1 caused the code incorrectly treats the argument as a real path. But, path->narrow is NULL, so calling pathconf() on it will crash the interpreter.

This change adds a helper that checks whether the original argument is index-like to determine if it's an fd, so we can correctly the behavior and avoid the crash.

There are other os functions that accept path_t and may have the same issue. Since that is outside the scope of the original issue, I think we should fix those in a separate PR.

static bool
path_is_fd(const path_t *path)
{
return path->allow_fd && path->object != NULL && PyIndex_Check(path->object);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the problem really with the check or with is it with the converter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: nvm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking path->object type (PyIndex_Check()), I would prefer adding a path_t.is_fd member and modify path_converter() to set this member.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@vstinner
Copy link
Member

vstinner commented Mar 2, 2026

Other functions have the same issues, even if they don't crash:

  • os.stat(-1)
  • os.statx(-1, mask=0)
  • os.chdir(-1)
  • os.chmod(-1, 0)
  • os.chown(-1, 0, 0)
  • os.listdir(-1): current it behaves as os.listdir(os.path.curdir)
  • os._path_exists(-1) (Windows only)
  • os._path_lexists(-1) (Windows only)
  • os._path_isdir(-1) (Windows only)
  • os._path_isfile(-1) (Windows only)
  • os._path_islink(-1) (Windows only)
  • os._path_isjunction(-1) (Windows only)
  • os.utime(-1, (0, 0))
  • os.execve(-1, ['/bin/true'], os.environ)
  • os.truncate(-1, 0)
  • os.statvfs(-1)
  • os.pathconf(-1, "PC_NAME_MAX") crash (issue Segfault from running os.pathconf(-1, 1) #145335)
  • os.getxattr(-1, "user")
  • os.setxattr(-1, "user", b"1")
  • os.removexattr(-1, "user")
  • os.listxattr(-1): current it behaves as os.listxattr(os.path.curdir)
  • os.scandir(-1): current it behaves as os.scandir(os.path.curdir)

On Linux (with the glibc), most functions fail with OSError: [Errno 14] Bad address: -1.

This list are functions using path_t(allow_fd=True) type in Argument Clinic, or variants such as path_t(allow_fd='PATH_HAVE_FCHDIR') type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants