diff mbox

net/xfrm/xfrm_replay: avoid division by zero

Message ID 1358449108-67839-1-git-send-email-nickolai@csail.mit.edu
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Nickolai Zeldovich Jan. 17, 2013, 6:58 p.m. UTC
All of the xfrm_replay->advance functions in xfrm_replay.c check if
x->replay_esn->replay_window is zero (and return if so).  However,
one of them, xfrm_replay_advance_bmp(), divides by that value (in the
'%' operator) before doing the check, which can potentially trigger
a divide-by-zero exception.  Some compilers will also assume that the
earlier division means the value cannot be zero later, and thus will
eliminate the subsequent zero check as dead code.

This patch moves the division to after the check.

Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---
 net/xfrm/xfrm_replay.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

David Miller Jan. 17, 2013, 8:46 p.m. UTC | #1
From: Nickolai Zeldovich <nickolai@csail.mit.edu>
Date: Thu, 17 Jan 2013 13:58:28 -0500

> All of the xfrm_replay->advance functions in xfrm_replay.c check if
> x->replay_esn->replay_window is zero (and return if so).  However,
> one of them, xfrm_replay_advance_bmp(), divides by that value (in the
> '%' operator) before doing the check, which can potentially trigger
> a divide-by-zero exception.  Some compilers will also assume that the
> earlier division means the value cannot be zero later, and thus will
> eliminate the subsequent zero check as dead code.
> 
> This patch moves the division to after the check.
> 
> Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>

I'm assuming Steffen will take care of this patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert Jan. 18, 2013, 8:57 a.m. UTC | #2
On Thu, Jan 17, 2013 at 01:58:28PM -0500, Nickolai Zeldovich wrote:
> All of the xfrm_replay->advance functions in xfrm_replay.c check if
> x->replay_esn->replay_window is zero (and return if so).  However,
> one of them, xfrm_replay_advance_bmp(), divides by that value (in the
> '%' operator) before doing the check, which can potentially trigger
> a divide-by-zero exception.  Some compilers will also assume that the
> earlier division means the value cannot be zero later, and thus will
> eliminate the subsequent zero check as dead code.
> 
> This patch moves the division to after the check.
> 
> Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>

Applied, thanks!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 765f6fe..35754cc 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -242,11 +242,13 @@  static void xfrm_replay_advance_bmp(struct xfrm_state *x, __be32 net_seq)
 	u32 diff;
 	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
 	u32 seq = ntohl(net_seq);
-	u32 pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+	u32 pos;
 
 	if (!replay_esn->replay_window)
 		return;
 
+	pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
 	if (seq > replay_esn->seq) {
 		diff = seq - replay_esn->seq;