Messages in this thread | | | Date | Mon, 28 Jan 2019 07:53:59 -0600 | From | Corey Minyard <> | Subject | Re: [PATCH v3] ipmi_si: fix use-after-free of resource->name |
| |
On Mon, Jan 28, 2019 at 11:08:54AM +0800, Yang Yingliang wrote: > When we excute the following commands, we got oops > rmmod ipmi_si > cat /proc/ioports >
snip..
> > If io_setup is called successful in try_smi_init() but try_smi_init() > goes out_err before calling ipmi_register_smi(), so ipmi_unregister_smi() > will not be called while removing module. It leads to the resource that > allocated in io_setup() can not be freed, but the name(DEVICE_NAME) of > resource is freed while removing the module. It causes use-after-free > when cat /proc/ioports. > > Fix this by calling io_cleanup() while try_smi_init() goes to out_err. > and don't call io_cleanup() until io_setup() returns successful to avoid > warning prints.
Thanks a bunch for working on this. Fix is in my next tree now, if it is stable in there then I will send up to Linus.
-corey
> > Fixes: 93c303d2045b ("ipmi_si: Clean up shutdown a bit") > Cc: stable@vger.kernel.org > Reported-by: NuoHan Qiao <qiaonuohan@huawei.com> > Suggested-by: Corey Minyard <cminyard@mvista.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/char/ipmi/ipmi_si_intf.c | 5 +++++ > drivers/char/ipmi/ipmi_si_mem_io.c | 5 +++-- > drivers/char/ipmi/ipmi_si_port_io.c | 5 +++-- > 3 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c > index dc8603d..f1b9fda 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -2085,6 +2085,11 @@ static int try_smi_init(struct smi_info *new_smi) > WARN_ON(new_smi->io.dev->init_name != NULL); > > out_err: > + if (rv && new_smi->io.io_cleanup) { > + new_smi->io.io_cleanup(&new_smi->io); > + new_smi->io.io_cleanup = NULL; > + } > + > kfree(init_name); > return rv; > } > diff --git a/drivers/char/ipmi/ipmi_si_mem_io.c b/drivers/char/ipmi/ipmi_si_mem_io.c > index fd0ec8d..7558361 100644 > --- a/drivers/char/ipmi/ipmi_si_mem_io.c > +++ b/drivers/char/ipmi/ipmi_si_mem_io.c > @@ -81,8 +81,6 @@ int ipmi_si_mem_setup(struct si_sm_io *io) > if (!addr) > return -ENODEV; > > - io->io_cleanup = mem_cleanup; > - > /* > * Figure out the actual readb/readw/readl/etc routine to use based > * upon the register size. > @@ -141,5 +139,8 @@ int ipmi_si_mem_setup(struct si_sm_io *io) > mem_region_cleanup(io, io->io_size); > return -EIO; > } > + > + io->io_cleanup = mem_cleanup; > + > return 0; > } > diff --git a/drivers/char/ipmi/ipmi_si_port_io.c b/drivers/char/ipmi/ipmi_si_port_io.c > index ef6dffc..03924c3 100644 > --- a/drivers/char/ipmi/ipmi_si_port_io.c > +++ b/drivers/char/ipmi/ipmi_si_port_io.c > @@ -68,8 +68,6 @@ int ipmi_si_port_setup(struct si_sm_io *io) > if (!addr) > return -ENODEV; > > - io->io_cleanup = port_cleanup; > - > /* > * Figure out the actual inb/inw/inl/etc routine to use based > * upon the register size. > @@ -109,5 +107,8 @@ int ipmi_si_port_setup(struct si_sm_io *io) > return -EIO; > } > } > + > + io->io_cleanup = port_cleanup; > + > return 0; > } > -- > 1.8.3 > >
| |