From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Wahren Subject: Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Date: Tue, 21 Feb 2017 09:58:16 +0100 Message-ID: <31b96f29-e369-546a-6270-266daea71062@i2se.com> References: <1487280047-29608-1-git-send-email-stefan.wahren@i2se.com> <1487280047-29608-2-git-send-email-stefan.wahren@i2se.com> <48907a31-eaa6-27e2-633f-d36de521e868@tronnes.org> <1983281189.110224.1487619653742@email.1und1.de> <5b14ed6d-4db8-abf7-ceba-ef46534b6023@tronnes.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= , Peter Robinson , Michael Zoran , Eric Anholt Cc: Will Deacon , Frank Rowand , Martin Sperl , Rob Herring , linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Catalin Marinas , Florian Fainelli , linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Wolfram Sang List-Id: devicetree@vger.kernel.org Am 20.02.2017 um 22:22 schrieb Noralf Trønnes: > > Den 20.02.2017 20.40, skrev Stefan Wahren: >> Hi, >> >>> Noralf Trønnes hat am 18. Februar 2017 um 19:34 >>> geschrieben: >>> >>> >>> >>> Den 16.02.2017 22.20, skrev Stefan Wahren: >>>> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >>>> larger than 16 bytes") the interrupt handler is prone to a possible >>>> NULL pointer dereference. This could happen if an interrupt fires >>>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >>>> on the RPi 3. Even this is an unexpected behavior the driver must >>>> handle that with an error instead of a crash. >>>> >>>> CC: Noralf Trønnes >>>> CC: Martin Sperl >>>> Reported-by: Peter Robinson >>>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger >>>> than 16 bytes") >>>> Signed-off-by: Stefan Wahren >>>> --- >>>> drivers/i2c/busses/i2c-bcm2835.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c >>>> b/drivers/i2c/busses/i2c-bcm2835.c >>>> index c3436f6..10e39c8 100644 >>>> --- a/drivers/i2c/busses/i2c-bcm2835.c >>>> +++ b/drivers/i2c/busses/i2c-bcm2835.c >>>> @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int >>>> this_irq, void *data) >>>> } >>>> if (val & BCM2835_I2C_S_DONE) { >>>> - if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>> + if (!i2c_dev->curr_msg) { >>>> + dev_err(i2c_dev->dev, "Got unexpected interrupt (from >>>> firmware?)\n"); >>>> + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>> bcm2835_drain_rxfifo(i2c_dev); >>>> val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); >>>> } >>> Thanks, >>> >>> Acked-by: Noralf Trønnes >>> >> thanks, but i would be more happier to receive feedback for patches 2 >> and 3. > > I have only worked on dts files downstream and never done any arm64 > stuff, so I'm not up to speed on this. > > Noralf. > Okay, at the end i only want to know 2 things: Does new message "Got unexpected interrupt" still appear after applying all 3 patches? Does the annoying message "i2c-bcm2835 3f805000.i2c: i2c transfer timed out" still appear after applying all 3 patches? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html