On Thu, Aug 09, 2018 at 10:17:17AM -0700, Stephen Boyd wrote: > Both callers of coreboot_table_init() ioremap the pointer that comes in > but they don't unmap the memory on failure. Both of them also fail probe > immediately with the return value of coreboot_table_init(), leaking a > mapping when it fails. Plug the leak so the mapping isn't left unused. > > Cc: Wei-Ning Huang <wnhu...@chromium.org> > Cc: Julius Werner <jwer...@chromium.org> > Cc: Brian Norris <briannor...@chromium.org> > Cc: Samuel Holland <sam...@sholland.org> > Fixes: 570d30c2823f ("firmware: coreboot: Expose the coreboot table as a bus")
I suppose this is fair, since that commit introduced error paths and didn't clean them up. But one warning below: > Signed-off-by: Stephen Boyd <swb...@chromium.org> > --- > drivers/firmware/google/coreboot_table.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/firmware/google/coreboot_table.c > b/drivers/firmware/google/coreboot_table.c > index 19db5709ae28..0d3e140444ae 100644 > --- a/drivers/firmware/google/coreboot_table.c > +++ b/drivers/firmware/google/coreboot_table.c > @@ -138,6 +138,9 @@ int coreboot_table_init(struct device *dev, void __iomem > *ptr) > ptr_entry += entry.size; > } > > + if (ret) > + iounmap(ptr); This works because no sub-driver is using this mapping any more (i.e., because we killed coreboot_table_find()). Otherwise, we'd need to explicitly kill all the sub-devices first. IOW, if this gets backported to older kernels, it would need to go along with this and its other dependencies: b616cf53aa7a firmware: coreboot: Remove unused coreboot_table_find But I guess that's a question for -stable. Or, we remove the 'Fixes' tag? Or add another tag, to list other dependencies? Or just ignore it. But for this change as applied to mainline: Reviewed-by: Brian Norris <briannor...@chromium.org> > + > return ret; > } > EXPORT_SYMBOL(coreboot_table_init); > -- > Sent by a computer through tubes >