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] [thread-next>] [day] [month] [year] [list]
Message-ID: <867653EE-136B-488D-8F85-716B1ED9BD1A@intel.com>
Date:	Tue, 12 Apr 2016 17:43:03 +0000
From:	"Rustad, Mark D" <mark.d.rustad@...el.com>
To:	Andy Shevchenko <andy.shevchenko@...il.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

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.
>
> IIUC the code does nothing with dereference.
>
> I would have told NAK if I would have been maintainer.
>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@...ethink.co.uk>
>> ---
>>  drivers/net/wireless/marvell/mwifiex/pcie.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c  
>> b/drivers/net/wireless/marvell/mwifiex/pcie.c
>> index edf8b07..84562d0 100644
>> --- 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!

>> +       struct pci_dev *pdev;
>>         int i;
>>
>>         if (card) {
>> +               pdev = card->dev;
>>                 if (card->msix_enable) {
>>                         for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
>>                                 synchronize_irq(card->msix_entries[i].vector);
>> --
>> 1.9.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko


--
Mark Rustad, Networking Division, Intel Corporation

Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ