From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Am=C3=A9rico?= Wang Subject: Re: [PATCH] Check return from argv_split() in do_coredump(). Date: Fri, 26 Dec 2008 15:06:53 +0000 Message-ID: <20081226150653.GD3156@hack.private> References: <200812240616.mBO6GGEe078727@www262.sakura.ne.jp> <20081226144548.GB3156@hack.private> <200812260700.mBQ70uH3009595@www262.sakura.ne.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: xiyou.wangcong@gmail.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org To: Tetsuo Handa Return-path: Received: from ti-out-0910.google.com ([209.85.142.184]:64805 "EHLO ti-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750810AbYLZHIO convert rfc822-to-8bit (ORCPT ); Fri, 26 Dec 2008 02:08:14 -0500 Content-Disposition: inline In-Reply-To: <200812260700.mBQ70uH3009595@www262.sakura.ne.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Dec 26, 2008 at 04:00:56PM +0900, Tetsuo Handa wrote: >Hello. > >Am$(D+1rico Wang wrote: >> How about going to the line: >> >> current->fsuid = fsuid; >> >> ? Because when argv_split() fails, helper_argv is NULL and doesn't need >> to be checked again. > >I didn't understand what you say. I'm saying that >"do_coredump() may accesss helper_argv[0] when helper_argv == NULL", >which will result in "NULL pointer dereference" problem. >Yes, this problem unlikely happens. Thus, > >if (!helper_argv) > goto fail_unlock; > >may be enough. > Yes, goto fail_unlock will go to this line: if (helper_argv) argv_free(helper_argv); but in this situation, helper_argv is known as NULL, thus another check doesn't need. So we can go to the line below, i.e. fail_unlock: if (helper_argv) argv_free(helper_argv); current->fsuid = fsuid; //<=== goto this line coredump_finish(mm); You need to add a new label, of course. :) -- "Against stupidity, the gods themselves, contend in vain."