Issue 9647: os.confstr() does not handle value changing length between calls (original) (raw)

Created on 2010-08-19 18:44 by baikie, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
confstr-realloc-endless-2.7.diff baikie,2014-12-12 19:41
confstr-realloc-endless-3.4.diff baikie,2014-12-12 19:41
confstr-realloc-endless-3.5.diff baikie,2014-12-12 19:41 review
confstr-realloc-limited-2.7.diff baikie,2014-12-12 19:41
confstr-realloc-limited-3.4.diff baikie,2014-12-12 19:41
confstr-realloc-limited-3.5.diff baikie,2014-12-12 19:41 review
Messages (11)
msg114396 - (view) Author: David Watson (baikie) Date: 2010-08-19 18:44
This came up in relation to issue #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 #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.
msg117633 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-09-29 17:32
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.
msg117898 - (view) Author: David Watson (baikie) Date: 2010-10-02 21:25
> 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.
msg231997 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-02 14:14
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.
msg232216 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-12-05 21:52
New changeset a7a8947e9ce4 by Victor Stinner in branch 'default': Issue #9647: os.confstr() ensures that the second call to confstr() returns the https://hg.python.org/cpython/rev/a7a8947e9ce4
msg232217 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-12-05 22:00
I added an assertion. Can we close this issue?
msg232220 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-12-05 22:15
(Oops, I didn't want to close the issue.)
msg232280 - (view) Author: David Watson (baikie) Date: 2014-12-07 20:54
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 #9579.
msg232283 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-07 21:27
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.
msg232572 - (view) Author: David Watson (baikie) Date: 2014-12-12 19:41
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.
msg348627 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-29 11:43
This issue is 9 years old and has patches: it is no newcomer friendly, I remove the "easy" keyword.
History
Date User Action Args
2022-04-11 14:57:05 admin set github: 53856
2019-07-29 11:43:10 vstinner set keywords: - easymessages: +
2014-12-12 19:41:50 baikie set 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.diffkeywords: + patchmessages: +
2014-12-07 21:27:48 serhiy.storchaka set messages: + versions: - Python 2.6
2014-12-07 20:54:59 baikie set messages: +
2014-12-05 22:15:56 vstinner set status: closed -> openresolution: fixed -> messages: +
2014-12-05 22:00:10 vstinner set status: open -> closedresolution: fixedmessages: +
2014-12-05 21:52:30 python-dev set nosy: + python-devmessages: +
2014-12-02 14:14:10 serhiy.storchaka set versions: + Python 3.4, Python 3.5, - Python 3.1, Python 3.2nosy: + serhiy.storchakamessages: + keywords: + easystage: needs patch
2010-10-02 21:25:45 baikie set messages: +
2010-09-29 17:32:38 vstinner set nosy: + vstinnermessages: +
2010-08-19 18:44:52 baikie create