Skip to content

Commit 072a07c

Browse files
committed
struct dirent is variable length in the original POSIX specification (the final
filename field being an unsized array) and in actual practice on the WASI platform. Thus just copying one into a stack-allocated structure is not good enough. The obvious way to address this is to allocate memory on the heap and return that. However, it seems wasteful to dynamically allocate a temporary chunk of data just to copy some data in and then out again. Instead, if the name field is potentially not long enough, allocate some extra space *on the stack* following the structure such that it must be sufficient to hold the the full record length (and copy all of that, not just the potentially smaller fixed structure size).
1 parent 6b3b46d commit 072a07c

File tree

1 file changed

+27
-3
lines changed

1 file changed

+27
-3
lines changed

Modules/posixmodule.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@
204204
# include <sanitizer/msan_interface.h> // __msan_unpoison()
205205
#endif
206206

207+
#ifdef HAVE_ALLOCA_H
208+
#include <alloca.h>
209+
#endif
207210

208211
// --- More complex system includes -----------------------------------------
209212

@@ -16303,9 +16306,19 @@ ScandirIterator_nextdirentry(ScandirIterator *iterator, struct dirent *direntp)
1630316306
Py_END_ALLOW_THREADS
1630416307
has_error = !result && errno != 0;
1630516308
}
16309+
1630616310
/* We need to make a copy of the result before releasing the lock */
16307-
if (result)
16308-
memcpy(direntp, result, sizeof(struct dirent));
16311+
if (result) {
16312+
16313+
/* A note on calculating the size: the dirent structure may be variable
16314+
* length with the d_name field being a flexible array member, so we
16315+
* cannot just use sizeof(struct dirent). On Linux there is a d_reclen
16316+
* field, however that is not guaranteed to exist (and doesn't on WASI,
16317+
* which happens to be the platform where this is an issue). Hence we
16318+
* calculate the relevant record size thus. */
16319+
size_t reclen = offsetof(struct dirent, d_name) + strlen(result->d_name) + 1;
16320+
memcpy(direntp, result, reclen);
16321+
}
1630916322
PyMutex_Unlock(&iterator->mutex);
1631016323

1631116324
/* Error or no more files */
@@ -16320,12 +16333,22 @@ ScandirIterator_nextdirentry(ScandirIterator *iterator, struct dirent *direntp)
1632016333
static PyObject *
1632116334
ScandirIterator_iternext(PyObject *op)
1632216335
{
16336+
ScandirIterator *iterator = ScandirIterator_CAST(op);
1632316337
#ifdef MS_WINDOWS
1632416338
WIN32_FIND_DATAW dirent;
1632516339
#else
16340+
/*
16341+
* This may be a variable length structure, with the final "d_name" field
16342+
* possibly being a flexible array member. In that case allocate enough
16343+
* extra space on the stack, immediately following the structure,
16344+
* sufficient to handle the longest possible filename. */
1632616345
struct dirent dirent;
16346+
if (sizeof(dirent) - offsetof(struct dirent, d_name) < (NAME_MAX + 1)
16347+
&& !alloca(NAME_MAX + 1 - (sizeof(dirent) - offsetof(struct dirent, d_name)))) {
16348+
PyErr_NoMemory();
16349+
goto error;
16350+
}
1632716351
#endif
16328-
ScandirIterator *iterator = ScandirIterator_CAST(op);
1632916352

1633016353
while ((ScandirIterator_nextdirentry(iterator, &dirent))) {
1633116354

@@ -16341,6 +16364,7 @@ ScandirIterator_iternext(PyObject *op)
1634116364
}
1634216365

1634316366
/* Already closed, error, or no more files */
16367+
error:
1634416368
ScandirIterator_closedir(iterator);
1634516369
return NULL;
1634616370
}

0 commit comments

Comments
 (0)