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: Fri, 8 Mar 2019 23:32:53 +0100 From: Tomasz Duszynski <tduszyns@...il.com> To: Sven Van Asbroeck <thesven73@...il.com> Cc: Tomasz Duszynski <tduszyns@...il.com>, Jonathan Cameron <jic23@...nel.org>, Jonathan Cameron <jonathan.cameron@...wei.com>, Hartmut Knaack <knaack.h@....de>, Lars-Peter Clausen <lars@...afoo.de>, Peter Meerwald-Stadler <pmeerw@...erw.net>, linux-iio@...r.kernel.org, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Matt Ranostay <matt.ranostay@...sulko.com> Subject: Re: [PATCH v2] iio: proximity: as3935: fix use-after-free on device remove On Fri, Mar 08, 2019 at 03:42:04PM -0500, Sven Van Asbroeck wrote: > On Fri, Mar 8, 2019 at 3:30 PM Tomasz Duszynski <tduszyns@...il.com> wrote: > > > > > @@ -368,7 +376,6 @@ static int as3935_probe(struct spi_device *spi) > > > > > > spi_set_drvdata(spi, indio_dev); > > > mutex_init(&st->lock); > > > - INIT_DELAYED_WORK(&st->work, as3935_event_work); > > > > Any specific reason for moving this elsewhere? > > Yes. On the remove path, cancel_delayed_work_sync() should execute after > free_irq(), but before triggered_buffer_cleanup(). So the devm_add_action() > must run right before devm_request_irq(). I figured it would make sense to > group the devm_add_action() and INIT_WORK() together, as they are > related. This also makes it easier to understand the probe/remove order > when reading the code. > So perhaps that change deserves a separate patch because it smells like a code cleanup. > > > > > > ret = of_property_read_u32(np, > > > "ams,tuning-capacitor-pf", &st->tune_cap); > > > @@ -414,22 +421,27 @@ static int as3935_probe(struct spi_device *spi) > > > iio_trigger_set_drvdata(trig, indio_dev); > > > trig->ops = &iio_interrupt_trigger_ops; > > > > > > - ret = iio_trigger_register(trig); > > > + ret = devm_iio_trigger_register(&spi->dev, trig); > > > if (ret) { > > > dev_err(&spi->dev, "failed to register trigger\n"); > > > return ret; > > > } > > > > > > - ret = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time, > > > - &as3935_trigger_handler, NULL); > > > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, > > > + iio_pollfunc_store_time, as3935_trigger_handler, NULL); > > > > You can fix arguments alignment while you are at it. > > > > What type of alignment would you prefer? This? > > ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, > iio_pollfunc_store_time, > as3935_trigger_handler, NULL); Yes, this is what I've been thinking about. Thanks.
Powered by blists - more mailing lists