Skip to content

Commit 3a97ab5

Browse files
committed
fix: prevent SU2 Python runner stderr deadlock and harden config path copies
- SU2_PY: use subprocess.communicate() to reliably drain stderr while keeping stdout streamed, avoiding potential hangs when stderr output is large. - SU2_PY: fix incorrect path reporting in error messages (abspath(".") instead of abspath(",")). - C++ entrypoints: guard/limit copies into fixed-size config filename buffers to prevent potential overflow on long paths. Signed-off-by: shbhmexe <[email protected]>
1 parent b91b14d commit 3a97ab5

File tree

6 files changed

+57
-13
lines changed

6 files changed

+57
-13
lines changed

SU2_CFD/src/SU2_CFD.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727

2828
#include "../include/SU2_CFD.hpp"
2929

30+
#include <cstring>
31+
3032
/* Include file, needed for the runtime NaN catching. You also have to include feenableexcept(...) below. */
3133
//#include <fenv.h>
3234

@@ -78,7 +80,11 @@ int main(int argc, char *argv[]) {
7880

7981
/*--- Load in the number of zones and spatial dimensions in the mesh file (If no config
8082
file is specified, default.cfg is used) ---*/
81-
strcpy(config_file_name, filename.c_str());
83+
if (filename.size() >= MAX_STRING_SIZE) {
84+
SU2_MPI::Error("Config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
85+
}
86+
strncpy(config_file_name, filename.c_str(), MAX_STRING_SIZE - 1);
87+
config_file_name[MAX_STRING_SIZE - 1] = '\0';
8288

8389
/*--- Read the name and format of the input mesh file to get from the mesh
8490
file the number of zones and dimensions from the numerical grid (required

SU2_DEF/src/SU2_DEF.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727

2828
#include "../include/drivers/CDeformationDriver.hpp"
2929

30+
#include <cstring>
31+
3032
int main(int argc, char* argv[]) {
3133
char config_file_name[MAX_STRING_SIZE];
3234

@@ -44,7 +46,11 @@ int main(int argc, char* argv[]) {
4446
(if no config file is specified, default.cfg is used). ---*/
4547

4648
if (argc == 2) {
47-
strcpy(config_file_name, argv[1]);
49+
if (strlen(argv[1]) >= MAX_STRING_SIZE) {
50+
SU2_MPI::Error("Config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
51+
}
52+
strncpy(config_file_name, argv[1], MAX_STRING_SIZE - 1);
53+
config_file_name[MAX_STRING_SIZE - 1] = '\0';
4854
} else {
4955
strcpy(config_file_name, "default.cfg");
5056
}

SU2_DOT/src/SU2_DOT.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727

2828
#include "../../SU2_DEF/include/drivers/CDiscAdjDeformationDriver.hpp"
2929

30+
#include <cstring>
31+
3032
int main(int argc, char* argv[]) {
3133
char config_file_name[MAX_STRING_SIZE];
3234

@@ -47,7 +49,11 @@ int main(int argc, char* argv[]) {
4749
(if no config file is specified, default.cfg is used). ---*/
4850

4951
if (argc == 2) {
50-
strcpy(config_file_name, argv[1]);
52+
if (strlen(argv[1]) >= MAX_STRING_SIZE) {
53+
SU2_MPI::Error("Config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
54+
}
55+
strncpy(config_file_name, argv[1], MAX_STRING_SIZE - 1);
56+
config_file_name[MAX_STRING_SIZE - 1] = '\0';
5157
} else {
5258
strcpy(config_file_name, "default.cfg");
5359
}

SU2_GEO/src/SU2_GEO.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
*/
2727

2828
#include "../include/SU2_GEO.hpp"
29+
30+
#include <cstring>
31+
2932
using namespace std;
3033

3134
int main(int argc, char* argv[]) {
@@ -92,7 +95,11 @@ int main(int argc, char* argv[]) {
9295
file is specified, default.cfg is used) ---*/
9396

9497
if (argc == 2) {
95-
strcpy(config_file_name, argv[1]);
98+
if (strlen(argv[1]) >= MAX_STRING_SIZE) {
99+
SU2_MPI::Error("Config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
100+
}
101+
strncpy(config_file_name, argv[1], MAX_STRING_SIZE - 1);
102+
config_file_name[MAX_STRING_SIZE - 1] = '\0';
96103
} else {
97104
strcpy(config_file_name, "default.cfg");
98105
}

SU2_PY/SU2/run/interface.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,13 @@
3838
# Setup
3939
# ------------------------------------------------------------
4040

41-
SU2_RUN = os.environ["SU2_RUN"]
41+
try:
42+
SU2_RUN = os.environ["SU2_RUN"]
43+
except KeyError as exc:
44+
raise RuntimeError(
45+
'Environment variable "SU2_RUN" is not set. Please set SU2_RUN to the SU2 installation bin directory.'
46+
) from exc
47+
4248
sys.path.append(SU2_RUN)
4349
quote = '"' if sys.platform == "win32" else ""
4450

@@ -263,11 +269,13 @@ def run_command(Command):
263269
"""
264270
sys.stdout.flush()
265271

266-
proc = subprocess.Popen(
267-
Command, shell=True, stdout=sys.stdout, stderr=subprocess.PIPE
268-
)
269-
return_code = proc.wait()
270-
message = proc.stderr.read().decode()
272+
# Avoid potential deadlocks when a child process writes heavily to stderr:
273+
# read stderr via communicate() while streaming stdout to the console.
274+
proc = subprocess.Popen(Command, shell=True, stdout=sys.stdout, stderr=subprocess.PIPE)
275+
_, stderr = proc.communicate()
276+
277+
return_code = proc.returncode
278+
message = (stderr or b"").decode(errors="replace")
271279

272280
if return_code < 0:
273281
message = "SU2 process was terminated by signal '%s'\n%s" % (
@@ -277,7 +285,7 @@ def run_command(Command):
277285
raise SystemExit(message)
278286
elif return_code > 0:
279287
message = "Path = %s\nCommand = %s\nSU2 process returned error '%s'\n%s" % (
280-
os.path.abspath(","),
288+
os.path.abspath("."),
281289
Command,
282290
return_code,
283291
message,

SU2_SOL/src/SU2_SOL.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727

2828
#include "../include/SU2_SOL.hpp"
2929

30+
#include <cstring>
31+
3032
using namespace std;
3133

3234
int main(int argc, char* argv[]) {
@@ -56,7 +58,11 @@ int main(int argc, char* argv[]) {
5658
file is specified, default.cfg is used) ---*/
5759

5860
if (argc == 2 || argc == 3) {
59-
strcpy(config_file_name, argv[1]);
61+
if (strlen(argv[1]) >= MAX_STRING_SIZE) {
62+
SU2_MPI::Error("Config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
63+
}
64+
strncpy(config_file_name, argv[1], MAX_STRING_SIZE - 1);
65+
config_file_name[MAX_STRING_SIZE - 1] = '\0';
6066
} else {
6167
strcpy(config_file_name, "default.cfg");
6268
}
@@ -98,7 +104,12 @@ int main(int argc, char* argv[]) {
98104
read and stored. ---*/
99105

100106
if (driver_config->GetnConfigFiles() > 0) {
101-
strcpy(zone_file_name, driver_config->GetConfigFilename(iZone).c_str());
107+
const auto& zone_filename = driver_config->GetConfigFilename(iZone);
108+
if (zone_filename.size() >= MAX_STRING_SIZE) {
109+
SU2_MPI::Error("Zone config file path too long (exceeds MAX_STRING_SIZE).", CURRENT_FUNCTION);
110+
}
111+
strncpy(zone_file_name, zone_filename.c_str(), MAX_STRING_SIZE - 1);
112+
zone_file_name[MAX_STRING_SIZE - 1] = '\0';
102113
config_container[iZone] = new CConfig(driver_config, zone_file_name, SU2_COMPONENT::SU2_SOL, iZone, nZone, true);
103114
} else {
104115
config_container[iZone] =

0 commit comments

Comments
 (0)