From patchwork Thu Aug 9 16:39:05 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: danborkmann@iogearbox.net X-Patchwork-Id: 176191 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C00B02C00DA for ; Fri, 10 Aug 2012 02:39:16 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758688Ab2HIQjO (ORCPT ); Thu, 9 Aug 2012 12:39:14 -0400 Received: from www62.your-server.de ([213.133.104.62]:44887 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758678Ab2HIQjN (ORCPT ); Thu, 9 Aug 2012 12:39:13 -0400 Received: from [78.46.5.203] (helo=sslproxy01.your-server.de) by www62.your-server.de with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.74) (envelope-from ) id 1SzVlQ-0005PT-8S; Thu, 09 Aug 2012 18:39:12 +0200 Received: from [82.130.103.141] by sslproxy01.your-server.de with esmtpa (Exim 4.72) (envelope-from ) id 1SzVnA-0007Zv-7a; Thu, 09 Aug 2012 18:41:00 +0200 Subject: [PATCH net-next] af_packet: relax BUG statement in tpacket_destruct_skb From: Daniel Borkmann To: davem@davemloft.net Cc: netdev@vger.kernel.org Date: Thu, 09 Aug 2012 18:39:05 +0200 Message-ID: <1344530345.28842.11.camel@thinkbox> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 X-Authenticated-Sender: danborkmann@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.97.3/15235/Thu Aug 9 17:50:13 2012) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Here's a quote of the comment about the BUG macro from asm-generic/bug.h: Don't use BUG() or BUG_ON() unless there's really no way out; one example might be detecting data structure corruption in the middle of an operation that can't be backed out of. If the (sub)system can somehow continue operating, perhaps with reduced functionality, it's probably not BUG-worthy. If you're tempted to BUG(), think again: is completely giving up really the *only* solution? There are usually better options, where users don't need to reboot ASAP and can mostly shut down cleanly. In our case, the status flag of a ring buffer slot is managed from both sides, the kernel space and the user space. This means that even though the kernel side might work as expected, the user space screws up and changes this flag right between the send(2) is triggered when the flag is changed to TP_STATUS_SENDING and a given skb is destructed after some time. Then, this will hit the BUG macro. Instead, we relax this condition with a WARN_ON_ONCE macro, so that the user is aware of this situation. I've tested it and the system still behaves /stable/, so in accordance with the above comment, we should rather relax this behavior with a warning. Signed-off-by: Daniel Borkmann --- net/packet/af_packet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ceaca7c..4def36f 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1936,7 +1936,7 @@ static void tpacket_destruct_skb(struct sk_buff *skb) if (likely(po->tx_ring.pg_vec)) { ph = skb_shinfo(skb)->destructor_arg; - BUG_ON(__packet_get_status(po, ph) != TP_STATUS_SENDING); + WARN_ON_ONCE(__packet_get_status(po, ph) != TP_STATUS_SENDING); BUG_ON(atomic_read(&po->tx_ring.pending) == 0); atomic_dec(&po->tx_ring.pending); __packet_set_status(po, ph, TP_STATUS_AVAILABLE);