[STABLE 4.14+][PATCH 0/2] IRDA fixes

[STABLE 4.14+][PATCH 0/2] IRDA fixes

[Tyler Hicks]

Hello - Two issues were reported to Ubuntu in the IRDA subsystem. IRDA is no
longer present in the upstream kernel as of 4.17 but the stable tree is
affected.

This patch set addresses the issues in 4.14 to 4.17.

Tyler

[PATCH 1/2] irda: Fix memory leak caused by repeated binds of irda socket

[Tyler Hicks]

The irda_bind() function allocates memory for self->ias_obj without
checking to see if the socket is already bound. A userspace process
could repeatedly bind the socket, have each new object added into the
LM-IAS database, and lose the reference to the old object assigned to
the socket to exhaust memory resources. This patch errors out of the
bind operation when self->ias_obj is already assigned.

CVE-2018-6554

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
---
 drivers/staging/irda/net/af_irda.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/irda/net/af_irda.c b/drivers/staging/irda/net/af_irda.c
index 23fa7c8b09a5..a08cd3dd7a6e 100644
--- a/drivers/staging/irda/net/af_irda.c
+++ b/drivers/staging/irda/net/af_irda.c
@@ -775,6 +775,13 @@ static int irda_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		return -EINVAL;
 
 	lock_sock(sk);
+
+	/* Ensure that the socket is not already bound */
+	if (self->ias_obj) {
+		err = -EINVAL;
+		goto out;
+	}
+
 #ifdef CONFIG_IRDA_ULTRA
 	/* Special care for Ultra sockets */
 	if ((sk->sk_type == SOCK_DGRAM) &&
-- 
2.7.4

[PATCH 2/2] irda: Only insert new objects into the global database via setsockopt

[Tyler Hicks]

The irda_setsockopt() function conditionally allocates memory for a new
self->ias_object or, in some cases, reuses the existing
self->ias_object. Existing objects were incorrectly reinserted into the
LM_IAS database which corrupted the doubly linked list used for the
hashbin implementation of the LM_IAS database. When combined with a
memory leak in irda_bind(), this issue could be leveraged to create a
use-after-free vulnerability in the hashbin list. This patch fixes the
issue by only inserting newly allocated objects into the database.

CVE-2018-6555

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
---
 drivers/staging/irda/net/af_irda.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/irda/net/af_irda.c b/drivers/staging/irda/net/af_irda.c
index a08cd3dd7a6e..cebe9878ca03 100644
--- a/drivers/staging/irda/net/af_irda.c
+++ b/drivers/staging/irda/net/af_irda.c
@@ -2019,7 +2019,11 @@ static int irda_setsockopt(struct socket *sock, int level, int optname,
 			err = -EINVAL;
 			goto out;
 		}
-		irias_insert_object(ias_obj);
+
+		/* Only insert newly allocated objects */
+		if (free_ias)
+			irias_insert_object(ias_obj);
+
 		kfree(ias_opt);
 		break;
 	case IRLMP_IAS_DEL:
-- 
2.7.4

Re: [STABLE 4.14+][PATCH 0/2] IRDA fixes

[Greg KH]

On Tue, Sep 04, 2018 at 03:24:03PM +0000, Tyler Hicks wrote:
> Hello - Two issues were reported to Ubuntu in the IRDA subsystem. IRDA is no
> longer present in the upstream kernel as of 4.17 but the stable tree is
> affected.

Given that irda is broken in these trees, how can anyone even trigger
these bugs?  How is the code being loaded by a normal user?

thanks,

greg k-h

Re: [PATCH 1/2] irda: Fix memory leak caused by repeated binds of irda socket

[Greg KH]

On Tue, Sep 04, 2018 at 03:24:04PM +0000, Tyler Hicks wrote:
> The irda_bind() function allocates memory for self->ias_obj without
> checking to see if the socket is already bound. A userspace process
> could repeatedly bind the socket, have each new object added into the
> LM-IAS database, and lose the reference to the old object assigned to
> the socket to exhaust memory resources. This patch errors out of the
> bind operation when self->ias_obj is already assigned.
> 
> CVE-2018-6554
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
> Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
> ---

No "Reported-by:" lines?

And agin, how can you trigger any of this given the code doesn't even
work?  Can you load irda modules as a "normal" user?

thanks,

greg k-h

Re: [STABLE 4.14+][PATCH 0/2] IRDA fixes

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 1141 bytes --]

On 09/12/2018 02:34 PM, Greg KH wrote:
> On Tue, Sep 04, 2018 at 03:24:03PM +0000, Tyler Hicks wrote:
>> Hello - Two issues were reported to Ubuntu in the IRDA subsystem. IRDA is no
>> longer present in the upstream kernel as of 4.17 but the stable tree is
>> affected.
> 
> Given that irda is broken in these trees, how can anyone even trigger
> these bugs?  How is the code being loaded by a normal user?

I'm unaware about how broken irda is in these trees but opening an
AF_IRDA socket is sufficient for the reported issues:

$ uname -r
4.14.69+
$ lsmod | grep irda
$ cat irda.c
#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>

int main(void)
{
        int fd;

        fd = socket(AF_IRDA, SOCK_SEQPACKET, 0);
        if (fd == -1) {
                perror("socket");
                return 1;
        }

        return 0;
}
$ gcc -o irda irda.c
$ ./irda
$ lsmod | grep irda
irda                  233472  0
crc_ccitt              16384  1 irda

Once you have the socket fd, you can perform operations on it to
manipulate the LM_IAS database and trigger these issues.

Tyler


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] irda: Fix memory leak caused by repeated binds of irda socket

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 1249 bytes --]

On 09/12/2018 02:35 PM, Greg KH wrote:
> On Tue, Sep 04, 2018 at 03:24:04PM +0000, Tyler Hicks wrote:
>> The irda_bind() function allocates memory for self->ias_obj without
>> checking to see if the socket is already bound. A userspace process
>> could repeatedly bind the socket, have each new object added into the
>> LM-IAS database, and lose the reference to the old object assigned to
>> the socket to exhaust memory resources. This patch errors out of the
>> bind operation when self->ias_obj is already assigned.
>>
>> CVE-2018-6554
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
>> Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
>> Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
>> ---
> 
> No "Reported-by:" lines?

I always like to give credit with Reported-by tags but this was a rare
situation where the reporter didn't want to be acknowledged.

> And agin, how can you trigger any of this given the code doesn't even
> work?  Can you load irda modules as a "normal" user?

I answered these questions in my other reply. The irda socket interface
works well enough to reach the affected code.

Tyler

> 
> thanks,
> 
> greg k-h
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] irda: Fix memory leak caused by repeated binds of irda socket

[Greg KH]

On Wed, Sep 12, 2018 at 03:49:16PM -0500, Tyler Hicks wrote:
> On 09/12/2018 02:35 PM, Greg KH wrote:
> > On Tue, Sep 04, 2018 at 03:24:04PM +0000, Tyler Hicks wrote:
> >> The irda_bind() function allocates memory for self->ias_obj without
> >> checking to see if the socket is already bound. A userspace process
> >> could repeatedly bind the socket, have each new object added into the
> >> LM-IAS database, and lose the reference to the old object assigned to
> >> the socket to exhaust memory resources. This patch errors out of the
> >> bind operation when self->ias_obj is already assigned.
> >>
> >> CVE-2018-6554
> >>
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> >> Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
> >> Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
> >> ---
> > 
> > No "Reported-by:" lines?
> 
> I always like to give credit with Reported-by tags but this was a rare
> situation where the reporter didn't want to be acknowledged.

Fair enough, I had to ask :)

> > And agin, how can you trigger any of this given the code doesn't even
> > work?  Can you load irda modules as a "normal" user?
> 
> I answered these questions in my other reply. The irda socket interface
> works well enough to reach the affected code.

Ok, thanks for the patches, I'll go queue them up everywhere now.

greg k-h

[PATCH 1/2] irda: Fix memory leak caused by repeated binds of irda socket

[Tyler Hicks]

The irda_bind() function allocates memory for self->ias_obj without
checking to see if the socket is already bound. A userspace process
could repeatedly bind the socket, have each new object added into the
LM-IAS database, and lose the reference to the old object assigned to
the socket to exhaust memory resources. This patch errors out of the
bind operation when self->ias_obj is already assigned.

CVE-2018-6554

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
Reviewed-by: Stefan Bader <stefan.bader@canonical.com>
---
 net/irda/af_irda.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 4a116d766c15..82e632b2c5a1 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -774,6 +774,13 @@ static int irda_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		return -EINVAL;
 
 	lock_sock(sk);
+
+	/* Ensure that the socket is not already bound */
+	if (self->ias_obj) {
+		err = -EINVAL;
+		goto out;
+	}
+
 #ifdef CONFIG_IRDA_ULTRA
 	/* Special care for Ultra sockets */
 	if ((sk->sk_type == SOCK_DGRAM) &&
-- 
2.7.4