[PATCH] i2c: bcm2835: Clear current message and count after a transaction

[PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Paul Kocialkowski]

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 <paul.kocialkowski@bootlin.com>
---
 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;
+
 	if (!i2c_dev->msg_err)
 		return num;
 
-- 
2.20.1

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Florian Fainelli]

On 12/21/18 4:11 AM, Paul Kocialkowski wrote:
> 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.

Should not this get a Fixes tag?

> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  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;
> +
>  	if (!i2c_dev->msg_err)
>  		return num;
>  
> 


-- 
Florian

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Paul Kocialkowski]

Hi,

On Fri, 2018-12-21 at 09:52 -0800, Florian Fainelli wrote:
> On 12/21/18 4:11 AM, Paul Kocialkowski wrote:
> > 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.
> 
> Should not this get a Fixes tag?

Yes it totally should! Thanks for the suggestion.

Cheers,

Paul

> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  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;
> > +
> >  	if (!i2c_dev->msg_err)
> >  		return num;
> >  
> > 
> 
> 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Stefan Wahren]

Hi Paul,

> Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
> ---
>  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.

Btw: Why did you leave of the i2c transfer timeout case?

Thanks
Stefan

> +
>  	if (!i2c_dev->msg_err)
>  		return num;
>  
> -- 
> 2.20.1
>

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Paul Kocialkowski]

Hi,

On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:
> Hi Paul,
> 
> > Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
> > ---
> >  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)?

The msg_buf element (in association with msg_buf_remaining keeping its
previous value) could also lead to similar problems, so I'm thinking of
making a bcm2835_i2c_finish_transfer function to group all the cleanups
and call that right after wait_for_completion_timeout.

> Btw: Why did you leave of the i2c transfer timeout case?

I should definitely have included it, actually.

Cheers,

Paul

> Thanks
> Stefan
> 
> > +
> >  	if (!i2c_dev->msg_err)
> >  		return num;
> >  
> > -- 
> > 2.20.1
> > 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Stefan Wahren]

Hi Paul,

> Paul Kocialkowski <paul.kocialkowski@bootlin.com> hat am 24. Dezember 2018 um 10:10 geschrieben:
> 
> 
> Hi,
> 
> On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:
> > Hi Paul,
> > 
> > > Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
> > > ---
> > >  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

Stefan

> 
> The msg_buf element (in association with msg_buf_remaining keeping its
> previous value) could also lead to similar problems, so I'm thinking of
> making a bcm2835_i2c_finish_transfer function to group all the cleanups
> and call that right after wait_for_completion_timeout.
> 
> > Btw: Why did you leave of the i2c transfer timeout case?
> 
> I should definitely have included it, actually.
> 
> Cheers,
> 
> Paul
> 
> > Thanks
> > Stefan
> > 
> > > +
> > >  	if (!i2c_dev->msg_err)
> > >  		return num;
> > >  
> > > -- 
> > > 2.20.1
> > > 
> -- 
> Paul Kocialkowski, Bootlin (formerly Free Electrons)
> Embedded Linux and kernel engineering
> https://bootlin.com
>

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Paul Kocialkowski]

Hi Stefan,

On Thu, 2018-12-27 at 15:05 +0100, Stefan Wahren wrote:
> Hi Paul,
> 
> > Paul Kocialkowski <paul.kocialkowski@bootlin.com> hat am 24. Dezember 2018 um 10:10 geschrieben:
> > 
> > 
> > Hi,
> > 
> > On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:
> > > Hi Paul,
> > > 
> > > > Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
> > > > ---
> > > >  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

Well, I thought the VC4 has precedence over the ARM core, so if the
interrupts hits in hardware, it should be seen by the VC4 first and
forwarded to the ARM core if needed (I might be wrong though). So in
that case, perhaps the ARM core wouldn't see the interrupt and just
timeout?

Either way, I'm don't think that having both the VC4 and the ARM core
poke on the same bus is a scenario we can realisticly aim to support.
Although it should definitely not make our driver crash if that
happens.

The HDMI DDC bus is an I2C bus shared between the ARM core and VC4, but
the VC4 firmware should detect from the device-tree that Linux will
attempt to drive the bus and let the ARM core deal with it without
interference.

Thanks for your feedback!

Cheers,

Paul

> > The msg_buf element (in association with msg_buf_remaining keeping its
> > previous value) could also lead to similar problems, so I'm thinking of
> > making a bcm2835_i2c_finish_transfer function to group all the cleanups
> > and call that right after wait_for_completion_timeout.
> > 
> > > Btw: Why did you leave of the i2c transfer timeout case?
> > 
> > I should definitely have included it, actually.
> > 
> > Cheers,
> > 
> > Paul
> > 
> > > Thanks
> > > Stefan
> > > 
> > > > +
> > > >  	if (!i2c_dev->msg_err)
> > > >  		return num;
> > > >  
> > > > -- 
> > > > 2.20.1
> > > > 
> > -- 
> > Paul Kocialkowski, Bootlin (formerly Free Electrons)
> > Embedded Linux and kernel engineering
> > https://bootlin.com
> > 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Paul,
>
>> Paul Kocialkowski <paul.kocialkowski@bootlin.com> hat am 24. Dezember 2018 um 10:10 geschrieben:
>> 
>> 
>> Hi,
>> 
>> On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:
>> > Hi Paul,
>> > 
>> > > Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
>> > > ---
>> > >  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] i2c: bcm2835: Clear current message and count after a transaction

[Stefan Wahren]

> Eric Anholt <eric@anholt.net> hat am 27. Dezember 2018 um 19:15 geschrieben:
> 
> 
> Stefan Wahren <stefan.wahren@i2se.com> writes:
> 
> > Hi Paul,
> >
> >> Paul Kocialkowski <paul.kocialkowski@bootlin.com> hat am 24. Dezember 2018 um 10:10 geschrieben:
> >> 
> >> 
> >> Hi,
> >> 
> >> On Sat, 2018-12-22 at 13:19 +0100, Stefan Wahren wrote:
> >> > Hi Paul,
> >> > 
> >> > > Paul Kocialkowski <paul.kocialkowski@bootlin.com> 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 <paul.kocialkowski@bootlin.com>
> >> > > ---
> >> > >  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