All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Josh Hunt <joshhunt00@gmail.com>,
	Honggang Li <honli@redhat.com>,
	Bang Nguyen <bang.nguyen@oracle.com>,
	Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 3.4 02/27] rds: prevent BUG_ON triggered on congestion update to loopback
Date: Mon, 13 Jan 2014 16:26:16 -0800	[thread overview]
Message-ID: <20140114002623.428482670@linuxfoundation.org> (raw)
In-Reply-To: <20140114002623.356220317@linuxfoundation.org>

3.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>

[ Upstream commit 18fc25c94eadc52a42c025125af24657a93638c0 ]

After congestion update on a local connection, when rds_ib_xmit returns
less bytes than that are there in the message, rds_send_xmit calls
back rds_ib_xmit with an offset that causes BUG_ON(off & RDS_FRAG_SIZE)
to trigger.

For a 4Kb PAGE_SIZE rds_ib_xmit returns min(8240,4096)=4096 when actually
the message contains 8240 bytes. rds_send_xmit thinks there is more to send
and calls rds_ib_xmit again with a data offset "off" of 4096-48(rds header)
=4048 bytes thus hitting the BUG_ON(off & RDS_FRAG_SIZE) [RDS_FRAG_SIZE=4k].

The commit 6094628bfd94323fc1cea05ec2c6affd98c18f7f
"rds: prevent BUG_ON triggering on congestion map updates" introduced
this regression. That change was addressing the triggering of a different
BUG_ON in rds_send_xmit() on PowerPC architecture with 64Kbytes PAGE_SIZE:
 	BUG_ON(ret != 0 &&
    		 conn->c_xmit_sg == rm->data.op_nents);
This was the sequence it was going through:
(rds_ib_xmit)
/* Do not send cong updates to IB loopback */
if (conn->c_loopback
   && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
  	rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
    	return sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
}
rds_ib_xmit returns 8240
rds_send_xmit:
  c_xmit_data_off = 0 + 8240 - 48 (rds header accounted only the first time)
   		 = 8192
  c_xmit_data_off < 65536 (sg->length), so calls rds_ib_xmit again
rds_ib_xmit returns 8240
rds_send_xmit:
  c_xmit_data_off = 8192 + 8240 = 16432, calls rds_ib_xmit again
  and so on (c_xmit_data_off 24672,32912,41152,49392,57632)
rds_ib_xmit returns 8240
On this iteration this sequence causes the BUG_ON in rds_send_xmit:
    while (ret) {
    	tmp = min_t(int, ret, sg->length - conn->c_xmit_data_off);
    	[tmp = 65536 - 57632 = 7904]
    	conn->c_xmit_data_off += tmp;
    	[c_xmit_data_off = 57632 + 7904 = 65536]
    	ret -= tmp;
    	[ret = 8240 - 7904 = 336]
    	if (conn->c_xmit_data_off == sg->length) {
    		conn->c_xmit_data_off = 0;
    		sg++;
    		conn->c_xmit_sg++;
    		BUG_ON(ret != 0 &&
    			conn->c_xmit_sg == rm->data.op_nents);
    		[c_xmit_sg = 1, rm->data.op_nents = 1]

What the current fix does:
Since the congestion update over loopback is not actually transmitted
as a message, all that rds_ib_xmit needs to do is let the caller think
the full message has been transmitted and not return partial bytes.
It will return 8240 (RDS_CONG_MAP_BYTES+48) when PAGE_SIZE is 4Kb.
And 64Kb+48 when page size is 64Kb.

Reported-by: Josh Hunt <joshhunt00@gmail.com>
Tested-by: Honggang Li <honli@redhat.com>
Acked-by: Bang Nguyen <bang.nguyen@oracle.com>
Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/rds/ib_send.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -552,9 +552,8 @@ int rds_ib_xmit(struct rds_connection *c
 	    && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
 		rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
 		scat = &rm->data.op_sg[sg];
-		ret = sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
-		ret = min_t(int, ret, scat->length - conn->c_xmit_data_off);
-		return ret;
+		ret = max_t(int, RDS_CONG_MAP_BYTES, scat->length);
+		return sizeof(struct rds_header) + ret;
 	}
 
 	/* FIXME we may overallocate here */



  parent reply	other threads:[~2014-01-14  1:16 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-14  0:26 [PATCH 3.4 00/27] 3.4.77-stable review Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 01/27] net: do not pretend FRAGLIST support Greg Kroah-Hartman
2014-01-14  0:26 ` Greg Kroah-Hartman [this message]
2014-01-14  0:26 ` [PATCH 3.4 03/27] macvtap: Do not double-count received packets Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 04/27] macvtap: update file current position Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 05/27] tun: " Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 06/27] macvtap: signal truncated packets Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 07/27] ipv6: dont count addrconf generated routes against gc limit Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 08/27] net: drop_monitor: fix the value of maxattr Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 09/27] net: unix: allow set_peek_off to fail Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 10/27] tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0 Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 11/27] netvsc: dont flush peers notifying work during setting mtu Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 12/27] net: unix: allow bind to fail on mutex lock Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 13/27] net: inet_diag: zero out uninitialized idiag_{src,dst} fields Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 14/27] drivers/net/hamradio: Integer overflow in hdlcdrv_ioctl() Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 16/27] rds: prevent dereference of a NULL device Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 17/27] net: rose: restore old recvmsg behavior Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 18/27] vlan: Fix header ops passthru when doing TX VLAN offload Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 19/27] net: llc: fix use after free in llc_ui_recvmsg Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 20/27] bridge: use spin_lock_bh() in br_multicast_set_hash_max Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 21/27] ARM: fix "bad mode in ... handler" message for undefined instructions Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 22/27] ARM: shmobile: mackerel: Fix coherent DMA mask Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 23/27] x86, fpu, amd: Clear exceptions in AMD FXSAVE workaround Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 24/27] sched: Fix race on toggling cfs_bandwidth_used Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 25/27] sched: Fix cfs_bandwidth misuse of hrtimer_expires_remaining Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 26/27] sched: Fix hrtimer_cancel()/rq->lock deadlock Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 27/27] sched: Guarantee new group-entities always have weight Greg Kroah-Hartman
2014-01-14  2:59 ` [PATCH 3.4 00/27] 3.4.77-stable review Guenter Roeck
2014-01-14  3:03   ` Greg Kroah-Hartman
2014-01-14 19:29 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140114002623.428482670@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=bang.nguyen@oracle.com \
    --cc=davem@davemloft.net \
    --cc=honli@redhat.com \
    --cc=joshhunt00@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=venkat.x.venkatsubra@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.