Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssl: reduce allocate space for ssl backend when FTP is not supported. #8471

Closed
wants to merge 1 commit into from

Conversation

MAntoniak
Copy link
Contributor

Additionally, adding a backend check for NULL everywhere

conn->proxy_ssl[SECONDARYSOCKET].backend = calloc(1, sslsize);
allocate_result &= conn->proxy_ssl[SECONDARYSOCKET].backend != NULL;
#endif
#endif
Copy link
Member

@bagder bagder Feb 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a total fan of how this turns one malloc into four for a regular build. How about instead making the function similar to this? (untested)

static struct connectdata *allocate_conn(struct Curl_easy *data)
{
  struct connectdata *conn = calloc(1, sizeof(struct connectdata));
  if(!conn)
    return NULL;

#ifdef USE_SSL
  /* The SSL backend-specific data (ssl_backend_data) objects are allocated as
     a separate array to ensure suitable alignment.
     Note that these backend pointers can be swapped by vtls (eg ssl backend
     data becomes proxy backend data). */
  {
    size_t onesize = Curl_ssl->sizeof_ssl_backend_data;
    size_t totalsize = onesize;
    void *ptr;
    bool allocate_result = true;

#ifndef CURL_DISABLE_FTP
    totalsize *= 2;
#endif
#ifndef CURL_DISABLE_PROXY
    totalsize *= 2;
#endif

    conn->ssl[FIRSTSOCKET].backend = ptr = calloc(1, totalsize);
    if(!ptr)
      return NULL;
#ifndef CURL_DISABLE_FTP
    conn->ssl[SECONDARYSOCKET].backend = ptr += onesize;
#endif
#ifndef CURL_DISABLE_PROXY
    conn->proxy_ssl[FIRSTSOCKET].backend = ptr += onesize;
#ifndef CURL_DISABLE_FTP
    conn->proxy_ssl[SECONDARYSOCKET].backend = ptr += onesize;
#endif
#endif
  }
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified the memory allocation as you wrote.

@@ -339,6 +339,8 @@ static CURLcode bearssl_connect_step1(struct Curl_easy *data,
struct in_addr addr;
#endif

DEBUGASSERT(backend != NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should remove != NULL from these checks, as we normally check pointers for being non-NULL like that everywhere else.

@@ -1862,6 +1867,8 @@ static CURLcode verifystatus(struct Curl_easy *data,
ASN1_GENERALIZEDTIME *rev, *thisupd, *nextupd;
int ret;

DEBUGASSERT(backend != NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first of 4 compiler warnings because you add the assert before the last variable declaration:

vtls/openssl.c:1872:3: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

@MAntoniak MAntoniak force-pushed the ssl_allocate branch 3 times, most recently from c8427f7 to 0aec3e4 Compare February 20, 2022 14:00
@bagder bagder closed this in ccc2752 Feb 21, 2022
@bagder
Copy link
Member

bagder commented Feb 21, 2022

Thanks!

@MAntoniak MAntoniak deleted the ssl_allocate branch February 21, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants