Skip to content

Commit 2276d94

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
Move out buffer allocation and Add cleanup phase for resource management
1 parent e8f54ae commit 2276d94

File tree

1 file changed

+91
-44
lines changed

1 file changed

+91
-44
lines changed

‎src/wolfsftp.c‎

Lines changed: 91 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,6 +2026,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20262026
word32 idx = 0;
20272027
int m = 0;
20282028
int ret = WS_SUCCESS;
2029+
int fdOpened = 0;
2030+
int outOwnedBySsh = 0;
20292031

20302032
word32 outSz = sizeof(WFD) + UINT32_SZ + WOLFSSH_SFTP_HEADER;
20312033
byte* out = NULL;
@@ -2034,6 +2036,9 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20342036
char ier[] = "Internal Failure";
20352037
char oer[] = "Open File Error";
20362038
char naf[] = "Not A File";
2039+
#ifdef WOLFSSH_STOREHANDLE
2040+
int handleStored = 0;
2041+
#endif
20372042

20382043
if (ssh == NULL) {
20392044
return WS_BAD_ARGUMENT;
@@ -2065,7 +2070,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20652070
if (GetAndCleanPath(ssh->sftpDefaultPath,
20662071
data + idx, sz, dir, sizeof(dir)) != WS_SUCCESS) {
20672072
WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
2068-
return WS_FATAL_ERROR;
2073+
ret = WS_FATAL_ERROR;
2074+
goto cleanup;
20692075
}
20702076
idx += sz;
20712077

@@ -2118,7 +2124,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21182124
res = naf;
21192125
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId,
21202126
res, "English", NULL, &outSz) != WS_SIZE_ONLY) {
2121-
return WS_FATAL_ERROR;
2127+
ret = WS_FATAL_ERROR;
2128+
goto cleanup;
21222129
}
21232130
ret = WS_FATAL_ERROR;
21242131
}
@@ -2146,10 +2153,14 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21462153
res = oer;
21472154
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
21482155
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2149-
return WS_FATAL_ERROR;
2156+
ret = WS_FATAL_ERROR;
2157+
goto cleanup;
21502158
}
21512159
ret = WS_BAD_FILE_E;
21522160
}
2161+
else {
2162+
fdOpened = 1;
2163+
}
21532164
}
21542165

21552166
#ifdef WOLFSSH_STOREHANDLE
@@ -2160,58 +2171,59 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21602171
res = ier;
21612172
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
21622173
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2163-
#ifdef MICROCHIP_MPLAB_HARMONY
2164-
WFCLOSE(ssh->fs, &fd);
2165-
#else
2166-
WCLOSE(ssh->fs, fd);
2167-
#endif
2168-
return WS_FATAL_ERROR;
2174+
ret = WS_FATAL_ERROR;
2175+
goto cleanup;
21692176
}
21702177
ret = WS_FATAL_ERROR;
21712178
}
2179+
else {
2180+
handleStored = 1;
2181+
}
21722182
}
21732183
#endif
21742184

2175-
if (ret == WS_SUCCESS) {
2176-
/* create packet */
2177-
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2178-
if (out == NULL) {
2179-
#ifdef MICROCHIP_MPLAB_HARMONY
2180-
WFCLOSE(ssh->fs, &fd);
2181-
#else
2182-
WCLOSE(ssh->fs, fd);
2183-
#endif
2184-
return WS_MEMORY_E;
2185-
}
2185+
/* create packet */
2186+
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2187+
if (out == NULL) {
2188+
ret = WS_MEMORY_E;
2189+
goto cleanup;
21862190
}
2191+
21872192
if (ret == WS_SUCCESS) {
21882193
if (SFTP_CreatePacket(ssh, WOLFSSH_FTP_HANDLE, out, outSz,
21892194
(byte*)&fd, sizeof(WFD)) != WS_SUCCESS) {
2190-
#ifdef MICROCHIP_MPLAB_HARMONY
2191-
WFCLOSE(ssh->fs, &fd);
2192-
#else
2193-
WCLOSE(ssh->fs, fd);
2194-
#endif
2195-
return WS_FATAL_ERROR;
2195+
ret = WS_FATAL_ERROR;
2196+
goto cleanup;
21962197
}
21972198
}
21982199
else {
21992200
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
22002201
"English", out, &outSz) != WS_SUCCESS) {
2201-
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2202-
if (fd >= 0) {
2203-
#ifdef MICROCHIP_MPLAB_HARMONY
2204-
WFCLOSE(ssh->fs, &fd);
2205-
#else
2206-
WCLOSE(ssh->fs, fd);
2207-
#endif
2208-
}
2209-
return WS_FATAL_ERROR;
2202+
ret = WS_FATAL_ERROR;
2203+
goto cleanup;
22102204
}
22112205
}
22122206

22132207
/* set send out buffer, "out" is taken by ssh */
22142208
wolfSSH_SFTP_RecvSetSend(ssh, out, outSz);
2209+
outOwnedBySsh = 1;
2210+
2211+
cleanup:
2212+
#ifdef WOLFSSH_STOREHANDLE
2213+
if (ret != WS_SUCCESS && handleStored) {
2214+
(void)SFTP_RemoveHandleNode(ssh, (byte*)&fd, sizeof(WFD));
2215+
}
2216+
#endif
2217+
if (!outOwnedBySsh && out != NULL) {
2218+
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2219+
}
2220+
if (ret != WS_SUCCESS && fdOpened) {
2221+
#ifdef MICROCHIP_MPLAB_HARMONY
2222+
WFCLOSE(ssh->fs, &fd);
2223+
#else
2224+
WCLOSE(ssh->fs, fd);
2225+
#endif
2226+
}
22152227

22162228
WOLFSSH_UNUSED(ier);
22172229
return ret;
@@ -2228,13 +2240,20 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
22282240
DWORD creationDisp = 0;
22292241
DWORD flagsAndAttrs = 0;
22302242
int ret = WS_SUCCESS;
2243+
int fileHandleOpened = 0;
2244+
int outOwnedBySsh = 0;
22312245

22322246
word32 outSz = sizeof(HANDLE) + UINT32_SZ + WOLFSSH_SFTP_HEADER;
22332247
byte* out = NULL;
22342248

22352249
char* res = NULL;
22362250
char ier[] = "Internal Failure";
22372251
char oer[] = "Open File Error";
2252+
#ifdef WOLFSSH_STOREHANDLE
2253+
int handleStored = 0;
2254+
#endif
2255+
2256+
fileHandle = INVALID_HANDLE_VALUE;
22382257

22392258
if (ssh == NULL) {
22402259
return WS_BAD_ARGUMENT;
@@ -2260,7 +2279,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
22602279
if (GetAndCleanPath(ssh->sftpDefaultPath, data + idx, sz, dir, sizeof(dir))
22612280
!= WS_SUCCESS) {
22622281
WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
2263-
return WS_FATAL_ERROR;
2282+
ret = WS_FATAL_ERROR;
2283+
goto cleanup;
22642284
}
22652285
idx += sz;
22662286

@@ -2305,45 +2325,72 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
23052325
res = oer;
23062326
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23072327
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2308-
return WS_FATAL_ERROR;
2328+
ret = WS_FATAL_ERROR;
2329+
goto cleanup;
23092330
}
23102331
ret = WS_BAD_FILE_E;
23112332
}
2333+
else {
2334+
fileHandleOpened = 1;
2335+
}
23122336

23132337
#ifdef WOLFSSH_STOREHANDLE
2314-
if (SFTP_AddHandleNode(ssh,
2315-
(byte*)&fileHandle, sizeof(HANDLE), dir) != WS_SUCCESS) {
2338+
if (ret == WS_SUCCESS) {
2339+
if (SFTP_AddHandleNode(ssh,
2340+
(byte*)&fileHandle, sizeof(HANDLE), dir) != WS_SUCCESS) {
23162341
WLOG(WS_LOG_SFTP, "Unable to store handle");
23172342
res = ier;
23182343
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23192344
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2320-
return WS_FATAL_ERROR;
2345+
ret = WS_FATAL_ERROR;
2346+
goto cleanup;
23212347
}
23222348
ret = WS_FATAL_ERROR;
2349+
}
2350+
else {
2351+
handleStored = 1;
2352+
}
23232353
}
23242354
#endif
23252355

23262356
/* create packet */
23272357
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
23282358
if (out == NULL) {
2329-
return WS_MEMORY_E;
2359+
ret = WS_MEMORY_E;
2360+
goto cleanup;
23302361
}
2362+
23312363
if (ret == WS_SUCCESS) {
23322364
if (SFTP_CreatePacket(ssh, WOLFSSH_FTP_HANDLE, out, outSz,
23332365
(byte*)&fileHandle, sizeof(HANDLE)) != WS_SUCCESS) {
2334-
return WS_FATAL_ERROR;
2366+
ret = WS_FATAL_ERROR;
2367+
goto cleanup;
23352368
}
23362369
}
23372370
else {
23382371
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23392372
"English", out, &outSz) != WS_SUCCESS) {
2340-
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2341-
return WS_FATAL_ERROR;
2373+
ret = WS_FATAL_ERROR;
2374+
goto cleanup;
23422375
}
23432376
}
23442377

23452378
/* set send out buffer, "out" is taken by ssh */
23462379
wolfSSH_SFTP_RecvSetSend(ssh, out, outSz);
2380+
outOwnedBySsh = 1;
2381+
2382+
cleanup:
2383+
#ifdef WOLFSSH_STOREHANDLE
2384+
if (ret != WS_SUCCESS && handleStored) {
2385+
(void)SFTP_RemoveHandleNode(ssh, (byte*)&fileHandle, sizeof(HANDLE));
2386+
}
2387+
#endif
2388+
if (!outOwnedBySsh && out != NULL) {
2389+
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2390+
}
2391+
if (ret != WS_SUCCESS && fileHandleOpened) {
2392+
CloseHandle(fileHandle);
2393+
}
23472394

23482395
WOLFSSH_UNUSED(ier);
23492396
return ret;

0 commit comments

Comments
 (0)