public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Guilherme G. Piccoli" <gpiccoli@linux•vnet.ibm.com>
To: Michael Chan <michael.chan@broadcom•com>
Cc: Siva Reddy Kallam <siva.kallam@broadcom•com>,
	Prashant Sreedharan <prashant@broadcom•com>,
	Michael Chan <mchan@broadcom•com>,
	Netdev <netdev@vger•kernel.org>,
	Milton Miller <miltonm@us•ibm.com>
Subject: Re: [PATCH net] tg3: Avoid NULL pointer dereference in tg3_io_error_detected()
Date: Tue, 27 Sep 2016 18:27:17 -0300	[thread overview]
Message-ID: <57EAE435.4000905@linux.vnet.ibm.com> (raw)
In-Reply-To: <CACKFLi=MVovAMnZwTe3Swb1+hgy+h9CfTURPVrZsM0MJGV9Khw@mail.gmail.com>

On 09/27/2016 05:58 PM, Michael Chan wrote:
> On Tue, Sep 27, 2016 at 1:05 PM, Guilherme G. Piccoli
> <gpiccoli@linux•vnet.ibm.com> wrote:
>> From: Milton Miller <miltonm@us•ibm.com>
>>
>> While the driver is probing the adapter, an error may occur before the
>> netdev structure is allocated and attached to pci_dev. In this case,
>> not only netdev isn't available, but the tg3 private structure is also
>> not available as it is just math from the NULL pointer, so dereferences
>> must be skipped.
>>
>> The following trace is seen when the error is triggered:
>>
>>    [1.402247] Unable to handle kernel paging request for data at address 0x00001a99
>>    [1.402410] Faulting instruction address: 0xc0000000007e33f8
>>    [1.402450] Oops: Kernel access of bad area, sig: 11 [#1]
>>    [1.402481] SMP NR_CPUS=2048 NUMA PowerNV
>>    [1.402513] Modules linked in:
>>    [1.402545] CPU: 0 PID: 651 Comm: eehd Not tainted 4.4.0-36-generic #55-Ubuntu
>>    [1.402591] task: c000001fe4e42a20 ti: c000001fe4e88000 task.ti: c000001fe4e88000
>>    [1.402742] NIP: c0000000007e33f8 LR: c0000000007e3164 CTR: c000000000595ea0
>>    [1.402787] REGS: c000001fe4e8b790 TRAP: 0300   Not tainted  (4.4.0-36-generic)
>>    [1.402832] MSR: 9000000100009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 28000422  XER: 20000000
>>    [1.403058] CFAR: c000000000008468 DAR: 0000000000001a99 DSISR: 42000000 SOFTE: 1
>>    GPR00: c0000000007e3164 c000001fe4e8ba10 c0000000015c5e00 0000000000000000
>>    GPR04: 0000000000000001 0000000000000000 0000000000000039 0000000000000299
>>    GPR08: 0000000000000000 0000000000000001 c000001fe4e88000 0000000000000006
>>    GPR12: 0000000000000000 c00000000fb40000 c0000000000e6558 c000003ca1bffd00
>>    GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>    GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000000d52768
>>    GPR24: c000000000d52740 0000000000000100 c000003ca1b52000 0000000000000002
>>    GPR28: 0000000000000900 0000000000000000 c00000000152a0c0 c000003ca1b52000
>>    [1.404226] NIP [c0000000007e33f8] tg3_io_error_detected+0x308/0x340
>>    [1.404265] LR [c0000000007e3164] tg3_io_error_detected+0x74/0x340
>>
>> This patch avoids the NULL pointer dereference by moving the access after
>> the netdev NULL pointer check on tg3_io_error_detected().
>>
>> Fixes: 0486a063b1ff ("tg3: prevent ifup/ifdown during PCI error recovery")
>> Fixes: dfc8f370316b ("net/tg3: Release IRQs on permanent error")
>> Tested-by: Guilherme G. Piccoli <gpiccoli@linux•vnet.ibm.com>
>> Signed-off-by: Milton Miller <miltonm@us•ibm.com>
>> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux•vnet.ibm.com>
>
> Looks good.  Do we need to add !netdev check in tg3_io_resume()?

Thanks Michael. It's a good point - I didn't trigger any error without 
the check, but looking at error handlers, every one seems to have this 
check except tg3_io_resume().

Do you want us to send a v2 including this check? Or maybe another patch?

Cheers,



Guilherme

  reply	other threads:[~2016-09-27 21:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-27 20:05 [PATCH net] tg3: Avoid NULL pointer dereference in tg3_io_error_detected() Guilherme G. Piccoli
2016-09-27 20:58 ` Michael Chan
2016-09-27 21:27   ` Guilherme G. Piccoli [this message]
2016-09-27 21:42     ` Michael Chan

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=57EAE435.4000905@linux.vnet.ibm.com \
    --to=gpiccoli@linux$(echo .)vnet.ibm.com \
    --cc=mchan@broadcom$(echo .)com \
    --cc=michael.chan@broadcom$(echo .)com \
    --cc=miltonm@us$(echo .)ibm.com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=prashant@broadcom$(echo .)com \
    --cc=siva.kallam@broadcom$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox