netdev - Re: [PATCH] ibmveth: Kernel crash LSO offload flag toggle

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: <6b990276-6e6f-f892-72c9-b330c9a292c2@linux.vnet.ibm.com>
Date:   Tue, 14 Nov 2017 09:24:49 -0600
From:   "Bryant G. Ly" <bryantly@...ux.vnet.ibm.com>
To:     Daniel Axtens <dja@...ens.net>, benh@...nel.crashing.org,
        paulus@...ba.org, mpe@...erman.id.au, tlfalcon@...ux.vnet.ibm.com
Cc:     netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        stable@...r.kernel.org
Subject: Re: [PATCH] ibmveth: Kernel crash LSO offload flag toggle

On 11/13/17 7:07 PM, Daniel Axtens wrote:

> Hi Bryant,
>
> A few things:
>
> 1) The commit message could probably be trimmed, especially the stack
> traces.

Yes, I can trim it. 

> 2) What exactly are you changing and why does it fix the issue? I
> couldn't figure that out from the commit message.

The bounce buffer can be null when skb_copy_from_linear_data tries to use
it and that causes the kernel crash. Thus adding the check to see whether or
not bounce buffer is null prior to utilizing it.

>
> 3) Does this need a Fixes: tag?
>
Honestly, this bug has existed for a very long time. It only became more evident when 
commit 66aa0678efc29abd2ab02a09b23f9a8bc9f12a6c was committed. It is rare for a customer
to his this issue since a customer only toggles LSO once and leaves it, but to re-create
this bug one would need to write a script like the one in the commit message to toggle it
on and off in a loop. Its ultimately a legacy bug that was uncovered more recently. 

>>  	}
>>  
>> -	netdev->min_mtu = IBMVETH_MIN_MTU;
>> -	netdev->max_mtu = ETH_MAX_MTU;
>> -
> 4) What does the above hunk do? It seems unrelated...
>
You are right the above is just cleanup, I should have separated the patch. 

> Regards,
> Daniel
>