Skip to content

Commit

Permalink
apport_python_hook: Fix crash if os.getcwd() fails
Browse files Browse the repository at this point in the history
Example `getcwd.py`:

```python
import os
import tempfile

tempdir = tempfile.mkdtemp()
os.chdir(tempdir)
os.rmdir(tempdir)
os.getcwd()
```

Apport's Python hook will fail:

```
$ python3 getcwd.py
Traceback (most recent call last):
  File "getcwd.py", line 7, in <module>
    os.getcwd()
FileNotFoundError: [Errno 2] No such file or directory
Error in sys.excepthook:
Traceback (most recent call last):
  File "apport_python_hook.py", line 71, in apport_excepthook
    binary = os.path.realpath(os.path.join(os.getcwd(), sys.argv[0]))
FileNotFoundError: [Errno 2] No such file or directory

Original exception was:
Traceback (most recent call last):
  File "getcwd.py", line 7, in <module>
    os.getcwd()
FileNotFoundError: [Errno 2] No such file or directory
```

Since the program can change the current directory and mutate `sys.argv`
(and also change `/proc/self/cmdline` using the `setproctitle` Python
module), there is no alternative to record the binary name during starup
when registering the Python hook.

Bug: https://launchpad.net/bugs/1977954
Signed-off-by: Benjamin Drung <[email protected]>
  • Loading branch information
bdrung committed Jun 23, 2022
1 parent 97f3e56 commit ad6d801
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 25 deletions.
40 changes: 25 additions & 15 deletions apport_python_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def enabled():
return True


def apport_excepthook(exc_type, exc_obj, exc_tb):
def apport_excepthook(binary, exc_type, exc_obj, exc_tb):
"""Catch an uncaught exception and make a traceback."""

# create and save a problem report. Note that exceptions in this code
Expand Down Expand Up @@ -66,21 +66,12 @@ def apport_excepthook(exc_type, exc_obj, exc_tb):
except (ImportError, IOError):
return

# apport will look up the package from the executable path.
try:
binary = os.path.realpath(os.path.join(os.getcwd(), sys.argv[0]))
except (TypeError, AttributeError, IndexError):
# the module has mutated sys.argv, plan B
try:
binary = os.readlink("/proc/%i/exe" % os.getpid())
except OSError:
return

# for interactive python sessions, sys.argv[0] == ''; catch that and
# other irregularities
if not os.access(binary, os.X_OK) or not os.path.isfile(binary):
# for interactive Python sessions, sys.argv[0] == ""
if not binary:
return

binary = os.path.realpath(binary)

# filter out binaries in user accessible paths
if not likely_packaged(binary):
return
Expand Down Expand Up @@ -220,4 +211,23 @@ def dbus_service_unknown_analysis(exc_obj, report):
def install():
"""Install the python apport hook."""

sys.excepthook = apport_excepthook
# Record before the program can mutate sys.argv and can call os.chdir().
binary = sys.argv[0]
if binary and not binary.startswith("/"):
# pylint: disable=import-outside-toplevel; for Python starup time
import os

try:
binary = f"{os.getcwd()}/{binary}"
except FileNotFoundError:
try:
binary = os.readlink("/proc/self/cwd")
if binary.endswith(" (deleted)"):
binary = binary[:-10]
except IOError:
return

def partial_apport_excepthook(exc_type, exc_obj, exc_tb):
return apport_excepthook(binary, exc_type, exc_obj, exc_tb)

sys.excepthook = partial_apport_excepthook
54 changes: 44 additions & 10 deletions tests/integration/test_python_crashes.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def tearDown(self):
for f in apport.fileutils.get_all_reports():
os.unlink(f)

def _test_crash(self, extracode="", scriptname=None):
def _test_crash(self, extracode="", scriptname=None, relpath=False):
"""Create a test crash."""

# put the script into /var/tmp, since that isn't ignored in the
Expand Down Expand Up @@ -73,12 +73,24 @@ def func(x):
)
os.close(fd)
os.chmod(script, 0o755)
env = self.env.copy()
env["PYTHONPATH"] = f"{env.get('PYTHONPATH', '.')}:/my/bogus/path"

p = subprocess.Popen(
[script, "testarg1", "testarg2"], stderr=subprocess.PIPE, env=env
)
if relpath:
binary = os.path.join(".", os.path.basename(script))
orig_cwd = os.getcwd()
os.chdir(os.path.dirname(script))
else:
binary = script
orig_cwd = "."
env = self.env.copy()
env["PYTHONPATH"] = f"{env.get('PYTHONPATH', orig_cwd)}:/my/bogus/path"
try:
p = subprocess.Popen(
[binary, "testarg1", "testarg2"],
stderr=subprocess.PIPE,
env=env,
)
finally:
os.chdir(orig_cwd)
err = p.communicate()[1].decode()
self.assertEqual(
p.returncode,
Expand Down Expand Up @@ -224,7 +236,7 @@ def test_symlink(self):
def test_no_argv(self):
"""with zapped sys.argv."""

self._test_crash("import sys\nsys.argv = None")
script = self._test_crash("import sys\nsys.argv = None")

# did we get a report?
reports = apport.fileutils.get_new_reports()
Expand Down Expand Up @@ -260,9 +272,7 @@ def test_no_argv(self):
"report has necessary fields",
)
self.assertIn("bin/python", pr["InterpreterPath"])
# we have no actual executable, so we should fall back to the
# interpreter
self.assertEqual(pr["ExecutablePath"], pr["InterpreterPath"])
self.assertEqual(pr["ExecutablePath"], script)
if "ExecutableTimestamp" in pr:
self.assertEqual(
pr["ExecutableTimestamp"],
Expand Down Expand Up @@ -503,6 +513,30 @@ def g():
pr.crash_signature(), "%s:OSError:%s@11:g" % (exe, exe)
)

def test_getcwd_error(self):
"""FileNotFoundError on os.getcwd() call"""
for relpath in (True, False):
with self.subTest(relpath=relpath):
self._test_crash(
extracode=textwrap.dedent(
"""\
import os
import tempfile
tempdir = tempfile.mkdtemp()
os.chdir(tempdir)
os.rmdir(tempdir)
os.getcwd()
"""
),
relpath=relpath,
)
pr = self._load_report()
self.assertIn(":FileNotFoundError:", pr.crash_signature())
self.assertIn(
"os.getcwd()\nFileNotFoundError", pr["Traceback"]
)

def test_subclassed_os_error(self):
"""OSError with known subclass"""

Expand Down

0 comments on commit ad6d801

Please sign in to comment.