Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 14 Mar 2020 21:35:00 +0000
From:   Sudip Mukherjee <sudipm.mukherjee@gmail.com>
To:     Paul Wise <pabs3@bonedaddy.net>, viro@zeniv.linux.org.uk,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     Matthew Ruffell <matthew.ruffell@canonical.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Neil Horman <nhorman@tuxdriver.com>,
        Jakub Wilk <jwilk@jwilk.net>
Subject: Re: [PATCH 0/1] coredump: Fix null pointer dereference when
 kernel.core_pattern is "|"
Message-ID: <20200314213500.s7y4wyok2lfc4w6f@debian>
References: <20200220051015.14971-1-matthew.ruffell@canonical.com>
 <645fcbdfdd1321ff3e0afaafe7eccfd034e57748.camel@bonedaddy.net>
 <87a47997-3cde-bc86-423b-6154849183e9@canonical.com>
 <fa636317af3a38badff322ca11e437701154b1be.camel@bonedaddy.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <fa636317af3a38badff322ca11e437701154b1be.camel@bonedaddy.net>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Paul,

On Sat, Mar 14, 2020 at 08:28:10AM +0800, Paul Wise wrote:
> On Tue, 2020-03-10 at 11:34 +1300, Matthew Ruffell wrote:
> 
> > Can I please get some feedback on this patch? Would be good to clear
> > up the null pointer dereference.

I could reproduce the problem very easily, though the core_pattern is
not supposed to be used that way. Only "|" is invalid core_pattern. But
in anycase I think the kernel should have a check for this invalid
usecase.

> 
> I had a thought about it, instead of using strlen, what about checking
> that the first item in the array is NUL or not? In the normal case this
> should be faster than strlen.

Why are you checking the corename in do_coredump() after it has done
almost everything? It can be very easily checked in format_corename().
Something like the following:

diff --git a/fs/coredump.c b/fs/coredump.c
index b1ea7dfbd149..d25bad2ed061 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -211,6 +211,8 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
 			return -ENOMEM;
 		(*argv)[(*argc)++] = 0;
 		++pat_ptr;
+		if (!(*pat_ptr))
+			return -ENOMEM;
 	}
 
 	/* Repeat as long as we have more pattern to process and more output


--
Regards
Sudip