w1: ds28e17: possible out of bounds write from a device length byte

[Maoyi Xie]

Hi all,

I think w1_f19_i2c_master_transfer() in drivers/w1/slaves/w1_ds28e17.c can
write past the SMBus buffer when an I2C device behind the DS28E17 bridge
reports a large length. I would appreciate it if you could take a look.

On an I2C_M_RECV_LEN read the driver takes the length from the device.

	if (msgs[i].flags & I2C_M_RECV_LEN) {
		result = w1_f19_i2c_read(sl, msgs[i].addr,
			&(msgs[i].buf[1]), msgs[i].buf[0]);

buf[0] is the length byte a previous read filled in from the device, so it
can be anything from 0 to 255. w1_f19_i2c_read() only rejects a zero count
and then writes that many bytes into buf[1].

The buffer the SMBus core hands in is I2C_SMBUS_BLOCK_MAX + 2, so 34 bytes,
with msg.len set to 1. A device that reports a length above 32 makes the
read run past the 34 byte buffer, up to about 222 bytes out of bounds.

The SMBus emulation does check buf[0] against I2C_SMBUS_BLOCK_MAX, but that
runs after the transfer returns, so it cannot stop the write that already
happened. A compliant bit banging adapter rejects the over long length
before copying, in i2c-algo-bit.

The attacker here is a malicious or faulty I2C device sitting behind a
DS28E17 1-wire to I2C bridge. It controls the length byte it returns.

I reproduced it under KASAN on 7.1-rc7. With a length of 32 the write stays
inside the buffer. With a length of 255 KASAN reports a slab out of bounds
write past the 34 byte allocation.

The fix I tried rejects a length above I2C_SMBUS_BLOCK_MAX at the two
I2C_M_RECV_LEN sites, before the second read.

Does this look like a real bug to you? If it does I am happy to send a
proper patch with a Fixes tag pointing at ebc4768ac497 ("add w1_ds28e17
driver for the DS28E17 Onewire to I2C master bridge").

Thanks,
Maoyi
https://maoyixie.com/