X86 platform drivers
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Sasha Levin' <sashal@kernel.org>, Greg KH <gregkh@linuxfoundation.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	Austin Kim <austindh.kim@gmail.com>,
	Dimitri Sivanich <dimitri.sivanich@hpe.com>,
	Hedi Berriche <hedi.berriche@hpe.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Mike Travis <mike.travis@hpe.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Russ Anderson <russ.anderson@hpe.com>,
	Steve Wahl <steve.wahl@hpe.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"allison@lohutok.net" <allison@lohutok.net>,
	"andy@infradead.org" <andy@infradead.org>,
	"armijn@tjaldur.nl" <armijn@tjaldur.nl>,
	"bp@alien8.de" <bp@alien8.de>,
	"dvhart@infradead.org" <dvhart@infradead.org>,
	"hpa@zytor.com" <hpa@zytor.com>, "kjlu@umn.edu" <kjlu@umn.edu>,
	platform-driver
Subject: RE: [PATCH AUTOSEL 5.3 169/203] x86/platform/uv: Fix kmalloc() NULL check routine
Date: Wed, 2 Oct 2019 08:34:08 +0000	[thread overview]
Message-ID: <ea163ee8ba4446978732c2c6607bd6da@AcuMS.aculab.com> (raw)
In-Reply-To: <20191001160601.GX8171@sasha-vm>

From: Sasha Levin
> Sent: 01 October 2019 17:06
> Subject: Re: [PATCH AUTOSEL 5.3 169/203] x86/platform/uv: Fix kmalloc() NULL check routine
> 
> On Sun, Sep 22, 2019 at 10:25:44PM +0200, Greg KH wrote:
> >On Sun, Sep 22, 2019 at 02:43:15PM -0400, Sasha Levin wrote:
> >> From: Austin Kim <austindh.kim@gmail.com>
> >>
> >> [ Upstream commit 864b23f0169d5bff677e8443a7a90dfd6b090afc ]
> >>
> >> The result of kmalloc() should have been checked ahead of below statement:
> >>
> >> 	pqp = (struct bau_pq_entry *)vp;
> >>
> >> Move BUG_ON(!vp) before above statement.
> >>
> >> Signed-off-by: Austin Kim <austindh.kim@gmail.com>
...
> >> ---
> >>  arch/x86/platform/uv/tlb_uv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
> >> index 20c389a91b803..5f0a96bf27a1f 100644
> >> --- a/arch/x86/platform/uv/tlb_uv.c
> >> +++ b/arch/x86/platform/uv/tlb_uv.c
> >> @@ -1804,9 +1804,9 @@ static void pq_init(int node, int pnode)
> >>
> >>  	plsize = (DEST_Q_SIZE + 1) * sizeof(struct bau_pq_entry);
> >>  	vp = kmalloc_node(plsize, GFP_KERNEL, node);
> >> -	pqp = (struct bau_pq_entry *)vp;
> >> -	BUG_ON(!pqp);
> >> +	BUG_ON(!vp);
> >>
> >> +	pqp = (struct bau_pq_entry *)vp;
> >>  	cp = (char *)pqp + 31;
> >>  	pqp = (struct bau_pq_entry *)(((unsigned long)cp >> 5) << 5);
> >>
> >
> >How did this even get merged in the first place?  I thought a number of
> >us complained about it.
> >
> >This isn't any change in code, and the original is just fine, the author
> >didn't realize how C works :(

Mind you, the code itself if pretty horrid.
Looks like it is aligning to 32 bytes, easier done by:
	pqp = (void *)((unsigned long)vp + 31 & ~31);
(and there's a roundup macro to obfuscate it somewhere.)
But I'd also expect to see a matching '+ 31' in the size passed to kmalloc().
Not to mention a comment!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  reply	other threads:[~2019-10-02  8:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190922184350.30563-1-sashal@kernel.org>
2019-09-22 18:43 ` [PATCH AUTOSEL 5.3 169/203] x86/platform/uv: Fix kmalloc() NULL check routine Sasha Levin
2019-09-22 20:25   ` Greg KH
2019-10-01 16:06     ` Sasha Levin
2019-10-02  8:34       ` David Laight [this message]
2019-10-02 14:35         ` Mike Travis
2019-09-22 18:43 ` [PATCH AUTOSEL 5.3 171/203] platform/x86: intel_pmc_core: Do not ioremap RAM Sasha Levin
2019-09-22 18:43 ` [PATCH AUTOSEL 5.3 172/203] platform/x86: intel_pmc_core_pltdrv: Module removal warning fix Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ea163ee8ba4446978732c2c6607bd6da@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=allison@lohutok.net \
    --cc=andy@infradead.org \
    --cc=armijn@tjaldur.nl \
    --cc=austindh.kim@gmail.com \
    --cc=bp@alien8.de \
    --cc=dimitri.sivanich@hpe.com \
    --cc=dvhart@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hedi.berriche@hpe.com \
    --cc=hpa@zytor.com \
    --cc=kjlu@umn.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.travis@hpe.com \
    --cc=peterz@infradead.org \
    --cc=russ.anderson@hpe.com \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=steve.wahl@hpe.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox