Thread (4 messages) 4 messages, 2 authors, 2016-11-23

Re: net/can: use-after-free in bcm_rx_thr_flush

From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: 2016-11-22 17:29:58
Also in: linux-can, lkml

Hi Andrey,

thanks for the report.

Although I can't see the issue in the code ...

On 11/22/2016 10:22 AM, Andrey Konovalov wrote:
==================================================================
BUG: KASAN: use-after-free in bcm_rx_thr_flush+0x284/0x2b0
Read of size 1 at addr ffff88006c1faae5 by task a.out/3874

page:ffffea0001b07e80 count:1 mapcount:0 mapping:          (null) index:0x0
flags: 0x100000000000080(slab)
page dumped because: kasan: bad access detected
(..)
The buggy address belongs to the object at ffff88006c1faae0
 which belongs to the cache kmalloc-32 of size 32
???
The buggy address ffff88006c1faae5 is located 5 bytes inside
 of 32-byte region [ffff88006c1faae0, ffff88006c1fab00)
(..)
Memory state around the buggy address:
 ffff88006c1fa980: fc fc fb fb fb fb fc fc fb fb fb fb fc fc fb fb
 ffff88006c1faa00: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
quoted
ffff88006c1faa80: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
                                                       ^
 ffff88006c1fab00: fc fc fb fb fb fb fc fc 00 00 00 00 fc fc 00 00
 ffff88006c1fab80: 00 00 fc fc fb fb fb fb fc fc fb fb fb fb fc fc
==================================================================
(should be some zero initialized memory here)

The relevant code of bcm_rx_do_flush() can be found here:

http://lxr.free-electrons.com/source/net/can/bcm.c#L589

static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
				  unsigned int index)
{
	struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;

	if ((op->last_frames) && (lcf->flags & RX_THR)) {  <<<----- !!!
		if (update)
			bcm_rx_changed(op, lcf);
		return 1;
	}
	return 0;
}


lcf->flags points into an array of struct canfd_frame at offset 5 which 
is allocated here:

http://lxr.free-electrons.com/source/net/can/bcm.c#L1105

	/* create and init array for received CAN frames */
	op->last_frames = kzalloc(msg_head->nframes * op->cfsiz,
				  GFP_KERNEL);

So why does KASAN complain about accessing some kind of 32 byte cache 
when it should point into a zero initialized allocated space?

I will write some other test cases with a similar setting of options to 
check if I can trigger the instability too.

Tnx & regards,
Oliver
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help