Hi,
Structures "l2cap_conninfo" and "rfcomm_conninfo" have one padding
byte each. This byte in "cinfo" is copied to userspace uninitialized.
patch no.1:
-- cut --
--- a/net/bluetooth/l2cap_sock.c 2011-05-04 03:59:13.000000000 +0100
+++ b/net/bluetooth/l2cap_sock.c 2011-05-08 18:57:20.000000000 +0100
@@ -446,6 +446,7 @@ static int l2cap_sock_getsockopt_old(str
break;
}
+ memset(&cinfo, 0, sizeof(cinfo));
cinfo.hci_handle = l2cap_pi(sk)->conn->hcon->handle;
memcpy(cinfo.dev_class, l2cap_pi(sk)->conn->hcon->dev_class, 3);
-- cut --
patch no.2:
-- cut --
--- a/net/bluetooth/rfcomm/sock.c 2011-05-04 03:59:13.000000000 +0100
+++ b/net/bluetooth/rfcomm/sock.c 2011-05-08 19:00:24.000000000 +0100
@@ -787,6 +787,7 @@ static int rfcomm_sock_getsockopt_old(st
l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;
+ memset(&cinfo, 0, sizeof(cinfo));
cinfo.hci_handle = l2cap_pi(l2cap_sk)->conn->hcon->handle;
memcpy(cinfo.dev_class,
l2cap_pi(l2cap_sk)->conn->hcon->dev_class, 3);
-- cut --
Found by Marek Kroemeke and Filip Palian. Special thanks to Vasiliy
Kulikov for verifying this bug.
Best regards.
Hi Filip,
> Structures "l2cap_conninfo" and "rfcomm_conninfo" have one padding
> byte each. This byte in "cinfo" is copied to userspace uninitialized.
>
> Signed-off-by: Filip Palian <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
Structures "l2cap_conninfo" and "rfcomm_conninfo" have one padding
byte each. This byte in "cinfo" is copied to userspace uninitialized.
Signed-off-by: Filip Palian <[email protected]>
---
net/bluetooth/l2cap_sock.c | 1 +
net/bluetooth/rfcomm/sock.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 299fe56..581310f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -446,6 +446,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
break;
}
+ memset(&cinfo, 0, sizeof(cinfo));
cinfo.hci_handle = l2cap_pi(sk)->conn->hcon->handle;
memcpy(cinfo.dev_class, l2cap_pi(sk)->conn->hcon->dev_class, 3);
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 66cc1f0..784af0f 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -787,6 +787,7 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u
l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;
+ memset(&cinfo, 0, sizeof(cinfo));
cinfo.hci_handle = l2cap_pi(l2cap_sk)->conn->hcon->handle;
memcpy(cinfo.dev_class, l2cap_pi(l2cap_sk)->conn->hcon->dev_class, 3);
--
1.5.6.3
Found by Marek Kroemeke and Filip Palian. Special thanks to Vasiliy Kulikov for advices.
Best regards.
Hi Filip,
> Structures "l2cap_conninfo" and "rfcomm_conninfo" have one padding
> byte each. This byte in "cinfo" is copied to userspace uninitialized.
>
> patch no.1:
> -- cut --
> --- a/net/bluetooth/l2cap_sock.c 2011-05-04 03:59:13.000000000 +0100
> +++ b/net/bluetooth/l2cap_sock.c 2011-05-08 18:57:20.000000000 +0100
> @@ -446,6 +446,7 @@ static int l2cap_sock_getsockopt_old(str
> break;
> }
>
> + memset(&cinfo, 0, sizeof(cinfo));
> cinfo.hci_handle = l2cap_pi(sk)->conn->hcon->handle;
> memcpy(cinfo.dev_class, l2cap_pi(sk)->conn->hcon->dev_class, 3);
nice catch, but please create a patch via git format-patch and don't
forget a Signed-off-by line.
Regards
Marcel
On Sun, May 08, 2011 at 21:57 +0200, Filip Palian wrote:
> Structures "l2cap_conninfo" and "rfcomm_conninfo" have one padding
> byte each. This byte in "cinfo" is copied to userspace uninitialized.
Your mail client mangles the patch, it should contain original
identation tabs, not spaces. Please see Documentation/email-clients.txt.
> patch no.1:
Don't send 2 patches as one email. Either send 2 email or merge them
into one patch (if they are similar changes). You'll ease maintainers'
work.
> Found by Marek Kroemeke and Filip Palian.
Please see Documentation/SubmittingPatches, chapter 12 to find out how
to sign the patch.
And please don't crosspost to oss-security and LKML - send a separate
email to o-s with a short description of a bug and a link to the LKML
thread. LKML thread is a place to argue and discuss the solution, but
o-s subscribers are probably not interested in such a discussion.
Thanks,
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments