All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Branden <scott.branden@broadcom.com>
To: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>,
	Dhananjay Phadke <dphadke@linux.microsoft.com>
Cc: linux-i2c <linux-i2c@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Wolfram Sang <wsa@kernel.org>, Ray Jui <rjui@broadcom.com>,
	BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>
Subject: Re: [PATCH] i2c: iproc: fix race between client unreg and isr
Date: Mon, 20 Jul 2020 10:40:16 -0700	[thread overview]
Message-ID: <f7397069-e22b-a921-132c-144ee3a07580@broadcom.com> (raw)
In-Reply-To: <CAHO=5PFGqyacnneCp76brDymyZK0axG+4n3RPThFp6w=-xP7hA@mail.gmail.com>

looks good.

On 2020-07-19 12:59 a.m., Rayagonda Kokatanur wrote:
> On Sun, Jul 19, 2020 at 5:10 AM Dhananjay Phadke
> <dphadke@linux.microsoft.com> wrote:
>> When i2c client unregisters, synchronize irq before setting
>> iproc_i2c->slave to NULL.
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000318
>>
>> [  371.020421] pc : bcm_iproc_i2c_isr+0x530/0x11f0
>> [  371.025098] lr : __handle_irq_event_percpu+0x6c/0x170
>> [  371.030309] sp : ffff800010003e40
>> [  371.033727] x29: ffff800010003e40 x28: 0000000000000060
>> [  371.039206] x27: ffff800010ca9de0 x26: ffff800010f895df
>> [  371.044686] x25: ffff800010f18888 x24: ffff0008f7ff3600
>> [  371.050165] x23: 0000000000000003 x22: 0000000001600000
>> [  371.055645] x21: ffff800010f18888 x20: 0000000001600000
>> [  371.061124] x19: ffff0008f726f080 x18: 0000000000000000
>> [  371.066603] x17: 0000000000000000 x16: 0000000000000000
>> [  371.072082] x15: 0000000000000000 x14: 0000000000000000
>> [  371.077561] x13: 0000000000000000 x12: 0000000000000001
>> [  371.083040] x11: 0000000000000000 x10: 0000000000000040
>> [  371.088519] x9 : ffff800010f317c8 x8 : ffff800010f317c0
>> [  371.093999] x7 : ffff0008f805b3b0 x6 : 0000000000000000
>> [  371.099478] x5 : ffff0008f7ff36a4 x4 : ffff8008ee43d000
>> [  371.104957] x3 : 0000000000000000 x2 : ffff8000107d64c0
>> [  371.110436] x1 : 00000000c00000af x0 : 0000000000000000
>>
>> [  371.115916] Call trace:
>> [  371.118439]  bcm_iproc_i2c_isr+0x530/0x11f0
>> [  371.122754]  __handle_irq_event_percpu+0x6c/0x170
>> [  371.127606]  handle_irq_event_percpu+0x34/0x88
>> [  371.132189]  handle_irq_event+0x40/0x120
>> [  371.136234]  handle_fasteoi_irq+0xcc/0x1a0
>> [  371.140459]  generic_handle_irq+0x24/0x38
>> [  371.144594]  __handle_domain_irq+0x60/0xb8
>> [  371.148820]  gic_handle_irq+0xc0/0x158
>> [  371.152687]  el1_irq+0xb8/0x140
>> [  371.155927]  arch_cpu_idle+0x10/0x18
>> [  371.159615]  do_idle+0x204/0x290
>> [  371.162943]  cpu_startup_entry+0x24/0x60
>> [  371.166990]  rest_init+0xb0/0xbc
>> [  371.170322]  arch_call_rest_init+0xc/0x14
>> [  371.174458]  start_kernel+0x404/0x430
>>
>> Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave mode")
>> Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
Acked-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>>   drivers/i2c/busses/i2c-bcm-iproc.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
>> index b58224b7b..37d2a79e7 100644
>> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
>> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
>> @@ -1074,14 +1074,15 @@ static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave)
>>          if (!iproc_i2c->slave)
>>                  return -EINVAL;
>>
>> -       iproc_i2c->slave = NULL;
>> -
>>          /* disable all slave interrupts */
>>          tmp = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
>>          tmp &= ~(IE_S_ALL_INTERRUPT_MASK <<
>>                          IE_S_ALL_INTERRUPT_SHIFT);
>>          iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, tmp);
>>
>> +       synchronize_irq(iproc_i2c->irq);
>> +       iproc_i2c->slave = NULL;
>> +
>>          /* Erase the slave address programmed */
>>          tmp = iproc_i2c_rd_reg(iproc_i2c, S_CFG_SMBUS_ADDR_OFFSET);
>>          tmp &= ~BIT(S_CFG_EN_NIC_SMB_ADDR3_SHIFT);
> Looks good to me. Thank you for the patch.
> Reviewed-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
>
> Regards,
> Rayagonda


  reply	other threads:[~2020-07-20 17:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-18 23:39 [PATCH] i2c: iproc: fix race between client unreg and isr Dhananjay Phadke
2020-07-19  7:59 ` Rayagonda Kokatanur
2020-07-20 17:40   ` Scott Branden [this message]
2020-07-20 17:49 ` Ray Jui
2020-07-22 10:41   ` Wolfram Sang
2020-07-22 15:51     ` Ray Jui
2020-07-22 16:55       ` Ray Jui
2020-07-23  0:58       ` Dhananjay Phadke
2020-07-25 10:18       ` Wolfram Sang
2020-07-27  4:32         ` Rayagonda Kokatanur
2020-07-27 15:42         ` Ray Jui
2020-07-27 17:38           ` Dhananjay Phadke
2020-07-27 18:13           ` Wolfram Sang
2020-07-27 20:26             ` Wolfram Sang
2020-07-27 20:43               ` Ray Jui
2020-08-05  9:17                 ` Wolfram Sang
2020-08-07 16:40                   ` Ray Jui
2020-08-07 17:38                     ` Dhananjay Phadke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f7397069-e22b-a921-132c-144ee3a07580@broadcom.com \
    --to=scott.branden@broadcom.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=dphadke@linux.microsoft.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rayagonda.kokatanur@broadcom.com \
    --cc=rjui@broadcom.com \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.