diff mbox series

[1/2] af_key: Use DIV_ROUND_UP() instead of open-coded equivalent

Message ID 930c4e2a88e93c6863ddb97df9ebd0fa1b32149e.1522063171.git.kevin@guarana.org
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show
Series af_key: Fix for sadb_key memcpy read overrun | expand

Commit Message

Kevin Easton March 26, 2018, 11:39 a.m. UTC 
Several places use (x + 7) / 8 to convert from a number of bits to a number
of bytes.  Replace those with DIV_ROUND_UP(x, 8) instead, for consistency
with other parts of the same file.

Signed-off-by: Kevin Easton <kevin@guarana.org>
---
 net/key/af_key.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Steffen Klassert March 28, 2018, 5:59 a.m. UTC | #1 
On Mon, Mar 26, 2018 at 07:39:16AM -0400, Kevin Easton wrote:
> Several places use (x + 7) / 8 to convert from a number of bits to a number
> of bytes.  Replace those with DIV_ROUND_UP(x, 8) instead, for consistency
> with other parts of the same file.
> 
> Signed-off-by: Kevin Easton <kevin@guarana.org>

Is this a fix or just a cleanup?
If it is just a cleanup, please resent it based on ipsec-next.
Kevin Easton March 29, 2018, 1:35 a.m. UTC | #2 
On Wed, Mar 28, 2018 at 07:59:25AM +0200, Steffen Klassert wrote:
> On Mon, Mar 26, 2018 at 07:39:16AM -0400, Kevin Easton wrote:
> > Several places use (x + 7) / 8 to convert from a number of bits to a number
> > of bytes.  Replace those with DIV_ROUND_UP(x, 8) instead, for consistency
> > with other parts of the same file.
> > 
> > Signed-off-by: Kevin Easton <kevin@guarana.org>
> 
> Is this a fix or just a cleanup?
> If it is just a cleanup, please resent it based on ipsec-next.

Hi Steffen,

This patch (1/2) is just a cleanup, but it's in preparation for patch 2/2
which is a fix and modifies some of the same lines.

    - Kevin
Steffen Klassert April 6, 2018, 4:31 a.m. UTC | #3 
On Wed, Mar 28, 2018 at 09:35:26PM -0400, Kevin Easton wrote:
> On Wed, Mar 28, 2018 at 07:59:25AM +0200, Steffen Klassert wrote:
> > On Mon, Mar 26, 2018 at 07:39:16AM -0400, Kevin Easton wrote:
> > > Several places use (x + 7) / 8 to convert from a number of bits to a number
> > > of bytes.  Replace those with DIV_ROUND_UP(x, 8) instead, for consistency
> > > with other parts of the same file.
> > > 
> > > Signed-off-by: Kevin Easton <kevin@guarana.org>
> > 
> > Is this a fix or just a cleanup?
> > If it is just a cleanup, please resent it based on ipsec-next.
> 
> Hi Steffen,
> 
> This patch (1/2) is just a cleanup, but it's in preparation for patch 2/2
> which is a fix and modifies some of the same lines.

So please resend the fix without the cleanup, otherwise we can
get conflicts when backporting the fix to the stable trees.
diff mbox series

Patch

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 7e2e718..911b68d 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -795,12 +795,12 @@  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);
+				PFKEY_ALIGN8(DIV_ROUND_UP(x->aalg->alg_key_len, 8));
 			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);
+				PFKEY_ALIGN8(DIV_ROUND_UP(x->ealg->alg_key_len, 8));
 			size += sizeof(struct sadb_key) + encrypt_key_size;
 		}
 	}
@@ -960,7 +960,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,
+			DIV_ROUND_UP(x->aalg->alg_key_len, 8));
 	}
 	/* encrypt key */
 	if (add_keys && encrypt_key_size) {
@@ -971,7 +972,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);
+			DIV_ROUND_UP(x->ealg->alg_key_len, 8));
 	}
 
 	/* sa */
@@ -1104,14 +1105,14 @@  static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
 	key = ext_hdrs[SADB_EXT_KEY_AUTH - 1];
 	if (key != NULL &&
 	    sa->sadb_sa_auth != SADB_X_AALG_NULL &&
-	    ((key->sadb_key_bits+7) / 8 == 0 ||
-	     (key->sadb_key_bits+7) / 8 > key->sadb_key_len * sizeof(uint64_t)))
+	    (DIV_ROUND_UP(key->sadb_key_bits, 8) == 0 ||
+	     DIV_ROUND_UP(key->sadb_key_bits, 8) > key->sadb_key_len * sizeof(uint64_t)))
 		return ERR_PTR(-EINVAL);
 	key = ext_hdrs[SADB_EXT_KEY_ENCRYPT-1];
 	if (key != NULL &&
 	    sa->sadb_sa_encrypt != SADB_EALG_NULL &&
-	    ((key->sadb_key_bits+7) / 8 == 0 ||
-	     (key->sadb_key_bits+7) / 8 > key->sadb_key_len * sizeof(uint64_t)))
+	    (DIV_ROUND_UP(key->sadb_key_bits, 8) == 0 ||
+	     DIV_ROUND_UP(key->sadb_key_bits, 8) > key->sadb_key_len * sizeof(uint64_t)))
 		return ERR_PTR(-EINVAL);
 
 	x = xfrm_state_alloc(net);
@@ -1168,7 +1169,7 @@  static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
 			goto out;
 		}
 		if (key)
-			keysize = (key->sadb_key_bits + 7) / 8;
+			keysize = DIV_ROUND_UP(key->sadb_key_bits, 8);
 		x->aalg = kmalloc(sizeof(*x->aalg) + keysize, GFP_KERNEL);
 		if (!x->aalg) {
 			err = -ENOMEM;
@@ -1207,7 +1208,7 @@  static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
 			}
 			key = (struct sadb_key*) ext_hdrs[SADB_EXT_KEY_ENCRYPT-1];
 			if (key)
-				keysize = (key->sadb_key_bits + 7) / 8;
+				keysize = DIV_ROUND_UP(key->sadb_key_bits, 8);
 			x->ealg = kmalloc(sizeof(*x->ealg) + keysize, GFP_KERNEL);
 			if (!x->ealg) {
 				err = -ENOMEM;