linux-kernel - Re: [PATCH] mwifiex: fix possible NULL dereference

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHp75VfACJiVvdtA4Ln9nXq8U7i=7mX17YYq6X-=H3aFS8EyBg@mail.gmail.com>
Date:	Wed, 13 Apr 2016 01:15:19 +0300
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	"Rustad, Mark D" <mark.d.rustad@...el.com>
Cc:	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Amitkumar Karwar <akarwar@...vell.com>,
	Nishant Sarmukadam <nishants@...vell.com>,
	Kalle Valo <kvalo@...eaurora.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"open list:TI WILINK WIRELES..." <linux-wireless@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>,
	Sudip Mukherjee <sudip.mukherjee@...ethink.co.uk>
Subject: Re: [PATCH] mwifiex: fix possible NULL dereference

On Tue, Apr 12, 2016 at 8:43 PM, Rustad, Mark D <mark.d.rustad@...el.com> wrote:
> Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>
>> On Mon, Apr 11, 2016 at 6:27 PM, Sudip Mukherjee
>> <sudipm.mukherjee@...il.com> wrote:
>>>
>>> From: Sudip Mukherjee <sudip.mukherjee@...ethink.co.uk>
>>>
>>> We have a check for card just after dereferencing it. So if it is NULL
>>> we have already dereferenced it before its check. Lets dereference it
>>> after checking card for NULL.

>>> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
>>> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
>>> @@ -2884,10 +2884,11 @@ static void mwifiex_unregister_dev(struct
>>> mwifiex_adapter *adapter)
>>>  {
>>>         struct pcie_service_card *card = adapter->card;>>
>>
>> Let's say it's 0.
>>
>>>        const struct mwifiex_pcie_card_reg *reg;
>>> -       struct pci_dev *pdev = card->dev;>>
>>
>> This would be equal to offset of dev member in pcie_service_card struct.
>>
>> Nothing wrong here.
>
> Actually, that is not true. The dereference of card tells the compiler that
> card can't be NULL, so it is free to eliminate the check below.
> Unbelievably, this can even happen for a reference such as &ptr->thing where
> the pointer isn't even actually dereferenced at all!

Hmm... Can we look at the result assembly? If I'm not mistaken,
compiler wouldn't even try to calculate pdev pointer before first use
of it.


>
>>> +       struct pci_dev *pdev;
>>>         int i;
>>>
>>>         if (card) {
>>> +               pdev = card->dev;
>>>                 if (card->msix_enable) {
>>>                         for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)


-- 
With Best Regards,
Andy Shevchenko