Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
I've tossed these two patches under the carpet, so you'll need to repost whichever one you want me to consider. Basically, discussing old patches is pretty useless without a resend to get it back into the fore-front of the patchwork queue. So please don't reference old stale patches without an associated repost like this. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
Hi Dave, Have you thought about picking up one of the patches to tcp_recvmsg I proposed in this thread? We consider the underlying bug in Chromium OS that led mere here to be fixed now, but I bet this will not be the last time someone hits this code path and has to deal with the bad error handling. I understand that not everyone here agrees on what the best solution is, but I think both of them are far better than the inconsistent and potentially hard-disk-filling way that the current kernel does it. On Wed, 2012-11-07 at 11:33 -0800, Julius Werner wrote: > tcp_recvmsg contains a sanity check that WARNs when there is a gap > between the socket's copied_seq and the first buffer in the > sk_receive_queue. In theory, the TCP stack makes sure that This Should > Never Happen (TM)... however, practice shows that there are still a few > bug reports from it out there (and one in my inbox). > > Unfortunately, when it does happen for whatever reason, the situation > is not handled very well: the kernel logs a warning and breaks out of > the loop that walks the receive queue. It proceeds to find nothing else > to do on the socket and hits sk_wait_data, which cannot block because > the receive queue is not empty. As no data was read, the outer while > loop repeats (logging the same warning again) ad infinitum until the > system's syslog exhausts all available hard drive capacity. > > This patch addresses that issue by closing the socket outright and > throwing EBADFD to userspace (which seems most appropriate to me at this > point). As the underlying bug condition is "impossible" and therefore by > definition unrecoverable, this is the only sensible action other than a > full panic. > > Signed-off-by: Julius Werner > --- > net/ipv4/tcp.c |7 ++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 197c000..d612308 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, > struct msghdr *msg, >"recvmsg bug: copied %X seq %X rcvnxt %X fl > %X\n", >*seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt, >flags)) > - break; > + goto selfdestruct; > > offset = *seq - TCP_SKB_CB(skb)->seq; > if (tcp_hdr(skb)->syn) > @@ -1936,6 +1936,11 @@ recv_urg: > recv_sndq: > err = tcp_peek_sndq(sk, msg, len); > goto out; > + > +selfdestruct: > + err = -EBADFD; > + tcp_done(sk); > + goto out; > } > EXPORT_SYMBOL(tcp_recvmsg); > On Tue, Nov 06, 2012 at 04:15:35PM -0800, Julius Werner wrote: > tcp_recvmsg contains a sanity check that WARNs when there is a gap > between the socket's copied_seq and the first buffer in the > sk_receive_queue. In theory, the TCP stack makes sure that This Should > Never Happen (TM)... however, practice shows that there are still a few > bug reports from it out there (and one in my inbox). > > Unfortunately, when it does happen for whatever reason, the situation > is not handled very well: the kernel logs a warning and breaks out of > the loop that walks the receive queue. It proceeds to find nothing else > to do on the socket and hits sk_wait_data, which cannot block because > the receive queue is not empty. As no data was read, the outer while > loop repeats (logging the same warning again) ad infinitum until the > system's syslog exhausts all available hard drive capacity. > > This patch improves that behavior by going straight to a proper kernel > crash. The cause of the error can be identified right away and the > system's hard drive is not unnecessarily strained. > > Signed-off-by: Julius Werner > --- > net/ipv4/tcp.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 197c000..fcb0927 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, > struct msghdr *msg, > "recvmsg bug: copied %X seq %X rcvnxt %X fl > %X\n", > *seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt, > flags)) > -break; > +BUG(); > > offset = *seq - TCP_SKB_CB(skb)->seq; > if (tcp_hdr(skb)->syn) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
Hi Dave, Have you thought about picking up one of the patches to tcp_recvmsg I proposed in this thread? We consider the underlying bug in Chromium OS that led mere here to be fixed now, but I bet this will not be the last time someone hits this code path and has to deal with the bad error handling. I understand that not everyone here agrees on what the best solution is, but I think both of them are far better than the inconsistent and potentially hard-disk-filling way that the current kernel does it. On Wed, 2012-11-07 at 11:33 -0800, Julius Werner wrote: tcp_recvmsg contains a sanity check that WARNs when there is a gap between the socket's copied_seq and the first buffer in the sk_receive_queue. In theory, the TCP stack makes sure that This Should Never Happen (TM)... however, practice shows that there are still a few bug reports from it out there (and one in my inbox). Unfortunately, when it does happen for whatever reason, the situation is not handled very well: the kernel logs a warning and breaks out of the loop that walks the receive queue. It proceeds to find nothing else to do on the socket and hits sk_wait_data, which cannot block because the receive queue is not empty. As no data was read, the outer while loop repeats (logging the same warning again) ad infinitum until the system's syslog exhausts all available hard drive capacity. This patch addresses that issue by closing the socket outright and throwing EBADFD to userspace (which seems most appropriate to me at this point). As the underlying bug condition is impossible and therefore by definition unrecoverable, this is the only sensible action other than a full panic. Signed-off-by: Julius Werner jwer...@chromium.org --- net/ipv4/tcp.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 197c000..d612308 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, recvmsg bug: copied %X seq %X rcvnxt %X fl %X\n, *seq, TCP_SKB_CB(skb)-seq, tp-rcv_nxt, flags)) - break; + goto selfdestruct; offset = *seq - TCP_SKB_CB(skb)-seq; if (tcp_hdr(skb)-syn) @@ -1936,6 +1936,11 @@ recv_urg: recv_sndq: err = tcp_peek_sndq(sk, msg, len); goto out; + +selfdestruct: + err = -EBADFD; + tcp_done(sk); + goto out; } EXPORT_SYMBOL(tcp_recvmsg); On Tue, Nov 06, 2012 at 04:15:35PM -0800, Julius Werner wrote: tcp_recvmsg contains a sanity check that WARNs when there is a gap between the socket's copied_seq and the first buffer in the sk_receive_queue. In theory, the TCP stack makes sure that This Should Never Happen (TM)... however, practice shows that there are still a few bug reports from it out there (and one in my inbox). Unfortunately, when it does happen for whatever reason, the situation is not handled very well: the kernel logs a warning and breaks out of the loop that walks the receive queue. It proceeds to find nothing else to do on the socket and hits sk_wait_data, which cannot block because the receive queue is not empty. As no data was read, the outer while loop repeats (logging the same warning again) ad infinitum until the system's syslog exhausts all available hard drive capacity. This patch improves that behavior by going straight to a proper kernel crash. The cause of the error can be identified right away and the system's hard drive is not unnecessarily strained. Signed-off-by: Julius Werner jwer...@chromium.org --- net/ipv4/tcp.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 197c000..fcb0927 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, recvmsg bug: copied %X seq %X rcvnxt %X fl %X\n, *seq, TCP_SKB_CB(skb)-seq, tp-rcv_nxt, flags)) -break; +BUG(); offset = *seq - TCP_SKB_CB(skb)-seq; if (tcp_hdr(skb)-syn) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
I've tossed these two patches under the carpet, so you'll need to repost whichever one you want me to consider. Basically, discussing old patches is pretty useless without a resend to get it back into the fore-front of the patchwork queue. So please don't reference old stale patches without an associated repost like this. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 18:25 -0800, Julius Werner wrote: > > So you probably are fighting a bug we already fixed in upstream kernel. > > > > (commit c8628155ece363 "tcp: reduce out_of_order memory use" did not > > played well with cloned skbs.) > > > > This issue was already discussed on netdev in the past. > > Thanks for the hint. Unfortunately, we have not pulled c8628 into our > tree yet, so that's not it. Is there another point where the cloned > skb or the faked truesize might make it break? We have been running > this test with that hardware some 30 times in the last months and only > seen it once, so it cannot be that common. Update : Chrome OS current tree is based on 3.4 and really needed the patch : https://gerrit.chromium.org/gerrit/#/c/37666/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 18:25 -0800, Julius Werner wrote: So you probably are fighting a bug we already fixed in upstream kernel. (commit c8628155ece363 tcp: reduce out_of_order memory use did not played well with cloned skbs.) This issue was already discussed on netdev in the past. Thanks for the hint. Unfortunately, we have not pulled c8628 into our tree yet, so that's not it. Is there another point where the cloned skb or the faked truesize might make it break? We have been running this test with that hardware some 30 times in the last months and only seen it once, so it cannot be that common. Update : Chrome OS current tree is based on 3.4 and really needed the patch : https://gerrit.chromium.org/gerrit/#/c/37666/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
> So you probably are fighting a bug we already fixed in upstream kernel. > > (commit c8628155ece363 "tcp: reduce out_of_order memory use" did not > played well with cloned skbs.) > > This issue was already discussed on netdev in the past. Thanks for the hint. Unfortunately, we have not pulled c8628 into our tree yet, so that's not it. Is there another point where the cloned skb or the faked truesize might make it break? We have been running this test with that hardware some 30 times in the last months and only seen it once, so it cannot be that common. I have noticed that you have already proposed a patch to repair smsc95xx (replacing the clone with a copy) on this list a few times... what's the status on that? Will it be committed eventually or did you abandon that approach? Regardless of that, I still think that the bug handling in tcp_recvmsg should be updated in one way or the other. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 15:33 -0800, Eric Dumazet wrote:
> So you probably are fighting a bug we already fixed in upstream kernel.
>
> (commit c8628155ece363 "tcp: reduce out_of_order memory use" did not
> played well with cloned skbs.)
>
> This issue was already discussed on netdev in the past.
If you use a 3.4 kernel, you want the following patch.
(I guess you could reproduce the crash easily running a tcpdump in //)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 257b617..9f8f68c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4496,7 +4496,9 @@ static void tcp_data_queue_ofo(struct sock *sk, struct
sk_buff *skb)
* to avoid future tcp_collapse_ofo_queue(),
* probably the most expensive function in tcp stack.
*/
- if (skb->len <= skb_tailroom(skb1) && !tcp_hdr(skb)->fin) {
+ if (skb->len <= skb_tailroom(skb1) &&
+ !tcp_hdr(skb)->fin &&
+ !skb_cloned(skb1)) {
NET_INC_STATS_BH(sock_net(sk),
LINUX_MIB_TCPRCVCOALESCE);
BUG_ON(skb_copy_bits(skb, 0,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 13:14 -0800, Julius Werner wrote: > > What I find very sad in all this is that you didnt mention the driver > > that was triggering this bug. > > Sorry, I was just trying to keep this thread focussed on one patch. > The bug report that led me to this is publicly accessible at > http://crosbug.com/35827. We have encountered the problem only once, > on an Acer AC700 Chromebook that ran automated tests. The ethernet > interface for the offending socket was provided by a USB-to-Ethernet > dongle using the smsc95xx/usbnet module (v1.0.4). This driver uses interesting skb_clone() games and skb->truesize lies : skb->truesize = size + sizeof(struct sk_buff); So you probably are fighting a bug we already fixed in upstream kernel. (commit c8628155ece363 "tcp: reduce out_of_order memory use" did not played well with cloned skbs.) This issue was already discussed on netdev in the past. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
> What I find very sad in all this is that you didnt mention the driver > that was triggering this bug. Sorry, I was just trying to keep this thread focussed on one patch. The bug report that led me to this is publicly accessible at http://crosbug.com/35827. We have encountered the problem only once, on an Acer AC700 Chromebook that ran automated tests. The ethernet interface for the offending socket was provided by a USB-to-Ethernet dongle using the smsc95xx/usbnet module (v1.0.4). Don't get me wrong, I do understand the importance of finding the underlying cause of this... I just don't think I have much of a chance with one report. I can go through the above-mentioned module and see if something looks suspicious in the skb handling code if I can find the time. But on the other hand the fact remains that this condition is not handled well... not just for this particular case, but for all future kernel and driver bugs that may trigger it again. I am not trying to "hide" any issues, I am all for making them as visible as possible... but as Dave pointed out, kernel panics may not be the best way to do that either, and I think damage mitigation also has some value. The current code clearly does the worst of both worlds, so please let's just improve it one way or the other. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 11:33 -0800, Julius Werner wrote: > tcp_recvmsg contains a sanity check that WARNs when there is a gap > between the socket's copied_seq and the first buffer in the > sk_receive_queue. In theory, the TCP stack makes sure that This Should > Never Happen (TM)... however, practice shows that there are still a few > bug reports from it out there (and one in my inbox). > > Unfortunately, when it does happen for whatever reason, the situation > is not handled very well: the kernel logs a warning and breaks out of > the loop that walks the receive queue. It proceeds to find nothing else > to do on the socket and hits sk_wait_data, which cannot block because > the receive queue is not empty. As no data was read, the outer while > loop repeats (logging the same warning again) ad infinitum until the > system's syslog exhausts all available hard drive capacity. > > This patch addresses that issue by closing the socket outright and > throwing EBADFD to userspace (which seems most appropriate to me at this > point). As the underlying bug condition is "impossible" and therefore by > definition unrecoverable, this is the only sensible action other than a > full panic. > > Signed-off-by: Julius Werner > --- > net/ipv4/tcp.c |7 ++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 197c000..d612308 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, > struct msghdr *msg, >"recvmsg bug: copied %X seq %X rcvnxt %X fl > %X\n", >*seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt, >flags)) > - break; > + goto selfdestruct; > > offset = *seq - TCP_SKB_CB(skb)->seq; > if (tcp_hdr(skb)->syn) > @@ -1936,6 +1936,11 @@ recv_urg: > recv_sndq: > err = tcp_peek_sndq(sk, msg, len); > goto out; > + > +selfdestruct: > + err = -EBADFD; > + tcp_done(sk); > + goto out; > } > EXPORT_SYMBOL(tcp_recvmsg); > What I find very sad in all this is that you didnt mention the driver that was triggering this bug. So instead of making real progress, we are discussing of some dubious 'fixes' -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 11:33 -0800, Julius Werner wrote: tcp_recvmsg contains a sanity check that WARNs when there is a gap between the socket's copied_seq and the first buffer in the sk_receive_queue. In theory, the TCP stack makes sure that This Should Never Happen (TM)... however, practice shows that there are still a few bug reports from it out there (and one in my inbox). Unfortunately, when it does happen for whatever reason, the situation is not handled very well: the kernel logs a warning and breaks out of the loop that walks the receive queue. It proceeds to find nothing else to do on the socket and hits sk_wait_data, which cannot block because the receive queue is not empty. As no data was read, the outer while loop repeats (logging the same warning again) ad infinitum until the system's syslog exhausts all available hard drive capacity. This patch addresses that issue by closing the socket outright and throwing EBADFD to userspace (which seems most appropriate to me at this point). As the underlying bug condition is impossible and therefore by definition unrecoverable, this is the only sensible action other than a full panic. Signed-off-by: Julius Werner jwer...@chromium.org --- net/ipv4/tcp.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 197c000..d612308 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1628,7 +1628,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, recvmsg bug: copied %X seq %X rcvnxt %X fl %X\n, *seq, TCP_SKB_CB(skb)-seq, tp-rcv_nxt, flags)) - break; + goto selfdestruct; offset = *seq - TCP_SKB_CB(skb)-seq; if (tcp_hdr(skb)-syn) @@ -1936,6 +1936,11 @@ recv_urg: recv_sndq: err = tcp_peek_sndq(sk, msg, len); goto out; + +selfdestruct: + err = -EBADFD; + tcp_done(sk); + goto out; } EXPORT_SYMBOL(tcp_recvmsg); What I find very sad in all this is that you didnt mention the driver that was triggering this bug. So instead of making real progress, we are discussing of some dubious 'fixes' -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
What I find very sad in all this is that you didnt mention the driver that was triggering this bug. Sorry, I was just trying to keep this thread focussed on one patch. The bug report that led me to this is publicly accessible at http://crosbug.com/35827. We have encountered the problem only once, on an Acer AC700 Chromebook that ran automated tests. The ethernet interface for the offending socket was provided by a USB-to-Ethernet dongle using the smsc95xx/usbnet module (v1.0.4). Don't get me wrong, I do understand the importance of finding the underlying cause of this... I just don't think I have much of a chance with one report. I can go through the above-mentioned module and see if something looks suspicious in the skb handling code if I can find the time. But on the other hand the fact remains that this condition is not handled well... not just for this particular case, but for all future kernel and driver bugs that may trigger it again. I am not trying to hide any issues, I am all for making them as visible as possible... but as Dave pointed out, kernel panics may not be the best way to do that either, and I think damage mitigation also has some value. The current code clearly does the worst of both worlds, so please let's just improve it one way or the other. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 13:14 -0800, Julius Werner wrote: What I find very sad in all this is that you didnt mention the driver that was triggering this bug. Sorry, I was just trying to keep this thread focussed on one patch. The bug report that led me to this is publicly accessible at http://crosbug.com/35827. We have encountered the problem only once, on an Acer AC700 Chromebook that ran automated tests. The ethernet interface for the offending socket was provided by a USB-to-Ethernet dongle using the smsc95xx/usbnet module (v1.0.4). This driver uses interesting skb_clone() games and skb-truesize lies : skb-truesize = size + sizeof(struct sk_buff); So you probably are fighting a bug we already fixed in upstream kernel. (commit c8628155ece363 tcp: reduce out_of_order memory use did not played well with cloned skbs.) This issue was already discussed on netdev in the past. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
On Wed, 2012-11-07 at 15:33 -0800, Eric Dumazet wrote:
So you probably are fighting a bug we already fixed in upstream kernel.
(commit c8628155ece363 tcp: reduce out_of_order memory use did not
played well with cloned skbs.)
This issue was already discussed on netdev in the past.
If you use a 3.4 kernel, you want the following patch.
(I guess you could reproduce the crash easily running a tcpdump in //)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 257b617..9f8f68c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4496,7 +4496,9 @@ static void tcp_data_queue_ofo(struct sock *sk, struct
sk_buff *skb)
* to avoid future tcp_collapse_ofo_queue(),
* probably the most expensive function in tcp stack.
*/
- if (skb-len = skb_tailroom(skb1) !tcp_hdr(skb)-fin) {
+ if (skb-len = skb_tailroom(skb1)
+ !tcp_hdr(skb)-fin
+ !skb_cloned(skb1)) {
NET_INC_STATS_BH(sock_net(sk),
LINUX_MIB_TCPRCVCOALESCE);
BUG_ON(skb_copy_bits(skb, 0,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Avoid infinite loop on recvmsg bug
So you probably are fighting a bug we already fixed in upstream kernel. (commit c8628155ece363 tcp: reduce out_of_order memory use did not played well with cloned skbs.) This issue was already discussed on netdev in the past. Thanks for the hint. Unfortunately, we have not pulled c8628 into our tree yet, so that's not it. Is there another point where the cloned skb or the faked truesize might make it break? We have been running this test with that hardware some 30 times in the last months and only seen it once, so it cannot be that common. I have noticed that you have already proposed a patch to repair smsc95xx (replacing the clone with a copy) on this list a few times... what's the status on that? Will it be committed eventually or did you abandon that approach? Regardless of that, I still think that the bug handling in tcp_recvmsg should be updated in one way or the other. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
