From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755401AbZFBAyL (ORCPT ); Mon, 1 Jun 2009 20:54:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753590AbZFBAx6 (ORCPT ); Mon, 1 Jun 2009 20:53:58 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:57946 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753456AbZFBAx6 (ORCPT ); Mon, 1 Jun 2009 20:53:58 -0400 Message-ID: <4A247875.9010507@cn.fujitsu.com> Date: Tue, 02 Jun 2009 08:55:17 +0800 From: Li Zefan User-Agent: Thunderbird 2.0.0.9 (X11/20071115) MIME-Version: 1.0 To: Frederic Weisbecker CC: Steven Rostedt , Tom Zanussi , Ingo Molnar , LKML Subject: Re: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp() References: <4A1F9FAC.6020506@cn.fujitsu.com> <4A20F71F.6030703@cn.fujitsu.com> <20090530135236.GB5969@nowhere> <4A223F7E.3090203@cn.fujitsu.com> <20090531132853.GA6013@nowhere> <4A236B0B.3000604@cn.fujitsu.com> <20090601130928.GA6000@nowhere> In-Reply-To: <20090601130928.GA6000@nowhere> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Frederic Weisbecker wrote: > On Mon, Jun 01, 2009 at 01:45:47PM +0800, Li Zefan wrote: >>>>>> I don't think there's any security issue. It's irrelevant how big the user-input >>>>>> strings are. The point is those strings are guaranteed to be NULL-terminated. >>>>>> Am I missing something? >>>>>> >>>>>> And I don't think it's necessary to make 2 patches that each patch converts >>>>>> one strncmp to strcmp. But maybe it's better to improve this changelog? >>>>> Hmm, you must be right, indeed they seem to be guaranted beeing NULL-terminated >>>>> strings. >>>>> >>>> Sorry, I was wrong. :( >>>> >>>> Though the user-input strings are guaranted to be NULL-terminated, strings >>>> generated by TRACE_EVENT might not. >>>> >>>> We define static strings this way: >>>> TP_struct( >>>> __array(char, foo, LEN) >>>> ) >>>> But foo is not necessarily a string, though I doubt someone will use it >>>> as non-string char array. >>> >>> Yeah, but the user defined comparison operand is NULL terminated. >>> So the strcmp will stop at this boundary. >>> >> The user input string is NULL terminated and is limited to MAX_FILTER_STR_VAL, >> and it's strcmp() not strcpy(), but it's still unsafe. No? >> >> cmp = strcmp(addr, pred->str_val); >> >> If addr is not NULL-terminated string but char array, and length of >> str_val > length of addr, then we'll be exceeding the boundary of the >> array. > > > > No, once both strings appear to be different, strcmp returns. > As an example, the generic strcmp in lib/string.c is as follows: > > int strcmp(const char *cs, const char *ct) > { > signed char __res; > > while (1) { > if ((__res = *cs - *ct++) != 0 || !*cs++) > break; > } > return __res; > } > > Once cs[n] != ct[n], or !cs[n] || !ct[n], strcmp() stops, > and the x86 implementation does exactly the same. > > So I guess it's safe. > See this example: cmp = strcmp(addr, pred->str_val); length(addr) == 6, strlen(str_val) == 10 123456 addr: abcdef? ^ | v str_val: abcdefzzzz\0 or the 2 happen to match even after addr overflowed: 123456 addr: abcdefzzzz? ^ | v str_val: abcdefzzzz\0