On Tue, Dec 17, 2024 at 03:32:31PM +0300, Dan Carpenter wrote:
>
> That seems like basic algebra but we have a long history of getting
> integer overflow checks wrong so these days I like to just use
> INT_MAX where ever I can. I wanted to use USHRT_MAX. We aren't allowed
> to use more than USHRT_MAX bytes, but maybe we're allowed USHRT_MAX
> bits, so I didn't do that.
There is no reason for this to overflow if we rewrite it do do
the division carefully. Something like this:
Steffen, this raises a new question: Can normal users create socket
policies of arbtirarily long key lengths? If so we probably should
look into limiting the key length to a sane value. Of course, given
namespaces we probably should do that in any case.
---8<---
Rewrite the IPsec conversion from bit-length to byte-length so that
large values do not overflow.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
index c7338ac6a5bb..eb7bb196d75d 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
@@ -165,7 +165,7 @@ static int ch_ipsec_setauthsize(struct xfrm_state *x,
static int ch_ipsec_setkey(struct xfrm_state *x,
struct ipsec_sa_entry *sa_entry)
{
- int keylen = (x->aead->alg_key_len + 7) / 8;
+ int keylen = xfrm_kblen2klen(x->aead->alg_key_len);
unsigned char *key = x->aead->alg_key;
int ck_size, key_ctx_size = 0;
unsigned char ghash_h[AEAD_H_SIZE];
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index ca92e518be76..24c96a4497b2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -323,7 +323,7 @@ void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
memset(attrs, 0, sizeof(*attrs));
/* key */
- crypto_data_len = (x->aead->alg_key_len + 7) / 8;
+ crypto_data_len = xfrm_kblen2klen(x->aead->alg_key_len);
key_len = crypto_data_len - 4; /* 4 bytes salt at end */
memcpy(aes_gcm->aes_key, x->aead->alg_key, key_len);
diff --git a/drivers/net/ethernet/netronome/nfp/crypto/ipsec.c b/drivers/net/ethernet/netronome/nfp/crypto/ipsec.c
index 515069d5637b..2660236eb07f 100644
--- a/drivers/net/ethernet/netronome/nfp/crypto/ipsec.c
+++ b/drivers/net/ethernet/netronome/nfp/crypto/ipsec.c
@@ -365,7 +365,7 @@ static int nfp_net_xfrm_add_state(struct xfrm_state *x,
}
if (x->aalg) {
- key_len = DIV_ROUND_UP(x->aalg->alg_key_len, BITS_PER_BYTE);
+ key_len = xfrm_kblen2klen(x->aalg->alg_key_len);
if (key_len > sizeof(cfg->auth_key)) {
NL_SET_ERR_MSG_MOD(extack, "Insufficient space for offloaded auth key");
return -EINVAL;
@@ -458,7 +458,7 @@ static int nfp_net_xfrm_add_state(struct xfrm_state *x,
int key_offset = 0;
int salt_len = 4;
- key_len = DIV_ROUND_UP(x->aead->alg_key_len, BITS_PER_BYTE);
+ key_len = xfrm_kblen2klen(x->aead->alg_key_len);
key_len -= salt_len;
if (key_len > sizeof(cfg->ciph_key)) {
@@ -485,7 +485,7 @@ static int nfp_net_xfrm_add_state(struct xfrm_state *x,
}
if (x->ealg) {
- key_len = DIV_ROUND_UP(x->ealg->alg_key_len, BITS_PER_BYTE);
+ key_len = xfrm_kblen2klen(x->ealg->alg_key_len);
if (key_len > sizeof(cfg->ciph_key)) {
NL_SET_ERR_MSG_MOD(extack, "ealg: Insufficient space for offloaded key");
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 32c09e85a64c..1ce897a9476b 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1912,19 +1912,24 @@ static inline int xfrm_acquire_is_on(struct net *net)
}
#endif
+static inline unsigned int xfrm_kblen2klen(unsigned int klen_in_bits)
+{
+ return klen_in_bits / 8 + !!(klen_in_bits & 7);
+}
+
static inline unsigned int aead_len(struct xfrm_algo_aead *alg)
{
- return sizeof(*alg) + ((alg->alg_key_len + 7) / 8);
+ return sizeof(*alg) + xfrm_kblen2klen(alg->alg_key_len);
}
static inline unsigned int xfrm_alg_len(const struct xfrm_algo *alg)
{
- return sizeof(*alg) + ((alg->alg_key_len + 7) / 8);
+ return sizeof(*alg) + xfrm_kblen2klen(alg->alg_key_len);
}
static inline unsigned int xfrm_alg_auth_len(const struct xfrm_algo_auth *alg)
{
- return sizeof(*alg) + ((alg->alg_key_len + 7) / 8);
+ return sizeof(*alg) + xfrm_kblen2klen(alg->alg_key_len);
}
static inline unsigned int xfrm_replay_state_esn_len(struct xfrm_replay_state_esn *replay_esn)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 64aec3dff8ec..efea16f28b8d 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -496,7 +496,7 @@ static int ah_init_state(struct xfrm_state *x, struct netlink_ext_ack *extack)
ahp->ahash = ahash;
if (crypto_ahash_setkey(ahash, x->aalg->alg_key,
- (x->aalg->alg_key_len + 7) / 8)) {
+ xfrm_kblen2klen(x->aalg->alg_key_len))) {
NL_SET_ERR_MSG(extack, "Kernel was unable to initialize cryptographic operations");
goto error;
}
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index f3281312eb5e..6dcbaf75c8b6 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -1028,7 +1028,7 @@ static int esp_init_aead(struct xfrm_state *x, struct netlink_ext_ack *extack)
x->data = aead;
err = crypto_aead_setkey(aead, x->aead->alg_key,
- (x->aead->alg_key_len + 7) / 8);
+ xfrm_kblen2klen(x->aead->alg_key_len));
if (err)
goto error;
@@ -1088,8 +1088,9 @@ static int esp_init_authenc(struct xfrm_state *x,
x->data = aead;
- keylen = (x->aalg ? (x->aalg->alg_key_len + 7) / 8 : 0) +
- (x->ealg->alg_key_len + 7) / 8 + RTA_SPACE(sizeof(*param));
+ keylen = x->aalg ? xfrm_kblen2klen(x->aalg->alg_key_len) : 0;
+ keylen += xfrm_kblen2klen(x->ealg->alg_key_len);
+ keylen += RTA_SPACE(sizeof(*param));
err = -ENOMEM;
key = kmalloc(keylen, GFP_KERNEL);
if (!key)
@@ -1105,8 +1106,8 @@ static int esp_init_authenc(struct xfrm_state *x,
if (x->aalg) {
struct xfrm_algo_desc *aalg_desc;
- memcpy(p, x->aalg->alg_key, (x->aalg->alg_key_len + 7) / 8);
- p += (x->aalg->alg_key_len + 7) / 8;
+ memcpy(p, x->aalg->alg_key, xfrm_kblen2klen(x->aalg->alg_key_len));
+ p += xfrm_kblen2klen(x->aalg->alg_key_len);
aalg_desc = xfrm_aalg_get_byname(x->aalg->alg_name, 0);
BUG_ON(!aalg_desc);
@@ -1126,8 +1127,8 @@ static int esp_init_authenc(struct xfrm_state *x,
}
}
- param->enckeylen = cpu_to_be32((x->ealg->alg_key_len + 7) / 8);
- memcpy(p, x->ealg->alg_key, (x->ealg->alg_key_len + 7) / 8);
+ param->enckeylen = cpu_to_be32(xfrm_kblen2klen(x->ealg->alg_key_len));
+ memcpy(p, x->ealg->alg_key, xfrm_kblen2klen(x->ealg->alg_key_len));
err = crypto_aead_setkey(aead, key, keylen);
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index eb474f0987ae..edf46ba35823 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -691,7 +691,7 @@ static int ah6_init_state(struct xfrm_state *x, struct netlink_ext_ack *extack)
ahp->ahash = ahash;
if (crypto_ahash_setkey(ahash, x->aalg->alg_key,
- (x->aalg->alg_key_len + 7) / 8)) {
+ xfrm_kblen2klen(x->aalg->alg_key_len))) {
NL_SET_ERR_MSG(extack, "Kernel was unable to initialize cryptographic operations");
goto error;
}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b2400c226a32..1cd8c4f71d33 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -1065,7 +1065,7 @@ static int esp_init_aead(struct xfrm_state *x, struct netlink_ext_ack *extack)
x->data = aead;
err = crypto_aead_setkey(aead, x->aead->alg_key,
- (x->aead->alg_key_len + 7) / 8);
+ xfrm_kblen2klen(x->aead->alg_key_len));
if (err)
goto error;
@@ -1125,8 +1125,9 @@ static int esp_init_authenc(struct xfrm_state *x,
x->data = aead;
- keylen = (x->aalg ? (x->aalg->alg_key_len + 7) / 8 : 0) +
- (x->ealg->alg_key_len + 7) / 8 + RTA_SPACE(sizeof(*param));
+ keylen = (x->aalg ? xfrm_kblen2klen(x->aalg->alg_key_len) : 0);
+ keylen += xfrm_kblen2klen(x->ealg->alg_key_len);
+ keylen += RTA_SPACE(sizeof(*param));
err = -ENOMEM;
key = kmalloc(keylen, GFP_KERNEL);
if (!key)
@@ -1142,8 +1143,8 @@ static int esp_init_authenc(struct xfrm_state *x,
if (x->aalg) {
struct xfrm_algo_desc *aalg_desc;
- memcpy(p, x->aalg->alg_key, (x->aalg->alg_key_len + 7) / 8);
- p += (x->aalg->alg_key_len + 7) / 8;
+ memcpy(p, x->aalg->alg_key, xfrm_kblen2klen(x->aalg->alg_key_len));
+ p += xfrm_kblen2klen(x->aalg->alg_key_len);
aalg_desc = xfrm_aalg_get_byname(x->aalg->alg_name, 0);
BUG_ON(!aalg_desc);
@@ -1163,8 +1164,8 @@ static int esp_init_authenc(struct xfrm_state *x,
}
}
- param->enckeylen = cpu_to_be32((x->ealg->alg_key_len + 7) / 8);
- memcpy(p, x->ealg->alg_key, (x->ealg->alg_key_len + 7) / 8);
+ param->enckeylen = cpu_to_be32(xfrm_kblen2klen(x->ealg->alg_key_len));
+ memcpy(p, x->ealg->alg_key, xfrm_kblen2klen(x->ealg->alg_key_len));
err = crypto_aead_setkey(aead, key, keylen);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index c56bb4f451e6..2d0b11780263 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -803,13 +803,11 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
if (add_keys) {
if (x->aalg && x->aalg->alg_key_len) {
- auth_key_size =
- PFKEY_ALIGN8((x->aalg->alg_key_len + 7) / 8);
+ auth_key_size = PFKEY_ALIGN8(xfrm_kblen2klen(x->aalg->alg_key_len));
size += sizeof(struct sadb_key) + auth_key_size;
}
if (x->ealg && x->ealg->alg_key_len) {
- encrypt_key_size =
- PFKEY_ALIGN8((x->ealg->alg_key_len+7) / 8);
+ encrypt_key_size = PFKEY_ALIGN8(xfrm_kblen2klen(x->ealg->alg_key_len));
size += sizeof(struct sadb_key) + encrypt_key_size;
}
}
@@ -967,7 +965,8 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
key->sadb_key_exttype = SADB_EXT_KEY_AUTH;
key->sadb_key_bits = x->aalg->alg_key_len;
key->sadb_key_reserved = 0;
- memcpy(key + 1, x->aalg->alg_key, (x->aalg->alg_key_len+7)/8);
+ memcpy(key + 1, x->aalg->alg_key,
+ xfrm_kblen2klen(x->aalg->alg_key_len));
}
/* encrypt key */
if (add_keys && encrypt_key_size) {
@@ -978,7 +977,7 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
key->sadb_key_bits = x->ealg->alg_key_len;
key->sadb_key_reserved = 0;
memcpy(key + 1, x->ealg->alg_key,
- (x->ealg->alg_key_len+7)/8);
+ xfrm_kblen2klen(x->ealg->alg_key_len));
}
/* sa */
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b2876e09328b..8d98bf7c7ad1 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -532,6 +532,7 @@ static int attach_auth(struct xfrm_algo_auth **algpp, u8 *props,
struct xfrm_algo *ualg;
struct xfrm_algo_auth *p;
struct xfrm_algo_desc *algo;
+ unsigned int klen;
if (!rta)
return 0;
@@ -545,14 +546,15 @@ static int attach_auth(struct xfrm_algo_auth **algpp, u8 *props,
}
*props = algo->desc.sadb_alg_id;
- p = kmalloc(sizeof(*p) + (ualg->alg_key_len + 7) / 8, GFP_KERNEL);
+ klen = xfrm_kblen2klen(ualg->alg_key_len);
+ p = kmalloc(sizeof(*p) + klen, GFP_KERNEL);
if (!p)
return -ENOMEM;
strcpy(p->alg_name, algo->name);
p->alg_key_len = ualg->alg_key_len;
p->alg_trunc_len = algo->uinfo.auth.icv_truncbits;
- memcpy(p->alg_key, ualg->alg_key, (ualg->alg_key_len + 7) / 8);
+ memcpy(p->alg_key, ualg->alg_key, klen);
*algpp = p;
return 0;
@@ -1089,23 +1091,22 @@ static bool xfrm_redact(void)
static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
{
+ unsigned int klen = xfrm_kblen2klen(auth->alg_key_len);
struct xfrm_algo *algo;
struct xfrm_algo_auth *ap;
struct nlattr *nla;
bool redact_secret = xfrm_redact();
- nla = nla_reserve(skb, XFRMA_ALG_AUTH,
- sizeof(*algo) + (auth->alg_key_len + 7) / 8);
+ nla = nla_reserve(skb, XFRMA_ALG_AUTH, sizeof(*algo) + klen);
if (!nla)
return -EMSGSIZE;
algo = nla_data(nla);
strscpy_pad(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
- if (redact_secret && auth->alg_key_len)
- memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+ if (redact_secret)
+ memset(algo->alg_key, 0, klen);
else
- memcpy(algo->alg_key, auth->alg_key,
- (auth->alg_key_len + 7) / 8);
+ memcpy(algo->alg_key, auth->alg_key, klen);
algo->alg_key_len = auth->alg_key_len;
nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
@@ -1115,16 +1116,16 @@ static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
strscpy_pad(ap->alg_name, auth->alg_name, sizeof(ap->alg_name));
ap->alg_key_len = auth->alg_key_len;
ap->alg_trunc_len = auth->alg_trunc_len;
- if (redact_secret && auth->alg_key_len)
- memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+ if (redact_secret)
+ memset(ap->alg_key, 0, klen);
else
- memcpy(ap->alg_key, auth->alg_key,
- (auth->alg_key_len + 7) / 8);
+ memcpy(ap->alg_key, auth->alg_key, klen);
return 0;
}
static int copy_to_user_aead(struct xfrm_algo_aead *aead, struct sk_buff *skb)
{
+ unsigned int klen = xfrm_kblen2klen(aead->alg_key_len);
struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
struct xfrm_algo_aead *ap;
bool redact_secret = xfrm_redact();
@@ -1137,16 +1138,16 @@ static int copy_to_user_aead(struct xfrm_algo_aead *aead, struct sk_buff *skb)
ap->alg_key_len = aead->alg_key_len;
ap->alg_icv_len = aead->alg_icv_len;
- if (redact_secret && aead->alg_key_len)
- memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+ if (redact_secret)
+ memset(ap->alg_key, 0, klen);
else
- memcpy(ap->alg_key, aead->alg_key,
- (aead->alg_key_len + 7) / 8);
+ memcpy(ap->alg_key, aead->alg_key, klen);
return 0;
}
static int copy_to_user_ealg(struct xfrm_algo *ealg, struct sk_buff *skb)
{
+ unsigned int klen = xfrm_kblen2klen(ealg->alg_key_len);
struct xfrm_algo *ap;
bool redact_secret = xfrm_redact();
struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
@@ -1158,11 +1159,10 @@ static int copy_to_user_ealg(struct xfrm_algo *ealg, struct sk_buff *skb)
strscpy_pad(ap->alg_name, ealg->alg_name, sizeof(ap->alg_name));
ap->alg_key_len = ealg->alg_key_len;
- if (redact_secret && ealg->alg_key_len)
- memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+ if (redact_secret)
+ memset(ap->alg_key, 0, klen);
else
- memcpy(ap->alg_key, ealg->alg_key,
- (ealg->alg_key_len + 7) / 8);
+ memcpy(ap->alg_key, ealg->alg_key, klen);
return 0;
}
@@ -3509,7 +3509,7 @@ static inline unsigned int xfrm_sa_len(struct xfrm_state *x)
l += nla_total_size(aead_len(x->aead));
if (x->aalg) {
l += nla_total_size(sizeof(struct xfrm_algo) +
- (x->aalg->alg_key_len + 7) / 8);
+ xfrm_kblen2klen(x->aalg->alg_key_len));
l += nla_total_size(xfrm_alg_auth_len(x->aalg));
}
if (x->ealg)
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
