From e280f2f7b07e249d116c8b1cd63670044e602119 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 14 Mar 2019 11:04:56 -0400 Subject: [PATCH] Try to get correct buffer size to avoid races (#53547) * Try to get correct buffer size to avoid races fixes #51393 * fix test, mock buffer function since all is mocked --- changelogs/fragments/avoid_race.yml | 2 + lib/ansible/module_utils/basic.py | 38 +++++++++++++------ .../module_utils/basic/test_run_command.py | 1 + 3 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/avoid_race.yml diff --git a/changelogs/fragments/avoid_race.yml b/changelogs/fragments/avoid_race.yml new file mode 100644 index 00000000000..6ad9b4c51ca --- /dev/null +++ b/changelogs/fragments/avoid_race.yml @@ -0,0 +1,2 @@ +bugfixes: + - Attempt to avoid race condition based on incorrect buffer size assumptions diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index d63b61bfe36..f98a0b7fd4d 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -47,25 +47,27 @@ PASS_BOOLS = ('no_log', 'debug', 'diff') import __main__ import atexit +import errno +import datetime +import grp +import fcntl import locale import os +import pwd +import platform import re +import select import shlex +import shutil import signal +import stat import subprocess import sys -import types -import time -import select -import shutil -import stat import tempfile +import time import traceback -import grp -import pwd -import platform -import errno -import datetime +import types + from collections import deque from itertools import chain, repeat @@ -2584,7 +2586,7 @@ class AnsibleModule(object): def _read_from_pipes(self, rpipes, rfds, file_descriptor): data = b('') if file_descriptor in rfds: - data = os.read(file_descriptor.fileno(), 9000) + data = os.read(file_descriptor.fileno(), self.get_buffer_size(file_descriptor)) if data == b(''): rpipes.remove(file_descriptor) @@ -2905,6 +2907,20 @@ class AnsibleModule(object): # In 2.0, moved from inside the module to the toplevel is_executable = is_executable + @staticmethod + def get_buffer_size(fd): + try: + # 1032 == FZ_GETPIPE_SZ + buffer_size = fcntl.fcntl(fd, 1032) + except Exception: + try: + # not as exact as above, but should be good enough for most platforms that fail the previous call + buffer_size = select.PIPE_BUF + except Exception: + buffer_size = 9000 # use sane default JIC + + return buffer_size + def get_module_path(): return os.path.dirname(os.path.realpath(__file__)) diff --git a/test/units/module_utils/basic/test_run_command.py b/test/units/module_utils/basic/test_run_command.py index cd1971d3f36..607f3f47dad 100644 --- a/test/units/module_utils/basic/test_run_command.py +++ b/test/units/module_utils/basic/test_run_command.py @@ -83,6 +83,7 @@ def rc_am(mocker, am, mock_os, mock_subprocess): am.fail_json = mocker.MagicMock(side_effect=SystemExit) am._os = mock_os am._subprocess = mock_subprocess + am.get_buffer_size = mocker.MagicMock(return_value=900) yield am