From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758100AbZFANJn (ORCPT ); Mon, 1 Jun 2009 09:09:43 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756605AbZFANJd (ORCPT ); Mon, 1 Jun 2009 09:09:33 -0400 Received: from ey-out-2122.google.com ([74.125.78.24]:59852 "EHLO ey-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755996AbZFANJd (ORCPT ); Mon, 1 Jun 2009 09:09:33 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=eS6RtSeWnnBVInJRV9qh2AiePFEverlTC3BzW9X1X/dbD1RFKeeXvpwgnAEK/5ujsN Ue4rj9tX2TIQdPPoWwtYY56u9Q2Ce8RNW5TDkVWEjhCXU7lSU0OaneSe5uVCCnVZgxNB oC3PoziTOOcZzicqSu3kIiFc8cnvJ5w66gqD8= Date: Mon, 1 Jun 2009 15:09:30 +0200 From: Frederic Weisbecker To: Li Zefan Cc: Steven Rostedt , Tom Zanussi , Ingo Molnar , LKML Subject: Re: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp() Message-ID: <20090601130928.GA6000@nowhere> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A236B0B.3000604@cn.fujitsu.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > > > >> Dynamic string is fine, because assign_str() makes it NULL-terminated. > >> > >> So we can use strcmp() for dynamic strings, but we'd better use strncmp() for > >> static string. > >> > >> > > > > > >