diff mbox

[v2] phonet: some signedness bugs

Message ID 20110110140658.GB2721@bicker
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Dan Carpenter Jan. 10, 2011, 2:06 p.m. UTC
Dan Rosenberg pointed out that there were some signed comparison bugs
in the phonet protocol.

http://marc.info/?l=full-disclosure&m=129424528425330&w=2

The problem is that we check for array overflows but "protocol" is
signed and we don't check for array underflows.  If you have already
have CAP_SYS_ADMIN then you could use the bugs to get root, or someone
could cause an oops by mistake.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
v2:  in v1 I changed pn_socket_create() but that change caused a
compiler warning.  That part of the patch wasn't needed so I've just
dropped.

--
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

Comments

Rémi Denis-Courmont Jan. 10, 2011, 2:12 p.m. UTC | #1
On Monday 10 January 2011 16:06:58 ext Dan Carpenter, you wrote:
> Dan Rosenberg pointed out that there were some signed comparison bugs
> in the phonet protocol.
> 
> http://marc.info/?l=full-disclosure&m=129424528425330&w=2
> 
> The problem is that we check for array overflows but "protocol" is
> signed and we don't check for array underflows.  If you have already
> have CAP_SYS_ADMIN then you could use the bugs to get root, or someone
> could cause an oops by mistake.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Acked-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
David Miller Jan. 11, 2011, 12:06 a.m. UTC | #2
From: Dan Carpenter <error27@gmail.com>
Date: Mon, 10 Jan 2011 17:06:58 +0300

> Dan Rosenberg pointed out that there were some signed comparison bugs
> in the phonet protocol.
> 
> http://marc.info/?l=full-disclosure&m=129424528425330&w=2
> 
> The problem is that we check for array overflows but "protocol" is
> signed and we don't check for array underflows.  If you have already
> have CAP_SYS_ADMIN then you could use the bugs to get root, or someone
> could cause an oops by mistake.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Applied.
--
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
Rémi Denis-Courmont Jan. 13, 2011, 12:32 p.m. UTC | #3
On Tuesday 11 January 2011 02:06:20 ext David Miller, you wrote:
> From: Dan Carpenter <error27@gmail.com>
> Date: Mon, 10 Jan 2011 17:06:58 +0300
> 
> > Dan Rosenberg pointed out that there were some signed comparison bugs
> > in the phonet protocol.
> > 
> > http://marc.info/?l=full-disclosure&m=129424528425330&w=2
> > 
> > The problem is that we check for array overflows but "protocol" is
> > signed and we don't check for array underflows.  If you have already
> > have CAP_SYS_ADMIN then you could use the bugs to get root, or someone
> > could cause an oops by mistake.
> > 
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> Applied.

Shouldn't this be sent to stable trees?
David Miller Jan. 13, 2011, 8:30 p.m. UTC | #4
From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Thu, 13 Jan 2011 14:32:57 +0200

> On Tuesday 11 January 2011 02:06:20 ext David Miller, you wrote:
>> From: Dan Carpenter <error27@gmail.com>
>> Date: Mon, 10 Jan 2011 17:06:58 +0300
>> 
>> > Dan Rosenberg pointed out that there were some signed comparison bugs
>> > in the phonet protocol.
>> > 
>> > http://marc.info/?l=full-disclosure&m=129424528425330&w=2
>> > 
>> > The problem is that we check for array overflows but "protocol" is
>> > signed and we don't check for array underflows.  If you have already
>> > have CAP_SYS_ADMIN then you could use the bugs to get root, or someone
>> > could cause an oops by mistake.
>> > 
>> > Signed-off-by: Dan Carpenter <error27@gmail.com>
>> 
>> Applied.
> 
> Shouldn't this be sent to stable trees?

It will be.
--
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/include/net/phonet/phonet.h b/include/net/phonet/phonet.h
index d5df797..5395e09 100644
--- a/include/net/phonet/phonet.h
+++ b/include/net/phonet/phonet.h
@@ -107,8 +107,8 @@  struct phonet_protocol {
 	int			sock_type;
 };
 
-int phonet_proto_register(int protocol, struct phonet_protocol *pp);
-void phonet_proto_unregister(int protocol, struct phonet_protocol *pp);
+int phonet_proto_register(unsigned int protocol, struct phonet_protocol *pp);
+void phonet_proto_unregister(unsigned int protocol, struct phonet_protocol *pp);
 
 int phonet_sysctl_init(void);
 void phonet_sysctl_exit(void);
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index fd95beb..1072b2c 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -37,7 +37,7 @@ 
 /* Transport protocol registration */
 static struct phonet_protocol *proto_tab[PHONET_NPROTO] __read_mostly;
 
-static struct phonet_protocol *phonet_proto_get(int protocol)
+static struct phonet_protocol *phonet_proto_get(unsigned int protocol)
 {
 	struct phonet_protocol *pp;
 
@@ -458,7 +458,7 @@  static struct packet_type phonet_packet_type __read_mostly = {
 
 static DEFINE_MUTEX(proto_tab_lock);
 
-int __init_or_module phonet_proto_register(int protocol,
+int __init_or_module phonet_proto_register(unsigned int protocol,
 						struct phonet_protocol *pp)
 {
 	int err = 0;
@@ -481,7 +481,7 @@  int __init_or_module phonet_proto_register(int protocol,
 }
 EXPORT_SYMBOL(phonet_proto_register);
 
-void phonet_proto_unregister(int protocol, struct phonet_protocol *pp)
+void phonet_proto_unregister(unsigned int protocol, struct phonet_protocol *pp)
 {
 	mutex_lock(&proto_tab_lock);
 	BUG_ON(proto_tab[protocol] != pp);