Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751427AbcLEHyD (ORCPT ); Mon, 5 Dec 2016 02:54:03 -0500 Received: from m12-18.163.com ([220.181.12.18]:35395 "EHLO m12-18.163.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750785AbcLEHyC (ORCPT ); Mon, 5 Dec 2016 02:54:02 -0500 From: Pan Bian To: Juergen Gross , Boris Ostrovsky , David Vrabel , xen-devel@lists.xenproject.org Cc: PanBian , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] xen: xenbus: set error code on failure Date: Mon, 5 Dec 2016 15:50:50 +0800 Message-Id: <20161205074728.GA19117@bp> X-Mailer: git-send-email 1.9.1 Reply-To: PanBian References: <1480762166-4883-1-git-send-email-bianpan2016@163.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mutt-References: X-Mutt-Fcc: =sent X-CM-TRANSID: EsCowACn+qRvHEVYHZyOEw--.3429S3 X-Coremail-Antispam: 1Uf129KBjvJXoW7Cw47uF4rZFWkAw4rAr47Arb_yoW8WF4fpr Z7Ka4YyFWUJ3W8Jr93Xr45ZF15Ca1kAF45CF98G347Zrsxur1vqF1SyFWj9F4DGrWFka1F yw4rW343ZFs8ArDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07bnKZXUUUUU= X-Originating-IP: [106.120.213.17] X-CM-SenderInfo: held01tdqsiiqw6rljoofrz/xtbBURU3claDsUsnMgAAs7 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1666 Lines: 52 From: PanBian On Mon, Dec 05, 2016 at 07:30:49AM +0100, Juergen Gross wrote: > On 03/12/16 11:49, Pan Bian wrote: > > In function xenstored_local_init(), the value of return variable err > > should be negative on errors. But the value of err keeps 0 even if the > > call to get_zeroed_page() returns a NULL pointer. This patch assigns > > "-ENOMEM" to err on the error branch. > > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188721 > > > > Signed-off-by: Pan Bian > > --- > > drivers/xen/xenbus/xenbus_probe.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > > index 33a31cf..f87d047 100644 > > --- a/drivers/xen/xenbus/xenbus_probe.c > > +++ b/drivers/xen/xenbus/xenbus_probe.c > > @@ -708,8 +708,10 @@ static int __init xenstored_local_init(void) > > > > /* Allocate Xenstore page */ > > page = get_zeroed_page(GFP_KERNEL); > > - if (!page) > > + if (!page) { > > + err = -ENOMEM; > > goto out_err; > > + } > > > > xen_store_gfn = xen_start_info->store_mfn = virt_to_gfn((void *)page); > > Why don't you preset err to -ENOMEM instead? Initializing it to 0 > is kind of pointless. I think presetting and setting on demand are both effective to fix this bug. Nevertheless, I will resubmit a second version if you insist. > > Ans while at it: preinitializing page isn't needed, too, and in the > error path testing page for being non-zero isn't neede either > (free_page() will do the right thing in case the parameter is 0). > > > Juergen > Thanks! Best regards, Pan