os.confstr() does not handle value changing length between calls

[baikie]

BPO	9647
Nosy	@vstinner, @serhiy-storchaka
Files	confstr-realloc-endless-2.7.diff confstr-realloc-endless-3.4.diff confstr-realloc-endless-3.5.diff confstr-realloc-limited-2.7.diff confstr-realloc-limited-3.4.diff confstr-realloc-limited-3.5.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2010-08-19.18:44:52.032>
labels = ['extension-modules', 'type-bug']
title = 'os.confstr() does not handle value changing length between calls'
updated_at = <Date 2019-07-29.11:43:10.037>
user = 'https://bugs.python.org/baikie'

bugs.python.org fields:

activity = <Date 2019-07-29.11:43:10.037>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2010-08-19.18:44:52.032>
creator = 'baikie'
dependencies = []
files = ['37428', '37429', '37430', '37431', '37432', '37433']
hgrepos = []
issue_num = 9647
keywords = ['patch']
message_count = 11.0
messages = ['114396', '117633', '117898', '231997', '232216', '232217', '232220', '232280', '232283', '232572', '348627']
nosy_count = 4.0
nosy_names = ['vstinner', 'baikie', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9647'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[baikie]

This came up in relation to issue bpo-9579; there is some discussion
of it there. Basically, if os.confstr() has to call confstr()
twice because the buffer wasn't big enough the first time, the
existing code assumes the string is the same length that the OS
reported in the first call instead of using the length from the
second call and resizing the buffer if necessary. This means the
returned value will be truncated or contain trailing garbage if
the string changed its length betweeen calls.

I don't know of an actual environment where configuration strings
can change at runtime, but it's not forbidden by POSIX as far as
I can see (the strings are described as "variables", after all,
and sysconf() values such as CHILD_MAX can change at runtime).
Implementations can also provide additional confstr() variables
not specified by POSIX.

The patch confstr-long-result.diff at issue bpo-9579 would fix this
(for 3.x), but Victor Stinner has expressed concern that a buggy
confstr() could create a near-infinite loop with that patch
applied.

[vstinner]

If I understood correctly, you don't want the value to be truncated if the variable grows between the two calls to confstr(). Which behaviour would you expect? A Python exception?

but Victor Stinner has expressed concern that a buggy
confstr() could create a near-infinite loop with that patch
applied

Yes, I think that two calls to confstr() should be enough.

[baikie]

If I understood correctly, you don't want the value to be truncated if the variable grows between the two calls to confstr(). Which behaviour would you expect? A Python exception?

A return size larger than the buffer is *supposed* to indicate
that the current value is larger than the supplied buffer, so I
would just expect it to reallocate the buffer, call confstr()
again and return the new value, unless it was known that such a
situation indicated an actual problem.

In other words, I would not expect it to do anything special. I
didn't write the original patch the way I did in order to fix
this (potential) bug - it just seemed like the most natural way
to write the code.

[serhiy-storchaka]

I agree with Victor that two calls to confstr() should be enough. An example in confstr manpage uses two calls and I think there is no many software (if any) in the world which does more.

[python-dev]

New changeset a7a8947e9ce4 by Victor Stinner in branch 'default':
Issue bpo-9647: os.confstr() ensures that the second call to confstr() returns the
https://hg.python.org/cpython/rev/a7a8947e9ce4

[vstinner]

I added an assertion. Can we close this issue?

[vstinner]

(Oops, I didn't want to close the issue.)

[baikie]

On Fri 5 Dec 2014, STINNER Victor wrote:

I added an assertion. Can we close this issue?

Well, if no one complains about the interpreter dying with
SIGABRT, that will suggest that the worries about OS bugs
creating infinite loops were unfounded :)

If you do want to leave this open, I can provide patches based on
the original one from issue bpo-9579.

[serhiy-storchaka]

I don't think that assert() is appropriate solution here. assert() is used to check internal logic, it shouldn't check conditions which rely on external world. Raising RuntimeError looks more appropriate to me.

[baikie]

Here are the alternative patches to allow more than two calls to
confstr(). One patch set just keeps reallocating the buffer
until it's big enough, while the other makes a limited number of
attempts (in this case 20) before raising RuntimeError.