netdev - Re: [PATCH] vmxnet3: avoid calling pskb_may

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]

Date:	Tue, 8 Mar 2016 09:31:46 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, skhare@...are.com, pv-drivers@...are.com
Subject: Re: [PATCH] vmxnet3: avoid calling pskb_may_pull with interrupts
 disabled

On Mon, Mar 07, 2016 at 03:16:14PM -0500, David Miller wrote:
> From: Neil Horman <nhorman@...driver.com>
> Date: Fri,  4 Mar 2016 13:40:48 -0500
> 
> > vmxnet3 has a function vmxnet3_parse_and_copy_hdr which, among other operations,
> > uses pskb_may_pull to linearize the header portion of an skb.  That operation
> > eventually uses local_bh_disable/enable to ensure that it doesn't race with the
> > drivers bottom half handler.  Unfortunately, vmxnet3 preforms this
> > parse_and_copy operation with a spinlock held and interrupts disabled.  This
> > causes us to run afoul of the WARN_ON_ONCE(irqs_disabled()) warning in
> > local_bh_enable, resulting in this:
> > 
> > WARNING: at kernel/softirq.c:159 local_bh_enable+0x59/0x90() (Not tainted)
> > Hardware name: VMware Virtual Platform
> > Modules linked in: ipv6 ppdev parport_pc parport microcode e1000 vmware_balloon
> > vmxnet3 i2c_piix4 sg ext4 jbd2 mbcache sd_mod crc_t10dif sr_mod cdrom mptspi
> > mptscsih mptbase scsi_transport_spi pata_acpi ata_generic ata_piix vmwgfx ttm
> > drm_kms_helper drm i2c_core dm_mirror dm_region_hash dm_log dm_mod [last
> > unloaded: mperf]
>  ...
> > Fix it by splitting vmxnet3_parse_and_copy_hdr into two functions:
> > 
> > vmxnet3_parse_hdr, which sets up the internal/on stack ctx datastructure, and
> > pulls the skb (both of which can be done without holding the spinlock with irqs
> > disabled
> > 
> > and
> > 
> > vmxnet3_copy_header, which just copies the skb to the tx ring under the lock
> > safely.
> > 
> > tested and shown to correct the described problem.  Applies cleanly to the head
> > of the net tree
> > 
> > Signed-off-by: Neil Horman <nhorman@...driver.com>
> 
> Applied, thanks Neil.
> 
> > +static void 
> 
> Trailing whitespace, which I fixed up while applying this.
Sorry about that, appreciate the fixup
Neil

> 
> Just FYI.
>