Dereferencing freed memory bugs

Dereferencing freed memory bugs

[Dan Carpenter]

I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
for when we dereference
freed memory.

drivers/dma/dmatest.c +410 dmatest_exit(7) 'dtc'
drivers/dma/dmatest.c +412 dmatest_exit(9) 'dtc'
drivers/infiniband/hw/nes/nes_cm.c +563 nes_cm_timer_tick(121) 'cm_node'
drivers/infiniband/hw/nes/nes_cm.c +621 nes_cm_timer_tick(179) 'cm_node'
drivers/scsi/dpt_i2o.c +246 adpt_detect(58) 'pHba'
drivers/scsi/dpt_i2o.c +266 adpt_detect(78) 'pHba'
drivers/scsi/dpt_i2o.c +1236 adpt_i2o_delete_hba(78) 'pHba'
drivers/usb/host/ehci-hcd.c +1661 itd_complete(79) 'stream'
drivers/usb/host/ehci-hcd.c +2036 sitd_complete(64) 'stream'
drivers/uwb/reset.c +193 __uwb_rc_cmd(26) 'cmd'
net/netfilter/nfnetlink_log.c +341 __nfulnl_flush(5) 'inst'
net/netfilter/xt_recent.c +273 recent_mt(69) 'e'
drivers/media/radio/radio-si470x.c +1144 si470x_fops_release(32) 'radio'
drivers/media/radio/radio-si470x.c +1722
si470x_usb_driver_disconnect(13) 'radio'
drivers/media/radio/radio-si470x.c +1144 si470x_fops_release(32) 'radio'
drivers/media/radio/radio-si470x.c +1722
si470x_usb_driver_disconnect(13) 'radio'
drivers/media/video/cpia_pp.c +777 cpia_pp_detach(28) 'cpia'
drivers/media/video/s2255drv.c +1711 s2255_destroy(42) 'dev'
drivers/mtd/mtd_blkdevs.c +389 register_mtd_blktrans(49) '*tr->blkcore_priv'
drivers/net/usb/hso.c +2616 hso_free_tiomget(5) 'tiocmget'

These mostly seem like real bugs.

regards,
dan carpenter

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter pisze:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> drivers/net/usb/hso.c +2616 hso_free_tiomget(5) 'tiocmget'

This error was added by 542f54823614915780c3459b0e6062f06c0c0f99:
tty: Modem functions for the HSO driver

Marcin

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter pisze:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> drivers/media/video/s2255drv.c +1711 s2255_destroy(42) 'dev'
> (...)

Broken by commit 14d962602c8bf86e63c9b9272be1f0360d0a448a:
V4L/DVB (8752): s2255drv: firmware improvement patch

[PATCH] mtd: fix use after free in register_mtd_blktrans

[Marcin Slusarz]

Dan Carpenter wrote:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> drivers/mtd/mtd_blkdevs.c +389 register_mtd_blktrans(49) '*tr->blkcore_priv'
> (...)

Fix:
---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH] mtd: fix use after free in register_mtd_blktrans

Reported-by: Dan Carpenter <error27@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/mtd/mtd_blkdevs.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 1409f01..4109e0b 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -382,11 +382,12 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 	tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,
 			"%sd", tr->name);
 	if (IS_ERR(tr->blkcore_priv->thread)) {
+		int ret = PTR_ERR(tr->blkcore_priv->thread);
 		blk_cleanup_queue(tr->blkcore_priv->rq);
 		unregister_blkdev(tr->major, tr->name);
 		kfree(tr->blkcore_priv);
 		mutex_unlock(&mtd_table_mutex);
-		return PTR_ERR(tr->blkcore_priv->thread);
+		return ret;
 	}
 
 	INIT_LIST_HEAD(&tr->devs);
-- 
1.6.0.6

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter pisze:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> drivers/media/radio/radio-si470x.c +1144 si470x_fops_release(32) 'radio'
> drivers/media/radio/radio-si470x.c +1722 si470x_usb_driver_disconnect(13) 'radio'
> (...)


Broken by commit ce5829e5fc8204af09db5b226a3dce9824e7d596:
V4L/DVB (7993): si470x: move global lock to device structure
Adding CCs.

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter pisze:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> net/netfilter/nfnetlink_log.c +341 __nfulnl_flush(5) 'inst'
> (...)

I think it's a false positive. In __nfulnl_flush we expect that caller
already holds a reference to inst and it looks like all callers do it.


Marcin

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter wrote:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> (...)
> drivers/scsi/dpt_i2o.c +246 adpt_detect(58) 'pHba'
> drivers/scsi/dpt_i2o.c +266 adpt_detect(78) 'pHba'
> drivers/scsi/dpt_i2o.c +1236 adpt_i2o_delete_hba(78) 'pHba'
> (...)

Very old bug - it predates git import (2.6.12-rc2).

Adding CCs.

Re: Dereferencing freed memory bugs

[Marcin Slusarz]

Dan Carpenter wrote:
> I added a check to smatch (http://repo.or.cz/w/smatch.git/) to check
> for when we dereference
> freed memory.
> 
> drivers/dma/dmatest.c +410 dmatest_exit(7) 'dtc'
> drivers/dma/dmatest.c +412 dmatest_exit(9) 'dtc'

Seems to be fixed by 7cbd4877e5b167b56a3d6033b926a9f925186e12:
dmatest: fix use after free in dmatest_exit

> drivers/infiniband/hw/nes/nes_cm.c +563 nes_cm_timer_tick(121) 'cm_node'
> drivers/infiniband/hw/nes/nes_cm.c +621 nes_cm_timer_tick(179) 'cm_node'
> (...)
> drivers/usb/host/ehci-hcd.c +1661 itd_complete(79) 'stream'
> drivers/usb/host/ehci-hcd.c +2036 sitd_complete(64) 'stream'
> drivers/uwb/reset.c +193 __uwb_rc_cmd(26) 'cmd'
> (...)
> net/netfilter/xt_recent.c +273 recent_mt(69) 'e'
> (...)
> drivers/media/video/cpia_pp.c +777 cpia_pp_detach(28) 'cpia'
> (...)

These are less obvious.

Adding CCs.
Please leave only one of openfabrics/linux-usb/netdev/linux-media in CCs
when responding.

ps: [s]itd_complete is in drivers/usb/host/ehci-sched.c