Re: [PATCH] crypto_user: Fix out-of-bounds read

2014-04-23 Thread Dan Carpenter
On Tue, Apr 22, 2014 at 12:30:28PM -0700, Andy Lutomirski wrote:
 This is unlikely to be exploitable for anything except an OOPS.
 
 Cc: sta...@vger.kernel.org
 Signed-off-by: Andy Lutomirski l...@amacapital.net
 ---
 
 Notes:
 This is entirely untested, but it looks obviously correct to me.
 
  crypto/crypto_user.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
 index 1512e41..bc7c4b5 100644
 --- a/crypto/crypto_user.c
 +++ b/crypto/crypto_user.c
 @@ -460,7 +460,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, 
 struct nlmsghdr *nlh)
   int type, err;
  
   type = nlh-nlmsg_type;
 - if (type  CRYPTO_MSG_MAX)
 + if (type  CRYPTO_MSG_BASE || type  CRYPTO_MSG_MAX)
   return -EINVAL;

Adding a check here is obviously harmless but I think this is only
called from netlink_rcv_skb() which already checks:

if (nlh-nlmsg_type  NLMSG_MIN_TYPE)
goto ack;

NLMSG_MIN_TYPE is 0x10 as well, so I don't think we can hit your
condition.

Your patch freaked me out a little because this is one of the bugs that
I should have caught throught static analysis.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] crypto_user: Fix out-of-bounds read

2014-04-23 Thread Andy Lutomirski
On Apr 23, 2014 4:40 AM, Dan Carpenter dan.carpen...@oracle.com wrote:

 On Tue, Apr 22, 2014 at 12:30:28PM -0700, Andy Lutomirski wrote:
  This is unlikely to be exploitable for anything except an OOPS.
 
  Cc: sta...@vger.kernel.org
  Signed-off-by: Andy Lutomirski l...@amacapital.net
  ---
 
  Notes:
  This is entirely untested, but it looks obviously correct to me.
 
   crypto/crypto_user.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
  index 1512e41..bc7c4b5 100644
  --- a/crypto/crypto_user.c
  +++ b/crypto/crypto_user.c
  @@ -460,7 +460,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, 
  struct nlmsghdr *nlh)
int type, err;
 
type = nlh-nlmsg_type;
  - if (type  CRYPTO_MSG_MAX)
  + if (type  CRYPTO_MSG_BASE || type  CRYPTO_MSG_MAX)
return -EINVAL;

 Adding a check here is obviously harmless but I think this is only
 called from netlink_rcv_skb() which already checks:

 if (nlh-nlmsg_type  NLMSG_MIN_TYPE)
 goto ack;

 NLMSG_MIN_TYPE is 0x10 as well, so I don't think we can hit your
 condition.

 Your patch freaked me out a little because this is one of the bugs that
 I should have caught throught static analysis.

BUILD_BUG_ON(NLMSG_MIN_TYPE != CRYPTO_MSG_BASE) might be a better
thing to add, then.

I don't feel so bad for missing this.


 regards,
 dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] crypto_user: Fix out-of-bounds read

2014-04-22 Thread Andy Lutomirski
This is unlikely to be exploitable for anything except an OOPS.

Cc: sta...@vger.kernel.org
Signed-off-by: Andy Lutomirski l...@amacapital.net
---

Notes:
This is entirely untested, but it looks obviously correct to me.

 crypto/crypto_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 1512e41..bc7c4b5 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -460,7 +460,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh)
int type, err;
 
type = nlh-nlmsg_type;
-   if (type  CRYPTO_MSG_MAX)
+   if (type  CRYPTO_MSG_BASE || type  CRYPTO_MSG_MAX)
return -EINVAL;
 
type -= CRYPTO_MSG_BASE;
-- 
1.9.0

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html