From patchwork Thu Dec 16 08:56:27 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ralph Loader X-Patchwork-Id: 75726 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 C1D601007D3 for ; Thu, 16 Dec 2010 20:06:43 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752652Ab0LPJGS (ORCPT ); Thu, 16 Dec 2010 04:06:18 -0500 Received: from mailfilter4.ihug.co.nz ([203.109.136.4]:58593 "EHLO mailfilter4.ihug.co.nz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753044Ab0LPJGM (ORCPT ); Thu, 16 Dec 2010 04:06:12 -0500 X-Greylist: delayed 584 seconds by postgrey-1.27 at vger.kernel.org; Thu, 16 Dec 2010 04:06:12 EST X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AtAFAHdiCU12XLsg/2dsb2JhbACWBo4tdMEVhUoEinuFHIJW X-IronPort-AV: E=Sophos;i="4.59,354,1288522800"; d="scan'208";a="318448326" Received: from 118-92-187-32.dsl.dyn.ihug.co.nz (HELO i.geek.nz) ([118.92.187.32]) by cust.filter4.content.vf.net.nz with SMTP; 16 Dec 2010 21:56:27 +1300 Date: Thu, 16 Dec 2010 21:56:27 +1300 From: Ralph Loader To: netdev@vger.kernel.org Subject: Buglet in net/pkt_cls.h pointer handling. Message-Id: <20101216215627.43f34977.suckfish@ihug.co.nz> X-Mailer: Sylpheed 3.1.0beta3 (GTK+ 2.22.0; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, tcf_valid_offset() in net/pkt_cls.h appears to have a couple of problems (obvious patch below): (a) there is no check for overflow in the pointer arithmetic. (b) the pointers are presumably likely to be normally valid, so the hint should be 'likely()' not 'unlikely()'. The offsets used to construct the arguments to that function, e.g., as called in net/sched/em_u32.c, I think come from user-space & in theory could be crafted to cause an invalid pointer deref if ptr+len overflows? Possibly the '<' and '>' in that function should be '<=' and '>=' also. I'm not familiar enough with the data-structures to be sure. Also a question: in em_u32.c em_u32_match(), and in cls_u32.c u32_classify(), we dereference pointers that have had an offset (originally from user space) added to them. I can't see anything that keeps those pointers aligned. Is that a problem on architectures that don't support unaligned pointers, or am I missing something? Cheers, Ralph. --- 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/include/net/pkt_cls.h b/include/net/pkt_cls.h index dd3031a..99a2d7b 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -323,7 +323,7 @@ static inline unsigned char * tcf_get_base_ptr(struct sk_buff *skb, int layer) static inline int tcf_valid_offset(const struct sk_buff *skb, const unsigned char *ptr, const int len) { - return unlikely((ptr + len) < skb_tail_pointer(skb) && ptr > skb->head); + return likely((ptr + len) < skb_tail_pointer(skb) && ptr > skb->head && ptr <= ptr + len); } #ifdef CONFIG_NET_CLS_IND