Skip to content

Commit 1f3532f

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 370627c commit 1f3532f

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
@@ -1015,6 +1015,10 @@ static int do_putenv(char **env, const char *name, int size, int free_old);
10151015
static int environ_size = 0;
10161016
/* allocated size of environ array, in bytes */
10171017
static int environ_alloc = 0;
1018+
/* used as a indicator when the environment has been changed outside mingw.c */
1019+
static char **saved_environ;
1020+
1021+
static void maybe_reinitialize_environ(void);
10181022

10191023
/*
10201024
* Create environment block suitable for CreateProcess. Merges current
@@ -1024,7 +1028,10 @@ static wchar_t *make_environment_block(char **deltaenv)
10241028
{
10251029
wchar_t *wenvblk = NULL;
10261030
char **tmpenv;
1027-
int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
1031+
int i = 0, size, wenvsz = 0, wenvpos = 0;
1032+
1033+
maybe_reinitialize_environ();
1034+
size = environ_size;
10281035

10291036
while (deltaenv && deltaenv[i] && *deltaenv[i])
10301037
i++;
@@ -1327,6 +1334,41 @@ static int compareenv(const void *v1, const void *v2)
13271334
}
13281335
}
13291336

1337+
/*
1338+
* Functions implemented outside Git are able to modify the environment,
1339+
* too. For example, cURL's curl_global_init() function sets the CHARSET
1340+
* environment variable (at least in certain circumstances).
1341+
*
1342+
* Therefore we need to be *really* careful *not* to assume that we have
1343+
* sole control over the environment and reinitalize it when necessary.
1344+
*/
1345+
static void maybe_reinitialize_environ(void)
1346+
{
1347+
int i;
1348+
1349+
if (!saved_environ) {
1350+
warning("MinGW environment not initialized yet");
1351+
return;
1352+
}
1353+
1354+
if (environ_size <= 0)
1355+
return;
1356+
1357+
if (saved_environ != environ)
1358+
/* We have *no* idea how much space was allocated outside */
1359+
environ_alloc = 0;
1360+
else if (!environ[environ_size - 1])
1361+
return; /* still consistent */
1362+
1363+
for (i = 0; environ[i] && *environ[i]; i++)
1364+
; /* continue counting */
1365+
environ[i] = NULL;
1366+
environ_size = i + 1;
1367+
1368+
/* sort environment for O(log n) getenv / putenv */
1369+
qsort(environ, i, sizeof(char*), compareenv);
1370+
}
1371+
13301372
static int bsearchenv(char **env, const char *name, size_t size)
13311373
{
13321374
unsigned low = 0, high = size;
@@ -1380,6 +1422,7 @@ char *mingw_getenv(const char *name)
13801422
if (environ_size <= 0)
13811423
return NULL;
13821424

1425+
maybe_reinitialize_environ();
13831426
pos = bsearchenv(environ, name, environ_size - 1);
13841427

13851428
if (pos < 0)
@@ -1390,7 +1433,9 @@ char *mingw_getenv(const char *name)
13901433

13911434
int mingw_putenv(const char *namevalue)
13921435
{
1436+
maybe_reinitialize_environ();
13931437
ALLOC_GROW(environ, (environ_size + 1) * sizeof(char*), environ_alloc);
1438+
saved_environ = environ;
13941439
environ_size = do_putenv(environ, namevalue, environ_size, 1);
13951440
return 0;
13961441
}
@@ -2313,7 +2358,7 @@ void mingw_startup()
23132358
*/
23142359
environ_size = i + 1;
23152360
environ_alloc = alloc_nr(environ_size * sizeof(char*));
2316-
environ = malloc_startup(environ_alloc);
2361+
saved_environ = environ = malloc_startup(environ_alloc);
23172362

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

0 commit comments

Comments
 (0)