Re: [PATCH] peak_pci: fix use after free in netdev teardown

[Stephane Grosjean <s.grosjean@peak-system.com>]

Hi All,

So I finally had a look to the diff file. It clearly does what 
Christopher says, that is, it fixes some memory access order issues when 
the CAN devices have to be removed from the system, either when probing 
has failed or when the driver is unloaded from the system.

The diff file has to be re-written into a Linux-coding style patch, and 
yes, maybe all this removing stuff could be put into a (new) single 
function made for that.

So, how do we proceed, please ?

Regards,

Stéphane

Le 19/05/2014 09:11, Marc Kleine-Budde a écrit :
> On 04/09/2014 09:45 PM, Christopher R. Baker wrote:
>> Hi All,
>>
>> In the course of tracking down (and eventually backporting) a fix to one
>> of my systems that is still running a 3.2 kernel, I noticed what I
>> believe to be a pair of use-after-free bugs in peak_pci.c pertaining to
>> the linked list of netdevs that is maintained for multi-port cards.
>> These bugs persist in 3.14, so I thought I should send along a patch for
>> review.
>>
>> Basically, the "prev_dev" pointer that is used for this list lives in
>> memory that is allocated by alloc_sja1000dev(), but it is referenced
>> after the call to free_sja1000dev() when walking the list during
>> teardown, both in the failure case of peak_pci_probe and in
>> peak_pci_remove.  Unless I'm missing something, these are toes waiting
>> to be stubbed...
>>
>> Caveats:
>>   - This is a growing blob of copy-pasta that should probably be
>> refactored to a common location.  For example, peak_pci_remove could be
>> restructured to incrementally check and free allocated resources,
>> allowing the "failure_remove_channels" label to delegate the cleanup to
>> peak_pci_remove.  I didn't want to bite off too much this time, though,
>> so I left that alone.
>>   - I don't have an expresscard adapter to check the placement of the
>> pciec_remove stanzas.  By inspection, unregister_sja1000dev does not
>> appear to have a path back to the pciec stuff, but I may have missed
>> something.
>>
>> -ChrisR
>>
>> Signed-of-by: Christopher R. Baker <cbaker@rec.ri.cmu.edu>
> Stephane, can you please have a look at the patch.
>
> Marc
>

--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt 
Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt 
HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 
Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com
--