Skip to content

Commit 689fc11

Browse files
LibretroAdminLibretroAdmin
committed
net/ssl-mbed: fix sub-context leaks on ssl_socket_init error label
Deferred follow-up noted in e3dc586 ('ssl-mbed NULL-check calloc'). ssl_socket_init's error label (line 138 pre-patch) only did free(state), but the function initialised six mbedtls sub- contexts on 'state' before the two failure-checked calls that can branch to it: mbedtls_net_init(&state->net_ctx); mbedtls_ssl_init(&state->ctx); mbedtls_ssl_config_init(&state->conf); mbedtls_x509_crt_init(&state->ca); mbedtls_ctr_drbg_init(&state->ctr_drbg); mbedtls_entropy_init(&state->entropy); ... if (mbedtls_ctr_drbg_seed(...) != 0) goto error; if (mbedtls_x509_crt_parse(...) < 0) goto error; Each _init pairs with a matching _free that releases internally- allocated buffers (entropy pool, DRBG state, SSL and config contexts, X509 chain). Pre-patch, a ctr_drbg_seed or x509_crt_parse failure leaked all of them. Fix: mirror ssl_socket_free's teardown in the error label, in reverse of init order. Same five _free calls, same #ifdef gate for the X509 chain. mbedtls_net_free is intentionally NOT called - the net_ctx only holds state->net_ctx.fd which is the caller's fd parameter (assigned at line 120) and the caller still owns it on this error exit. ssl_socket_free similarly avoids mbedtls_net_free; it just socket_close's the fd through the separate ssl_socket_close path at line 345. The pre-existing 'if (state)' guard is now redundant (state is NULL-checked at line 102 immediately after the calloc, so the error label is only reachable with state != NULL). Kept it to minimise the diff. Thread-safety: unchanged. ssl_socket_init runs on the http- task worker thread and doesn't touch shared state. Reachability: every HTTPS connection that reaches the error label - either a ctr_drbg_seed failure (extremely rare, would indicate entropy source failure) or an x509_crt_parse failure on the bundled cacert_pem (would only happen after local cert data corruption). Neither is a common runtime condition, but when they do hit the leak is significant (~10-40KB depending on mbedtls build config) and repeated across every retry.
1 parent 0fe401c commit 689fc11

2 files changed

Lines changed: 113 additions & 16 deletions

File tree

‎gfx/drivers/d3d9hlsl.c‎

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3521,9 +3521,11 @@ static char *d3d9_hlsl_preprocess_includes(
35213521
size_t resolved_len = strlen(resolved_inc);
35223522
while (pos + resolved_len + 2 >= cap)
35233523
{
3524+
char *tmp;
35243525
cap *= 2;
3525-
out = (char*)realloc(out, cap);
3526-
if (!out) { free(resolved_inc); return NULL; }
3526+
if (!(tmp = (char*)realloc(out, cap)))
3527+
{ free(out); free(resolved_inc); return NULL; }
3528+
out = tmp;
35273529
}
35283530
memcpy(out + pos, resolved_inc, resolved_len);
35293531
pos += resolved_len;
@@ -3545,9 +3547,11 @@ static char *d3d9_hlsl_preprocess_includes(
35453547
{
35463548
while (pos + line_len + 1 >= cap)
35473549
{
3550+
char *tmp;
35483551
cap *= 2;
3549-
out = (char*)realloc(out, cap);
3550-
if (!out) return NULL;
3552+
if (!(tmp = (char*)realloc(out, cap)))
3553+
{ free(out); return NULL; }
3554+
out = tmp;
35513555
}
35523556
memcpy(out + pos, p, line_len);
35533557
pos += line_len;
@@ -3586,9 +3590,20 @@ static bool d3d9_hlsl_buf_append(char **buf, size_t *pos, size_t *cap,
35863590
{
35873591
while (*pos + len + 1 >= *cap)
35883592
{
3593+
char *tmp;
35893594
*cap *= 2;
3590-
*buf = (char*)realloc(*buf, *cap);
3591-
if (!*buf) return false;
3595+
/* realloc-into-tmp: the pre-patch '*buf = realloc(*buf, *cap)'
3596+
* form leaked the old buffer on OOM (self-assign overwrites
3597+
* the only pointer to it with NULL). On failure free the old
3598+
* buffer and clear the caller's pointer so they don't double-
3599+
* free or use a now-invalid pointer. */
3600+
if (!(tmp = (char*)realloc(*buf, *cap)))
3601+
{
3602+
free(*buf);
3603+
*buf = NULL;
3604+
return false;
3605+
}
3606+
*buf = tmp;
35923607
}
35933608
memcpy(*buf + *pos, str, len);
35943609
*pos += len;
@@ -3756,7 +3771,13 @@ static char *d3d9_hlsl_add_struct_semantics(const char *source)
37563771
{
37573772
size_t len = (size_t)(ob + 1 - p);
37583773
while (opos + len + 1 >= cap)
3759-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
3774+
{
3775+
char *tmp;
3776+
cap *= 2;
3777+
if (!(tmp = (char*)realloc(out, cap)))
3778+
{ free(out); return NULL; }
3779+
out = tmp;
3780+
}
37603781
memcpy(out + opos, p, len);
37613782
opos += len;
37623783
}
@@ -3805,20 +3826,38 @@ static char *d3d9_hlsl_add_struct_semantics(const char *source)
38053826
size_t sl = snprintf(sem, sizeof(sem),
38063827
" : TEXCOORD%d", texcoord_counter++);
38073828
while (opos + sl + 2 >= cap)
3808-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
3829+
{
3830+
char *tmp;
3831+
cap *= 2;
3832+
if (!(tmp = (char*)realloc(out, cap)))
3833+
{ free(out); return NULL; }
3834+
out = tmp;
3835+
}
38093836
memcpy(out + opos, sem, sl);
38103837
opos += sl;
38113838
}
38123839
}
38133840
while (opos + 2 >= cap)
3814-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
3841+
{
3842+
char *tmp;
3843+
cap *= 2;
3844+
if (!(tmp = (char*)realloc(out, cap)))
3845+
{ free(out); return NULL; }
3846+
out = tmp;
3847+
}
38153848
out[opos++] = *mp++;
38163849
}
38173850
}
38183851

38193852
/* Copy '}' and advance past struct */
38203853
while (opos + 2 >= cap)
3821-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
3854+
{
3855+
char *tmp;
3856+
cap *= 2;
3857+
if (!(tmp = (char*)realloc(out, cap)))
3858+
{ free(out); return NULL; }
3859+
out = tmp;
3860+
}
38223861
out[opos++] = '}';
38233862
p = cb;
38243863
continue;
@@ -3829,7 +3868,13 @@ static char *d3d9_hlsl_add_struct_semantics(const char *source)
38293868

38303869
/* Regular char */
38313870
while (opos + 2 >= cap)
3832-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
3871+
{
3872+
char *tmp;
3873+
cap *= 2;
3874+
if (!(tmp = (char*)realloc(out, cap)))
3875+
{ free(out); return NULL; }
3876+
out = tmp;
3877+
}
38333878
out[opos++] = *p++;
38343879
}
38353880

@@ -4037,7 +4082,13 @@ static char *d3d9_hlsl_decompose_struct_samplers(const char *source)
40374082
/* Copy 'VARNAME;' then add sampler declaration */
40384083
size_t chunk = (size_t)(after + 1 - p);
40394084
while (opos + chunk + ilen + 40 >= cap)
4040-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
4085+
{
4086+
char *tmp;
4087+
cap *= 2;
4088+
if (!(tmp = (char*)realloc(out, cap)))
4089+
{ free(out); return NULL; }
4090+
out = tmp;
4091+
}
40414092
memcpy(out + opos, p, chunk);
40424093
opos += chunk;
40434094

@@ -4076,7 +4127,13 @@ static char *d3d9_hlsl_decompose_struct_samplers(const char *source)
40764127
}
40774128

40784129
while (opos + 14 >= cap)
4079-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
4130+
{
4131+
char *tmp;
4132+
cap *= 2;
4133+
if (!(tmp = (char*)realloc(out, cap)))
4134+
{ free(out); return NULL; }
4135+
out = tmp;
4136+
}
40804137

40814138
if (has_paste)
40824139
{
@@ -4095,7 +4152,13 @@ static char *d3d9_hlsl_decompose_struct_samplers(const char *source)
40954152

40964153
/* Regular char */
40974154
while (opos + 2 >= cap)
4098-
{ cap *= 2; out = (char*)realloc(out, cap); if (!out) return NULL; }
4155+
{
4156+
char *tmp;
4157+
cap *= 2;
4158+
if (!(tmp = (char*)realloc(out, cap)))
4159+
{ free(out); return NULL; }
4160+
out = tmp;
4161+
}
40994162
out[opos++] = *p++;
41004163
}
41014164

@@ -5087,8 +5150,13 @@ static char *d3d9_hlsl_fixup_cg_source(const char *source)
50875150
if (!already_global)
50885151
{
50895152
while (pos + hoist_len + 1 >= cap)
5090-
{ cap *= 2; out = (char*)realloc(out, cap);
5091-
if (!out) return NULL; }
5153+
{
5154+
char *tmp;
5155+
cap *= 2;
5156+
if (!(tmp = (char*)realloc(out, cap)))
5157+
{ free(out); return NULL; }
5158+
out = tmp;
5159+
}
50925160
memmove(out + last_func_start + hoist_len,
50935161
out + last_func_start, pos - last_func_start);
50945162
memcpy(out + last_func_start, hoist, hoist_len);

‎libretro-common/net/net_socket_ssl_mbed.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,37 @@ void* ssl_socket_init(int fd, const char *domain)
136136
return state;
137137

138138
error:
139+
/* Pair each non-trivial _init call above (lines 112-118) with
140+
* its matching _free. Pre-patch only free(state) was called,
141+
* leaking the five mbedtls sub-context heap allocations that
142+
* the _init / _free functions internally manage (entropy
143+
* pool buffers, DRBG state buffers, SSL context buffers,
144+
* config buffers, X509 chain).
145+
*
146+
* Mirrors the normal-teardown path in ssl_socket_free below
147+
* (lines 355-361) in reverse of init order. mbedtls_net_free
148+
* is intentionally not called for net_ctx (line 111):
149+
* - init sets state->net_ctx.fd from the caller's fd
150+
* parameter at line 120, and the caller still owns that
151+
* fd on this error exit.
152+
* - ssl_socket_free similarly never calls mbedtls_net_free;
153+
* it just socket_close's the fd via ssl_socket_close at
154+
* line 345.
155+
*
156+
* The if (state) guard is redundant - the state calloc is
157+
* NULL-checked at line 102 so we can't reach this label with
158+
* state == NULL - but leave it to keep the diff minimal. */
139159
if (state)
160+
{
161+
mbedtls_entropy_free(&state->entropy);
162+
mbedtls_ctr_drbg_free(&state->ctr_drbg);
163+
#if defined(MBEDTLS_X509_CRT_PARSE_C)
164+
mbedtls_x509_crt_free(&state->ca);
165+
#endif
166+
mbedtls_ssl_config_free(&state->conf);
167+
mbedtls_ssl_free(&state->ctx);
140168
free(state);
169+
}
141170
return NULL;
142171
}
143172

0 commit comments

Comments
 (0)