Changeset View
Changeset View
Standalone View
Standalone View
sys/netipsec/xform_esp.c
| Context not available. | |||||||||
| return EINVAL; | return EINVAL; | ||||||||
| } | } | ||||||||
| csp.csp_mode = CSP_MODE_AEAD; | csp.csp_mode = CSP_MODE_AEAD; | ||||||||
| } else if (sav->alg_auth != 0) | if (sav->flags & SADB_X_SAFLAGS_ESN) | ||||||||
| csp.csp_flags |= CSP_F_SEPARATE_AAD; | |||||||||
| } else if (sav->alg_auth != 0) { | |||||||||
| csp.csp_mode = CSP_MODE_ETA; | csp.csp_mode = CSP_MODE_ETA; | ||||||||
| if (sav->flags & SADB_X_SAFLAGS_ESN) | |||||||||
| csp.csp_flags |= CSP_F_ESN; | |||||||||
| } | |||||||||
| else | else | ||||||||
gnn: Push else up to } else and please check indenting in this section. | |||||||||
| csp.csp_mode = CSP_MODE_CIPHER; | csp.csp_mode = CSP_MODE_CIPHER; | ||||||||
| Context not available. | |||||||||
| crypto_session_t cryptoid; | crypto_session_t cryptoid; | ||||||||
| int alen, error, hlen, plen; | int alen, error, hlen, plen; | ||||||||
| uint32_t seqh; | uint32_t seqh; | ||||||||
| const struct crypto_session_params *csp; | |||||||||
| IPSEC_ASSERT(sav != NULL, ("null SA")); | IPSEC_ASSERT(sav != NULL, ("null SA")); | ||||||||
| IPSEC_ASSERT(sav->tdb_encalgxform != NULL, ("null encoding xform")); | IPSEC_ASSERT(sav->tdb_encalgxform != NULL, ("null encoding xform")); | ||||||||
| Context not available. | |||||||||
| error = EACCES; | error = EACCES; | ||||||||
| goto bad; | goto bad; | ||||||||
| } | } | ||||||||
| seqh = htonl(seqh); | |||||||||
| } | } | ||||||||
| cryptoid = sav->tdb_cryptoid; | cryptoid = sav->tdb_cryptoid; | ||||||||
| SECASVAR_UNLOCK(sav); | SECASVAR_UNLOCK(sav); | ||||||||
| Context not available. | |||||||||
| xd = malloc(sizeof(*xd), M_XDATA, M_NOWAIT | M_ZERO); | xd = malloc(sizeof(*xd), M_XDATA, M_NOWAIT | M_ZERO); | ||||||||
| if (xd == NULL) { | if (xd == NULL) { | ||||||||
| DPRINTF(("%s: failed to allocate xform_data\n", __func__)); | DPRINTF(("%s: failed to allocate xform_data\n", __func__)); | ||||||||
| ESPSTAT_INC(esps_crypto); | goto xd_fail; | ||||||||
| crypto_freereq(crp); | |||||||||
| error = ENOBUFS; | |||||||||
| goto bad; | |||||||||
| } | } | ||||||||
| if (esph != NULL) { | if (esph != NULL) { | ||||||||
Done Inline ActionsMoving this is fine but looks like a no-op? jhb: Moving this is fine but looks like a no-op? | |||||||||
Done Inline ActionsThis was leftover from previous approach - I've undo this change. Thank you. jaz_semihalf.com: This was leftover from previous approach - I've undo this change. Thank you. | |||||||||
| crp->crp_op = CRYPTO_OP_VERIFY_DIGEST; | crp->crp_op = CRYPTO_OP_VERIFY_DIGEST; | ||||||||
| crp->crp_aad_start = skip; | |||||||||
| if (SAV_ISGCM(sav)) | if (SAV_ISGCM(sav)) | ||||||||
| crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */ | crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */ | ||||||||
| else | else | ||||||||
| crp->crp_aad_length = hlen; | crp->crp_aad_length = hlen; | ||||||||
| csp = crypto_get_params(crp->crp_session); | |||||||||
| if (csp->csp_flags & CSP_F_SEPARATE_AAD && | |||||||||
Done Inline ActionsAdd braces to make this clearer as to which operation goes where. gnn: Add braces to make this clearer as to which operation goes where. | |||||||||
| sav->replay != NULL && sav->replay->wsize != 0) { | |||||||||
| crp->crp_aad_length += 4; | |||||||||
Done Inline ActionsConvert magic number (4) to a #define or const. gnn: Convert magic number (4) to a #define or const. | |||||||||
| crp->crp_aad = malloc(crp->crp_aad_length, M_XDATA, M_NOWAIT); | |||||||||
| if (crp->crp_aad == NULL) { | |||||||||
| DPRINTF(("%s: failed to allocate xform_data\n", | |||||||||
| __func__)); | |||||||||
| goto crp_aad_fail; | |||||||||
| } | |||||||||
| /* SPI */ | |||||||||
| m_copydata(m, skip, 4, crp->crp_aad); | |||||||||
| /* ESN */ | |||||||||
Not Done Inline ActionsMinor nit: blank line before here. jhb: Minor nit: blank line before here. | |||||||||
| bcopy(&seqh, (char *)crp->crp_aad + 4, 4); | |||||||||
| /* Rest of aad */ | |||||||||
| if (crp->crp_aad_length - 8 > 0) | |||||||||
| m_copydata(m, skip + 4, crp->crp_aad_length - 4, | |||||||||
| (char *)crp->crp_aad + 8); | |||||||||
| } else | |||||||||
| crp->crp_aad_start = skip; | |||||||||
| if (csp->csp_flags & CSP_F_ESN && | |||||||||
| sav->replay != NULL && sav->replay->wsize != 0) | |||||||||
| memcpy(crp->crp_esn, &seqh, 4); | |||||||||
| crp->crp_digest_start = m->m_pkthdr.len - alen; | crp->crp_digest_start = m->m_pkthdr.len - alen; | ||||||||
| } | } | ||||||||
| Context not available. | |||||||||
| crp->crp_iv_start = skip + hlen - sav->ivlen; | crp->crp_iv_start = skip + hlen - sav->ivlen; | ||||||||
| return (crypto_dispatch(crp)); | return (crypto_dispatch(crp)); | ||||||||
| crp_aad_fail: | |||||||||
| free(xd, M_XDATA); | |||||||||
| xd_fail: | |||||||||
| crypto_freereq(crp); | |||||||||
| ESPSTAT_INC(esps_crypto); | |||||||||
| error = ENOBUFS; | |||||||||
| bad: | bad: | ||||||||
| m_freem(m); | m_freem(m); | ||||||||
| key_freesav(&sav); | key_freesav(&sav); | ||||||||
| Context not available. | |||||||||
| /* Release the crypto descriptors */ | /* Release the crypto descriptors */ | ||||||||
| free(xd, M_XDATA), xd = NULL; | free(xd, M_XDATA), xd = NULL; | ||||||||
| free(crp->crp_aad, M_XDATA), crp->crp_aad = NULL; | |||||||||
| crypto_freereq(crp), crp = NULL; | crypto_freereq(crp), crp = NULL; | ||||||||
| /* | /* | ||||||||
| Context not available. | |||||||||
| m_freem(m); | m_freem(m); | ||||||||
| if (xd != NULL) | if (xd != NULL) | ||||||||
| free(xd, M_XDATA); | free(xd, M_XDATA); | ||||||||
| if (crp != NULL) | if (crp != NULL) { | ||||||||
| free(crp->crp_aad, M_XDATA); | |||||||||
| crypto_freereq(crp); | crypto_freereq(crp); | ||||||||
| } | |||||||||
Done Inline Actions
Alternatively, you could move the free(crp->crp_aad, M_XDATA) under the existing if (crp != NULL) below since free(NULL, ..) is defined to be a nop. jhb: Alternatively, you could move the `free(crp->crp_aad, M_XDATA)` under the existing `if (crp !=… | |||||||||
| return error; | return error; | ||||||||
| } | } | ||||||||
| /* | /* | ||||||||
| Context not available. | |||||||||
| int hlen, rlen, padding, blks, alen, i, roff; | int hlen, rlen, padding, blks, alen, i, roff; | ||||||||
| int error, maxpacketsize; | int error, maxpacketsize; | ||||||||
| uint8_t prot; | uint8_t prot; | ||||||||
| uint32_t seqh; | |||||||||
| const struct crypto_session_params *csp; | |||||||||
| IPSEC_ASSERT(sav != NULL, ("null SA")); | IPSEC_ASSERT(sav != NULL, ("null SA")); | ||||||||
| esph = sav->tdb_authalgxform; | esph = sav->tdb_authalgxform; | ||||||||
| Context not available. | |||||||||
| bcopy((caddr_t) &replay, mtod(mo, caddr_t) + roff + | bcopy((caddr_t) &replay, mtod(mo, caddr_t) + roff + | ||||||||
| sizeof(uint32_t), sizeof(uint32_t)); | sizeof(uint32_t), sizeof(uint32_t)); | ||||||||
| seqh = htonl((uint32_t)(sav->replay->count >> 32)); | |||||||||
Done Inline Actions#dedfine for magic number gnn: #dedfine for magic number | |||||||||
| } | } | ||||||||
| cryptoid = sav->tdb_cryptoid; | cryptoid = sav->tdb_cryptoid; | ||||||||
| if (SAV_ISCTRORGCM(sav)) | if (SAV_ISCTRORGCM(sav)) | ||||||||
| Context not available. | |||||||||
| } | } | ||||||||
| /* IPsec-specific opaque crypto info. */ | /* IPsec-specific opaque crypto info. */ | ||||||||
| xd = malloc(sizeof(struct xform_data), M_XDATA, M_NOWAIT | M_ZERO); | xd = malloc(sizeof(struct xform_data), M_XDATA, M_NOWAIT | M_ZERO); | ||||||||
| if (xd == NULL) { | if (xd == NULL) { | ||||||||
| crypto_freereq(crp); | |||||||||
| DPRINTF(("%s: failed to allocate xform_data\n", __func__)); | DPRINTF(("%s: failed to allocate xform_data\n", __func__)); | ||||||||
| ESPSTAT_INC(esps_crypto); | goto xd_fail; | ||||||||
| error = ENOBUFS; | |||||||||
| goto bad; | |||||||||
| } | } | ||||||||
| /* Encryption descriptor. */ | /* Encryption descriptor. */ | ||||||||
| Context not available. | |||||||||
| if (esph) { | if (esph) { | ||||||||
| /* Authentication descriptor. */ | /* Authentication descriptor. */ | ||||||||
| crp->crp_op |= CRYPTO_OP_COMPUTE_DIGEST; | crp->crp_op |= CRYPTO_OP_COMPUTE_DIGEST; | ||||||||
| crp->crp_aad_start = skip; | |||||||||
| if (SAV_ISGCM(sav)) | if (SAV_ISGCM(sav)) | ||||||||
| crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */ | crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */ | ||||||||
| else | else | ||||||||
| crp->crp_aad_length = hlen; | crp->crp_aad_length = hlen; | ||||||||
| csp = crypto_get_params(crp->crp_session); | |||||||||
| if (csp->csp_flags & CSP_F_SEPARATE_AAD && | |||||||||
| sav->replay != NULL) { | |||||||||
| crp->crp_aad_length += 4; | |||||||||
| crp->crp_aad = malloc(crp->crp_aad_length, M_XDATA, M_NOWAIT); | |||||||||
| if (crp->crp_aad == NULL) { | |||||||||
| DPRINTF(("%s: failed to allocate xform_data\n", | |||||||||
| __func__)); | |||||||||
| goto crp_aad_fail; | |||||||||
| } | |||||||||
| /* SPI */ | |||||||||
| m_copydata(m, skip, 4, crp->crp_aad); | |||||||||
| /* ESN */ | |||||||||
| bcopy(&seqh, (char *)crp->crp_aad + 4, 4); | |||||||||
| /* Rest of aad */ | |||||||||
| if (crp->crp_aad_length - 8 > 0) | |||||||||
| m_copydata(m, skip + 4, crp->crp_aad_length - 4, | |||||||||
Not Done Inline ActionsHere as well. jhb: Here as well. | |||||||||
| (char *)crp->crp_aad + 8); | |||||||||
| } else | |||||||||
| crp->crp_aad_start = skip; | |||||||||
| if (csp->csp_flags & CSP_F_ESN && sav->replay != NULL) | |||||||||
| memcpy(crp->crp_esn, &seqh, 4); | |||||||||
| crp->crp_digest_start = m->m_pkthdr.len - alen; | crp->crp_digest_start = m->m_pkthdr.len - alen; | ||||||||
| } | } | ||||||||
| return crypto_dispatch(crp); | return crypto_dispatch(crp); | ||||||||
| crp_aad_fail: | |||||||||
| free(xd, M_XDATA); | |||||||||
| xd_fail: | |||||||||
| crypto_freereq(crp); | |||||||||
| ESPSTAT_INC(esps_crypto); | |||||||||
| error = ENOBUFS; | |||||||||
| bad: | bad: | ||||||||
| if (m) | if (m) | ||||||||
| m_freem(m); | m_freem(m); | ||||||||
| Context not available. | |||||||||
| goto bad; | goto bad; | ||||||||
| } | } | ||||||||
| free(xd, M_XDATA); | free(xd, M_XDATA); | ||||||||
| free(crp->crp_aad, M_XDATA); | |||||||||
| crypto_freereq(crp); | crypto_freereq(crp); | ||||||||
| ESPSTAT_INC(esps_hist[sav->alg_enc]); | ESPSTAT_INC(esps_hist[sav->alg_enc]); | ||||||||
| if (sav->tdb_authalgxform != NULL) | if (sav->tdb_authalgxform != NULL) | ||||||||
Done Inline ActionsAs below, you can just call free() unconditionally. There is also no need to clear crp_aad. jhb: As below, you can just call free() unconditionally. There is also no need to clear crp_aad. | |||||||||
| Context not available. | |||||||||
| bad: | bad: | ||||||||
| CURVNET_RESTORE(); | CURVNET_RESTORE(); | ||||||||
| free(xd, M_XDATA); | free(xd, M_XDATA); | ||||||||
| free(crp->crp_aad, M_XDATA); | |||||||||
| crypto_freereq(crp); | crypto_freereq(crp); | ||||||||
| key_freesav(&sav); | key_freesav(&sav); | ||||||||
| key_freesp(&sp); | key_freesp(&sp); | ||||||||
| Context not available. | |||||||||
Done Inline ActionsThis can just be unconditional like the rest of this block since free(NULL) is ok. jhb: This can just be unconditional like the rest of this block since free(NULL) is ok. | |||||||||
Push else up to } else and please check indenting in this section.