Skip to content

Commit

Permalink
Try to get correct buffer size to avoid races (ansible#53547)
Browse files Browse the repository at this point in the history
* Try to get correct buffer size to avoid races

  fixes ansible#51393

* fix test, mock buffer function since all is mocked
  • Loading branch information
bcoca authored Mar 14, 2019
1 parent 86405b8 commit e280f2f
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/avoid_race.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- Attempt to avoid race condition based on incorrect buffer size assumptions
38 changes: 27 additions & 11 deletions lib/ansible/module_utils/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,27 @@

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

Expand Down Expand Up @@ -2584,7 +2586,7 @@ def _unsafe_writes(self, src, dest):
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)

Expand Down Expand Up @@ -2905,6 +2907,20 @@ def human_to_bytes(self, number, isbits=False):
# 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__))
Expand Down
1 change: 1 addition & 0 deletions test/units/module_utils/basic/test_run_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit e280f2f

Please sign in to comment.