Eric Anholt [off-list ref] hat am 27. Dezember 2018 um 19:15 geschrieben:


Stefan Wahren [off-list ref] writes:

quoted

Hi Paul,

quoted

Paul Kocialkowski [off-list ref] hat am 24. Dezember 2018 um 10:10 geschrieben:


Hi,

On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:

quoted

Hi Paul,

quoted

Paul Kocialkowski [off-list ref] hat am 21. Dezember 2018 um 13:11 geschrieben:


The driver's interrupt handler checks whether a message is currently
being handled with the curr_msg pointer. When it is NULL, the interrupt
is considered to be unexpected. Similarly, the i2c_start_transfer
routine checks for the remaining number of messages to handle in
num_msgs.

However, these values are never cleared and always keep the message and
number relevant to the latest transfer (which might be done already and
the underlying message memory might have been freed).

When an unexpected interrupt hits with the DONE bit set, the isr will
then try to access the flags field of the curr_msg structure, leading
to a fatal page fault.

Fix the issue by systematically clearing curr_msg and num_msgs in the
driver-wide device structure when a transfer is considered complete.

Signed-off-by: Paul Kocialkowski <redacted>
---
 drivers/i2c/busses/i2c-bcm2835.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 44deae78913e..5486252f5f2f 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c

@@ -298,6 +298,9 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 		return -ETIMEDOUT;
 	}
 
+	i2c_dev->curr_msg = NULL;
+	i2c_dev->num_msgs = 0;

AFAIU this would reduce the chance of a use-after-free dramatically but not completely.

Do you have a specific case of use-after-free related to these
variables in mind that this cleanup would not fix (except for the
timeout case)?

okay i was wrong about the use-after-free. But i'm not sure about this scenario on the I2C bus shared with the VC4:

1. ARM core starts I2C transfer (bcm2835_i2c_start_transfer)
2. VC4 triggers a BCM2835_I2C_S_DONE interrupt
3. ARM core catches this interrupt before the VC4

We don't share I2C buses with VC4, though, right?  That would just be
totally broken.

All i remember is this comment on Github:
https://github.com/anholt/linux/issues/89#issuecomment-280111496

and currently all 3 I2C busses are enabled for the ARM core in bcm2835-rpi.dtsi.

Stefan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel