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
| ||
|
Date: Tue, 23 May 2017 19:54:37 +0200 From: Mason <slash.tmp@...e.fr> To: Bjorn Helgaas <helgaas@...nel.org> Cc: Marc Gonzalez <marc_gonzalez@...madesigns.com>, Marc Zyngier <marc.zyngier@....com>, Thomas Gleixner <tglx@...utronix.de>, Jiang Liu <jiang.liu@...ux.intel.com>, Robin Murphy <robin.murphy@....com>, Lorenzo Pieralisi <lorenzo.pieralisi@....com>, Liviu Dudau <liviu.dudau@....com>, David Laight <david.laight@...lab.com>, linux-pci <linux-pci@...r.kernel.org>, Linux ARM <linux-arm-kernel@...ts.infradead.org>, Thibaud Cornic <thibaud_cornic@...madesigns.com>, Phuong Nguyen <phuong_nguyen@...madesigns.com>, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v4 1/2] PCI: Add tango MSI controller support On 23/05/2017 19:03, Bjorn Helgaas wrote: > On Wed, May 17, 2017 at 04:56:08PM +0200, Marc Gonzalez wrote: >> On 20/04/2017 16:28, Marc Gonzalez wrote: >> >>> +static int tango_set_affinity(struct irq_data *data, >>> + const struct cpumask *mask, bool force) >>> +{ >>> + return -EINVAL; >>> +} >>> + >>> +static struct irq_chip tango_chip = { >>> + .irq_ack = tango_ack, >>> + .irq_mask = tango_mask, >>> + .irq_unmask = tango_unmask, >>> + .irq_set_affinity = tango_set_affinity, >>> + .irq_compose_msi_msg = tango_compose_msi_msg, >>> +}; >> >> Hmmm... I'm wondering why .irq_set_affinity is required. >> >> static int setup_affinity(struct irq_desc *desc, struct cpumask *mask) >> first calls __irq_can_set_affinity() to check whether >> desc->irq_data.chip->irq_set_affinity) exists. >> >> then calls irq_do_set_affinity(&desc->irq_data, mask, false); >> which calls chip->irq_set_affinity(data, mask, force); >> = msi_domain_set_affinity() >> which calls parent->chip->irq_set_affinity() unconditionally. >> >> Would it make sense to test that the callback is implemented >> before calling it? >> >> >> [ 0.723895] Unable to handle kernel NULL pointer dereference at virtual address 00000000 > > I'm not sure what you're asking. > > Is this a bug report for the v4 tango driver? No. > Or are you asking whether msi_domain_set_affinity() should be changed > to check whether parent->chip->irq_set_affinity is implemented? Yes. The way things are implemented now, drivers are forced to define an irq_set_affinity callback, even if it just returns an error, otherwise, the kernel crashes, because of the unconditional function pointer deref. > msi_domain_set_affinity() has called parent->chip->irq_set_affinity() > without checking since it was added in 2014 by f3cf8bb0d6c3 ("genirq: Add > generic msi irq domain support"), so if there's a problem here, it's most > likely in the tango code. The issue is having to define an "empty" function. (Unnecessary code bloat and maintenance.) I'll send a patch illustrating exactly what I intended. Regards.
Powered by blists - more mailing lists