[PATCH] rds: fix local ping DoS

[PATCH] rds: fix local ping DoS

[Josh Hunt]

The rds_ib_xmit function in net/rds/ib_send.c in the Reliable Datagram Sockets
(RDS) protocol implementation allows local users to cause a denial of service
(BUG_ON and kernel panic) by establishing an RDS connection with the source
IP address equal to the IPoIB interface's own IP address, as demonstrated by
rds-ping.

A local unprivileged user could use this flaw to crash the system.

CVE-2012-2372

Reported-by: Honggang Li <honli@redhat.com>
Signed-off-by: Josh Hunt <johunt@akamai.com>
---
 net/rds/ib_send.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
index e590949..7920c85 100644
--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -544,7 +544,7 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm,
 	int flow_controlled = 0;
 	int nr_sig = 0;
 
-	BUG_ON(off % RDS_FRAG_SIZE);
+	BUG_ON(!conn->c_loopback && off % RDS_FRAG_SIZE);
 	BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header));
 
 	/* Do not send cong updates to IB loopback */
-- 
1.7.0.4

Re: [PATCH] rds: fix local ping DoS

[David Miller]

From: Josh Hunt <johunt@akamai.com>
Date: Wed, 13 Nov 2013 17:15:43 -0800

> The rds_ib_xmit function in net/rds/ib_send.c in the Reliable Datagram Sockets
> (RDS) protocol implementation allows local users to cause a denial of service
> (BUG_ON and kernel panic) by establishing an RDS connection with the source
> IP address equal to the IPoIB interface's own IP address, as demonstrated by
> rds-ping.
> 
> A local unprivileged user could use this flaw to crash the system.
> 
> CVE-2012-2372
> 
> Reported-by: Honggang Li <honli@redhat.com>
> Signed-off-by: Josh Hunt <johunt@akamai.com>

I'm sorry I can't apply this.  This commit message needs to be much
less terse and explain things more.

First of all, why is the "off % RDS_FRAG_SIZE" important?

And, even more importantly, why is is OK to avoid this assertion just
because we're going over loopback?

Furthermore, why doesn't net/rds/iw_send.c:rds_iw_xmit() have the same
exact problem?  It makes the same exact assertion check.

I know this RDS code is a steaming pile of poo, but that doesn't mean
we just randomly adjust assertions to make crashes go away without
sufficient understanding of exactly what's going on.

Thanks.

Re: [PATCH] rds: fix local ping DoS

[Josh Hunt]

On 11/14/2013 01:03 AM, David Miller wrote:
> From: Josh Hunt <johunt@akamai.com>
> Date: Wed, 13 Nov 2013 17:15:43 -0800
>
>> The rds_ib_xmit function in net/rds/ib_send.c in the Reliable Datagram Sockets
>> (RDS) protocol implementation allows local users to cause a denial of service
>> (BUG_ON and kernel panic) by establishing an RDS connection with the source
>> IP address equal to the IPoIB interface's own IP address, as demonstrated by
>> rds-ping.
>>
>> A local unprivileged user could use this flaw to crash the system.
>>
>> CVE-2012-2372
>>
>> Reported-by: Honggang Li <honli@redhat.com>
>> Signed-off-by: Josh Hunt <johunt@akamai.com>
>
> I'm sorry I can't apply this.  This commit message needs to be much
> less terse and explain things more.
>
> First of all, why is the "off % RDS_FRAG_SIZE" important?
>
> And, even more importantly, why is is OK to avoid this assertion just
> because we're going over loopback?
>
> Furthermore, why doesn't net/rds/iw_send.c:rds_iw_xmit() have the same
> exact problem?  It makes the same exact assertion check.
>
> I know this RDS code is a steaming pile of poo, but that doesn't mean
> we just randomly adjust assertions to make crashes go away without
> sufficient understanding of exactly what's going on.
>
> Thanks.
>

Sure understandable questions. Unfortunately I don't have the hardware 
to properly debug and analyze. I was just trying to get this through on 
the assumption that the previous attempts just failed due to incorrect 
submission procedures and lack of a reproducible testcase. If nothing 
else this whole thing brought out the testcase :)

Testcase from Honggang's earlier mail:
<snip>
The test case is very simple:
Steps to Reproduce:
1. yum install -y rds-tools

2. [root@rdma3 ~]# ifconfig ib0 | grep 'inet addr'
           inet addr:172.31.0.3  Bcast:172.31.0.255  Mask:255.255.255.0

3. [root@rdma3 ~]# /usr/bin/rds-ping 172.31.0.3  <<<< kernel panic (You
may need to wait for a few seconds before the kernel panic.)

This bug can be reproduced with Mellanox HCAs (mlx4_ib.ko and mthca.ko),
QLogic HCA (ib_qib.ko). I did not test the QLogic HCA running "ib_ipath.ko".
</snip>

Perhaps Venkat or someone else with the hardware mentioned can provide a 
better explanation and better solution to the crash.

Josh

Re: [PATCH] rds: fix local ping DoS

[Jay Fenlason]

On Thu, Nov 14, 2013 at 02:03:55AM -0500, David Miller wrote:
> From: Josh Hunt <johunt@akamai.com>
> Date: Wed, 13 Nov 2013 17:15:43 -0800
> 
> > The rds_ib_xmit function in net/rds/ib_send.c in the Reliable Datagram Sockets
> > (RDS) protocol implementation allows local users to cause a denial of service
> > (BUG_ON and kernel panic) by establishing an RDS connection with the source
> > IP address equal to the IPoIB interface's own IP address, as demonstrated by
> > rds-ping.
> > 
> > A local unprivileged user could use this flaw to crash the system.
> > 
> > CVE-2012-2372
> > 
> > Reported-by: Honggang Li <honli@redhat.com>
> > Signed-off-by: Josh Hunt <johunt@akamai.com>
> 
> I'm sorry I can't apply this.  This commit message needs to be much
> less terse and explain things more.
> 
> First of all, why is the "off % RDS_FRAG_SIZE" important?
> 
> And, even more importantly, why is is OK to avoid this assertion just
> because we're going over loopback?
> 
> Furthermore, why doesn't net/rds/iw_send.c:rds_iw_xmit() have the same
> exact problem?  It makes the same exact assertion check.
> 
> I know this RDS code is a steaming pile of poo, but that doesn't mean
> we just randomly adjust assertions to make crashes go away without
> sufficient understanding of exactly what's going on.

And that is why rds should be moved to staging, where nobody will
accidentally think that it is actually being maintained.

			-- JF