From e077455e9e57ed4ee4676996b4a9aa11df6327a6 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Thu, 29 Sep 2022 13:57:34 +0200 Subject: Stop raising ERR_R_MALLOC_FAILURE in most places Since OPENSSL_malloc() and friends report ERR_R_MALLOC_FAILURE, and at least handle the file name and line number they are called from, there's no need to report ERR_R_MALLOC_FAILURE where they are called directly, or when SSLfatal() and RLAYERfatal() is used, the reason `ERR_R_MALLOC_FAILURE` is changed to `ERR_R_CRYPTO_LIB`. There were a number of places where `ERR_R_MALLOC_FAILURE` was reported even though it was a function from a different sub-system that was called. Those places are changed to report ERR_R_{lib}_LIB, where {lib} is the name of that sub-system. Some of them are tricky to get right, as we have a lot of functions that belong in the ASN1 sub-system, and all the `sk_` calls or from the CRYPTO sub-system. Some extra adaptation was necessary where there were custom OPENSSL_malloc() wrappers, and some bugs are fixed alongside these changes. Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/19301) --- crypto/bio/bf_buff.c | 9 +++------ crypto/bio/bf_lbuf.c | 10 ++-------- crypto/bio/bf_nbio.c | 4 +--- crypto/bio/bio_addr.c | 30 ++++++++++++------------------ crypto/bio/bio_lib.c | 6 ++---- crypto/bio/bio_meth.c | 4 ++-- crypto/bio/bio_print.c | 8 ++------ crypto/bio/bio_sock.c | 7 ++++--- crypto/bio/bss_acpt.c | 4 +--- crypto/bio/bss_bio.c | 8 ++------ crypto/bio/bss_conn.c | 4 +--- crypto/bio/bss_dgram.c | 8 ++------ crypto/bio/bss_dgram_pair.c | 4 ++-- crypto/bio/bss_log.c | 4 +--- crypto/bio/bss_mem.c | 4 +--- 15 files changed, 38 insertions(+), 76 deletions(-) (limited to 'crypto/bio') diff --git a/crypto/bio/bf_buff.c b/crypto/bio/bf_buff.c index cfed63bd72..fe0ca92225 100644 --- a/crypto/bio/bf_buff.c +++ b/crypto/bio/bf_buff.c @@ -291,7 +291,7 @@ static long buffer_ctrl(BIO *b, int cmd, long num, void *ptr) return 0; p1 = OPENSSL_malloc((size_t)num); if (p1 == NULL) - goto malloc_error; + return 0; OPENSSL_free(ctx->ibuf); ctx->ibuf = p1; } @@ -322,14 +322,14 @@ static long buffer_ctrl(BIO *b, int cmd, long num, void *ptr) return 0; p1 = OPENSSL_malloc((size_t)num); if (p1 == NULL) - goto malloc_error; + return 0; } if ((obs > DEFAULT_BUFFER_SIZE) && (obs != ctx->obuf_size)) { p2 = OPENSSL_malloc((size_t)num); if (p2 == NULL) { if (p1 != ctx->ibuf) OPENSSL_free(p1); - goto malloc_error; + return 0; } } if (ctx->ibuf != p1) { @@ -405,9 +405,6 @@ static long buffer_ctrl(BIO *b, int cmd, long num, void *ptr) break; } return ret; - malloc_error: - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); - return 0; } static long buffer_callback_ctrl(BIO *b, int cmd, BIO_info_cb *fp) diff --git a/crypto/bio/bf_lbuf.c b/crypto/bio/bf_lbuf.c index 73f1216987..8854b4144c 100644 --- a/crypto/bio/bf_lbuf.c +++ b/crypto/bio/bf_lbuf.c @@ -57,13 +57,10 @@ static int linebuffer_new(BIO *bi) { BIO_LINEBUFFER_CTX *ctx; - if ((ctx = OPENSSL_malloc(sizeof(*ctx))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((ctx = OPENSSL_malloc(sizeof(*ctx))) == NULL) return 0; - } ctx->obuf = OPENSSL_malloc(DEFAULT_LINEBUFFER_SIZE); if (ctx->obuf == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); OPENSSL_free(ctx); return 0; } @@ -237,7 +234,7 @@ static long linebuffer_ctrl(BIO *b, int cmd, long num, void *ptr) if ((obs > DEFAULT_LINEBUFFER_SIZE) && (obs != ctx->obuf_size)) { p = OPENSSL_malloc((size_t)obs); if (p == NULL) - goto malloc_error; + return 0; } if (ctx->obuf != p) { if (ctx->obuf_len > obs) { @@ -294,9 +291,6 @@ static long linebuffer_ctrl(BIO *b, int cmd, long num, void *ptr) break; } return ret; - malloc_error: - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); - return 0; } static long linebuffer_callback_ctrl(BIO *b, int cmd, BIO_info_cb *fp) diff --git a/crypto/bio/bf_nbio.c b/crypto/bio/bf_nbio.c index f9ea1730ba..01138729b0 100644 --- a/crypto/bio/bf_nbio.c +++ b/crypto/bio/bf_nbio.c @@ -55,10 +55,8 @@ static int nbiof_new(BIO *bi) { NBIO_TEST *nt; - if ((nt = OPENSSL_zalloc(sizeof(*nt))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((nt = OPENSSL_zalloc(sizeof(*nt))) == NULL) return 0; - } nt->lrn = -1; nt->lwn = -1; bi->ptr = (char *)nt; diff --git a/crypto/bio/bio_addr.c b/crypto/bio/bio_addr.c index 747777a5ab..3c09543194 100644 --- a/crypto/bio/bio_addr.c +++ b/crypto/bio/bio_addr.c @@ -53,10 +53,8 @@ BIO_ADDR *BIO_ADDR_new(void) { BIO_ADDR *ret = OPENSSL_zalloc(sizeof(*ret)); - if (ret == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (ret == NULL) return NULL; - } ret->sa.sa_family = AF_UNSPEC; return ret; @@ -279,7 +277,6 @@ static int addr_strings(const BIO_ADDR *ap, int numeric, OPENSSL_free(*service); *service = NULL; } - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); return 0; } @@ -550,7 +547,7 @@ int BIO_parse_hostserv(const char *hostserv, char **host, char **service, } else { *host = OPENSSL_strndup(h, hl); if (*host == NULL) - goto memerr; + return 0; } } if (p != NULL && service != NULL) { @@ -560,7 +557,7 @@ int BIO_parse_hostserv(const char *hostserv, char **host, char **service, } else { *service = OPENSSL_strndup(p, pl); if (*service == NULL) - goto memerr; + return 0; } } @@ -571,9 +568,6 @@ int BIO_parse_hostserv(const char *hostserv, char **host, char **service, spec_err: ERR_raise(ERR_LIB_BIO, BIO_R_MALFORMED_HOST_OR_SERVICE); return 0; - memerr: - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); - return 0; } /* addrinfo_wrap is used to build our own addrinfo "chain". @@ -590,10 +584,8 @@ static int addrinfo_wrap(int family, int socktype, unsigned short port, BIO_ADDRINFO **bai) { - if ((*bai = OPENSSL_zalloc(sizeof(**bai))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((*bai = OPENSSL_zalloc(sizeof(**bai))) == NULL) return 0; - } (*bai)->bai_family = family; (*bai)->bai_socktype = socktype; @@ -688,7 +680,7 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type, if (addrinfo_wrap(family, socktype, host, strlen(host), 0, res)) return 1; else - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_BIO, ERR_R_BIO_LIB); return 0; } #endif @@ -732,7 +724,8 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type, # endif # ifdef EAI_MEMORY case EAI_MEMORY: - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise_data(ERR_LIB_BIO, ERR_R_SYS_LIB, + gai_strerror(old_ret ? old_ret : gai_ret)); break; # endif case 0: @@ -789,7 +782,8 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type, #endif if (!RUN_ONCE(&bio_lookup_init, do_bio_lookup_init)) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + /* Should this be raised inside do_bio_lookup_init()? */ + ERR_raise(ERR_LIB_BIO, ERR_R_CRYPTO_LIB); ret = 0; goto err; } @@ -927,14 +921,14 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type, if (!addrinfo_wrap(he->h_addrtype, socktype, *addrlistp, he->h_length, se->s_port, &tmp_bai)) - goto addrinfo_malloc_err; + goto addrinfo_wrap_err; tmp_bai->bai_next = *res; *res = tmp_bai; continue; - addrinfo_malloc_err: + addrinfo_wrap_err: BIO_ADDRINFO_free(*res); *res = NULL; - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_BIO, ERR_R_BIO_LIB); ret = 0; goto err; } diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c index b2b5620958..a0a83da8cb 100644 --- a/crypto/bio/bio_lib.c +++ b/crypto/bio/bio_lib.c @@ -82,10 +82,8 @@ BIO *BIO_new_ex(OSSL_LIB_CTX *libctx, const BIO_METHOD *method) { BIO *bio = OPENSSL_zalloc(sizeof(*bio)); - if (bio == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (bio == NULL) return NULL; - } bio->libctx = libctx; bio->method = method; @@ -97,7 +95,7 @@ BIO *BIO_new_ex(OSSL_LIB_CTX *libctx, const BIO_METHOD *method) bio->lock = CRYPTO_THREAD_lock_new(); if (bio->lock == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_BIO, ERR_R_CRYPTO_LIB); CRYPTO_free_ex_data(CRYPTO_EX_INDEX_BIO, bio, &bio->ex_data); goto err; } diff --git a/crypto/bio/bio_meth.c b/crypto/bio/bio_meth.c index 3865d82f0e..d7d480ea4f 100644 --- a/crypto/bio/bio_meth.c +++ b/crypto/bio/bio_meth.c @@ -25,7 +25,8 @@ int BIO_get_new_index(void) int newval; if (!RUN_ONCE(&bio_type_init, do_bio_type_init)) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + /* Perhaps the error should be raised in do_bio_type_init()? */ + ERR_raise(ERR_LIB_BIO, ERR_R_CRYPTO_LIB); return -1; } if (!CRYPTO_UP_REF(&bio_count, &newval, bio_type_lock)) @@ -40,7 +41,6 @@ BIO_METHOD *BIO_meth_new(int type, const char *name) if (biom == NULL || (biom->name = OPENSSL_strdup(name)) == NULL) { OPENSSL_free(biom); - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); return NULL; } biom->type = type; diff --git a/crypto/bio/bio_print.c b/crypto/bio/bio_print.c index 4c9c3af7cf..f573514f1f 100644 --- a/crypto/bio/bio_print.c +++ b/crypto/bio/bio_print.c @@ -847,10 +847,8 @@ doapr_outch(char **sbuffer, *maxlen += BUFFER_INC; if (*buffer == NULL) { - if ((*buffer = OPENSSL_malloc(*maxlen)) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((*buffer = OPENSSL_malloc(*maxlen)) == NULL) return 0; - } if (*currlen > 0) { if (!ossl_assert(*sbuffer != NULL)) return 0; @@ -861,10 +859,8 @@ doapr_outch(char **sbuffer, char *tmpbuf; tmpbuf = OPENSSL_realloc(*buffer, *maxlen); - if (tmpbuf == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (tmpbuf == NULL) return 0; - } *buffer = tmpbuf; } } diff --git a/crypto/bio/bio_sock.c b/crypto/bio/bio_sock.c index 48a9e7d5e6..7aa7bdc65e 100644 --- a/crypto/bio/bio_sock.c +++ b/crypto/bio/bio_sock.c @@ -305,13 +305,14 @@ int BIO_accept(int sock, char **ip_port) if (ip_port != NULL) { char *host = BIO_ADDR_hostname_string(&res, 1); char *port = BIO_ADDR_service_string(&res, 1); - if (host != NULL && port != NULL) + if (host != NULL && port != NULL) { *ip_port = OPENSSL_zalloc(strlen(host) + strlen(port) + 2); - else + } else { *ip_port = NULL; + ERR_raise(ERR_LIB_BIO, ERR_R_BIO_LIB); + } if (*ip_port == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); BIO_closesocket(ret); ret = (int)INVALID_SOCKET; } else { diff --git a/crypto/bio/bss_acpt.c b/crypto/bio/bss_acpt.c index f31e8be3e0..806186ae41 100644 --- a/crypto/bio/bss_acpt.c +++ b/crypto/bio/bss_acpt.c @@ -92,10 +92,8 @@ static BIO_ACCEPT *BIO_ACCEPT_new(void) { BIO_ACCEPT *ret; - if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL) return NULL; - } ret->accept_family = BIO_FAMILY_IPANY; ret->accept_sock = (int)INVALID_SOCKET; return ret; diff --git a/crypto/bio/bss_bio.c b/crypto/bio/bss_bio.c index 0bdff21eea..3af3b27ea5 100644 --- a/crypto/bio/bss_bio.c +++ b/crypto/bio/bss_bio.c @@ -620,20 +620,16 @@ static int bio_make_pair(BIO *bio1, BIO *bio2) if (b1->buf == NULL) { b1->buf = OPENSSL_malloc(b1->size); - if (b1->buf == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (b1->buf == NULL) return 0; - } b1->len = 0; b1->offset = 0; } if (b2->buf == NULL) { b2->buf = OPENSSL_malloc(b2->size); - if (b2->buf == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (b2->buf == NULL) return 0; - } b2->len = 0; b2->offset = 0; } diff --git a/crypto/bio/bss_conn.c b/crypto/bio/bss_conn.c index 13ac7f2d95..75bfe64566 100644 --- a/crypto/bio/bss_conn.c +++ b/crypto/bio/bss_conn.c @@ -256,10 +256,8 @@ BIO_CONNECT *BIO_CONNECT_new(void) { BIO_CONNECT *ret; - if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL) return NULL; - } ret->state = BIO_CONN_S_BEFORE; ret->connect_family = BIO_FAMILY_IPANY; return ret; diff --git a/crypto/bio/bss_dgram.c b/crypto/bio/bss_dgram.c index e18072cf99..97600e4cd3 100644 --- a/crypto/bio/bss_dgram.c +++ b/crypto/bio/bss_dgram.c @@ -1820,10 +1820,8 @@ static int dgram_sctp_new(BIO *bi) bi->init = 0; bi->num = 0; - if ((data = OPENSSL_zalloc(sizeof(*data))) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((data = OPENSSL_zalloc(sizeof(*data))) == NULL) return 0; - } # ifdef SCTP_PR_SCTP_NONE data->prinfo.pr_policy = SCTP_PR_SCTP_NONE; # endif @@ -2058,10 +2056,8 @@ static int dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t) (sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = OPENSSL_malloc(optlen); - if (authchunks == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (authchunks == NULL) return -1; - } memset(authchunks, 0, optlen); ii = getsockopt(b->num, IPPROTO_SCTP, SCTP_PEER_AUTH_CHUNKS, authchunks, &optlen); diff --git a/crypto/bio/bss_dgram_pair.c b/crypto/bio/bss_dgram_pair.c index fedcd66dd3..7210a9906a 100644 --- a/crypto/bio/bss_dgram_pair.c +++ b/crypto/bio/bss_dgram_pair.c @@ -286,13 +286,13 @@ static int dgram_pair_ctrl_make_bio_pair(BIO *bio1, BIO *bio2) if (b1->rbuf.len != b1->req_buf_len) if (ring_buf_init(&b1->rbuf, b1->req_buf_len) == 0) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_BIO, ERR_R_BIO_LIB); return 0; } if (b2->rbuf.len != b2->req_buf_len) if (ring_buf_init(&b2->rbuf, b2->req_buf_len) == 0) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_BIO, ERR_R_BIO_LIB); ring_buf_destroy(&b1->rbuf); return 0; } diff --git a/crypto/bio/bss_log.c b/crypto/bio/bss_log.c index 82abfd5cec..f2c84606d2 100644 --- a/crypto/bio/bss_log.c +++ b/crypto/bio/bss_log.c @@ -197,10 +197,8 @@ static int slg_write(BIO *b, const char *in, int inl) if (inl < 0) return 0; - if ((buf = OPENSSL_malloc(inl + 1)) == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if ((buf = OPENSSL_malloc(inl + 1)) == NULL) return 0; - } memcpy(buf, in, inl); buf[inl] = '\0'; diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c index a753380e64..014acf2963 100644 --- a/crypto/bio/bss_mem.c +++ b/crypto/bio/bss_mem.c @@ -326,10 +326,8 @@ static int mem_write(BIO *b, const char *in, int inl) if (bbm->use_dgrams) { struct buf_mem_dgram_st *dgram = OPENSSL_malloc(sizeof(*dgram)); - if (dgram == NULL) { - ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE); + if (dgram == NULL) goto end; - } dgram->dgram = bbm->buf->data + blen; dgram->dgramlen = inl; -- cgit v1.2.3