linux-kernel - Re: [PATCH v2] irqchip: irq-mvebu-odmi: new driver for platform MSI on Marvell 7K/8K

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160219103535.28062901@free-electrons.com>
Date:	Fri, 19 Feb 2016 10:35:35 +0100
From:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:	Marc Zyngier <marc.zyngier@....com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	linux-kernel@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Nadav Haklai <nadavh@...vell.com>,
	Lior Amsalem <alior@...vell.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] irqchip: irq-mvebu-odmi: new driver for platform MSI
 on Marvell 7K/8K

Marc,

On Thu, 18 Feb 2016 16:41:23 +0000, Marc Zyngier wrote:

> It looks really nice, except for a couple of points, see below.

Thanks again for the review.

> > +/*
> > + * We don't support the group events, so we simply have 8 interrupts
> > + * per frame.
> > + */
> > +#define NODMIS_PER_FRAME 8
> > +#define NODMIS_SHIFT     3
> > +#define NODMIS_MASK      7
> 
> All these values are directly related, so it would be nice if they would
> be expressed in term of each other:
> 
> #define NODMIS_SHIFT	 3
> #define NODMIS_PER_FRAME (1 << NODMIS_SHIFT)
> #define NODMIS_MASK      (NODMIS_PER_FRAME - 1)

Good point, will do.

> > +	odmis_bm = kzalloc(odmis_count * NODMIS_PER_FRAME / BITS_PER_BYTE,
> > +			   GFP_KERNEL);
> 
> Blah. this will allocate the exact number of bytes, which you will then
> access as longs, touching memory that's not yours in the process (I've
> been recently bitten and publicly shamed...).

Aah, yes.

> Consider the following:
> 
> odmis_bm = kzalloc(BITS_TO_LONGS(odmis_count * NODMIS_PER_FRAME) *
> sizeof(long), GFP_KERNEL);

Will do.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com