Skip to content

Commit e5c990c

Browse files
committed
mingw: be *very* wary about outside environment changes
The environment is modified in most surprising circumstances, and not all of them are under Git's control. For example, calling curl_global_init() on Windows will ensure that the CHARSET variable is set, adding one if necessary. While the previous commit worked around crashes triggered by such outside changes of the environment by relaxing the requirement that the environment be terminated by a NULL pointer, the other assumption made by `mingw_getenv()` and `mingw_putenv()` is that the environment is sorted, for efficient lookup via binary search. Let's make real sure that our environment is intact before querying or modifying it, and reinitialize our idea of the environment if necessary. With this commit, before working on the environment we look briefly for indicators that the environment was modified outside of our control, and to ensure that it is terminated with a NULL pointer and sorted again in that case. Note: the indicators are maybe not sufficient. For example, when a variable is removed, it will not be noticed. It might also be a problem if outside changes to the environment result in a modified `environ` pointer: it is unclear whether such a modification could result in a problem when `mingw_putenv()` needs to `realloc()` the environment buffer. For the moment, however, the current fix works well enough, so let's only face the potential problems when (and if!) they occur. Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 3a532ba commit e5c990c

File tree

1 file changed

+47
-2
lines changed

1 file changed

+47
-2
lines changed

compat/mingw.c

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,10 @@ static int do_putenv(char **env, const char *name, int size, int free_old);
10461046
static int environ_size = 0;
10471047
/* allocated size of environ array, in bytes */
10481048
static int environ_alloc = 0;
1049+
/* used as a indicator when the environment has been changed outside mingw.c */
1050+
static char **saved_environ;
1051+
1052+
static void maybe_reinitialize_environ(void);
10491053

10501054
/*
10511055
* Create environment block suitable for CreateProcess. Merges current
@@ -1055,7 +1059,10 @@ static wchar_t *make_environment_block(char **deltaenv)
10551059
{
10561060
wchar_t *wenvblk = NULL;
10571061
char **tmpenv;
1058-
int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
1062+
int i = 0, size, wenvsz = 0, wenvpos = 0;
1063+
1064+
maybe_reinitialize_environ();
1065+
size = environ_size;
10591066

10601067
while (deltaenv && deltaenv[i] && *deltaenv[i])
10611068
i++;
@@ -1391,6 +1398,41 @@ static int compareenv(const void *v1, const void *v2)
13911398
}
13921399
}
13931400

1401+
/*
1402+
* Functions implemented outside Git are able to modify the environment,
1403+
* too. For example, cURL's curl_global_init() function sets the CHARSET
1404+
* environment variable (at least in certain circumstances).
1405+
*
1406+
* Therefore we need to be *really* careful *not* to assume that we have
1407+
* sole control over the environment and reinitialize it when necessary.
1408+
*/
1409+
static void maybe_reinitialize_environ(void)
1410+
{
1411+
int i;
1412+
1413+
if (!saved_environ) {
1414+
warning("MinGW environment not initialized yet");
1415+
return;
1416+
}
1417+
1418+
if (environ_size <= 0)
1419+
return;
1420+
1421+
if (saved_environ != environ)
1422+
/* We have *no* idea how much space was allocated outside */
1423+
environ_alloc = 0;
1424+
else if (!environ[environ_size - 1])
1425+
return; /* still consistent */
1426+
1427+
for (i = 0; environ[i] && *environ[i]; i++)
1428+
; /* continue counting */
1429+
environ[i] = NULL;
1430+
environ_size = i + 1;
1431+
1432+
/* sort environment for O(log n) getenv / putenv */
1433+
qsort(environ, i, sizeof(char*), compareenv);
1434+
}
1435+
13941436
static int bsearchenv(char **env, const char *name, size_t size)
13951437
{
13961438
unsigned low = 0, high = size;
@@ -1444,6 +1486,7 @@ char *mingw_getenv(const char *name)
14441486
if (environ_size <= 0)
14451487
return NULL;
14461488

1489+
maybe_reinitialize_environ();
14471490
pos = bsearchenv(environ, name, environ_size - 1);
14481491

14491492
if (pos < 0)
@@ -1454,7 +1497,9 @@ char *mingw_getenv(const char *name)
14541497

14551498
int mingw_putenv(const char *namevalue)
14561499
{
1500+
maybe_reinitialize_environ();
14571501
ALLOC_GROW(environ, (environ_size + 1) * sizeof(char*), environ_alloc);
1502+
saved_environ = environ;
14581503
environ_size = do_putenv(environ, namevalue, environ_size, 1);
14591504
return 0;
14601505
}
@@ -2391,7 +2436,7 @@ void mingw_startup(void)
23912436
*/
23922437
environ_size = i + 1;
23932438
environ_alloc = alloc_nr(environ_size * sizeof(char*));
2394-
environ = malloc_startup(environ_alloc);
2439+
saved_environ = environ = malloc_startup(environ_alloc);
23952440

23962441
/* allocate buffer (wchar_t encodes to max 3 UTF-8 bytes) */
23972442
maxlen = 3 * maxlen + 1;

0 commit comments

Comments
 (0)