netdev - Re: mv643xxx_eth driver, failed to linearize skb with tiny unaligned fragment

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:   Wed, 28 Sep 2016 16:04:42 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Frans van de Wiel <fvdw@...w.eu>
Cc:     netdev@...r.kernel.org,
        Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
Subject: Re: mv643xxx_eth driver, failed to linearize skb with tiny
 unaligned fragment

On Wed, 2016-09-28 at 22:37 +0200, Frans van de Wiel wrote:
> We compiled kernel 4.6.6 for our nas devices running on ARM kirkwood cpu’s. 
> These nas devices have only 256 MB of RAM so limited memory resources
> When we fully load the cpu and are copying files via the interface we get 
> frequently this “ error” message in dmesg log
> 
> [ 1978.149929] mv643xx_eth_port mv643xx_eth_port.0 eth0: failed to linearize 
> skb with tiny unaligned fragment
> [ 1978.150138] mv643xx_eth_port mv643xx_eth_port.0 eth0: failed to linearize 
> skb with tiny unaligned fragment
> [ 1978.150318] mv643xx_eth_port mv643xx_eth_port.0 eth0: failed to linearize 
> skb with tiny unaligned fragment
> [ 1978.150360] mv643xx_eth_port mv643xx_eth_port.0 eth0: failed to linearize 
> skb with tiny unaligned fragment
> 
> ...
> 
> CPU[|||||||||||||||||||||||||||||||||||98.9%]     Tasks: 29, 0 thr; 2 
> running
>   Mem[||||||||||||||||||||||||||||||||30/244MB]     Load average: 1.36 1.29 
> 0.92
>   Swp[                                 0/511MB]     Uptime: 00:38:11
> 
> We analyzed the driver code code and think that it does not seems to be an 
> error but a warning, this we want to verify and if this warning can be taken 
> out without risk
> 
> The message originates from this part of the code of the driver
> 
> 1023 if (has_tiny_unaligned_frags(skb) && __skb_linearize(skb)) {
> 1024                 netdev_printk(KERN_DEBUG, dev,
> 1025                               "failed to linearize skb with tiny 
> unaligned fragment\n");
> 1026                 return NETDEV_TX_BUSY;
> 1027         }
> 
> __skb_linearize is set in skbuff.h and comments are useful
> 
> static inline int __skb_linearize(struct sk_buff *skb)
> 1636 {
> 1637         return __pskb_pull_tail(skb, skb->data_len) ? 0 : -ENOMEM;
> 1638 }
> 1639
> 1640 /**
> 1641  *      skb_linearize - convert paged skb to linear one
> 1642  *      @skb: buffer to linarize
> 1643  *
> 1644  *      If there is no free memory -ENOMEM is returned, otherwise zero
> 1645  *      is returned and the old skb data released.
> 1646  */
> 1647 static inline int skb_linearize(struct sk_buff *skb)
> 1648 {
> 
> So return a false state (0) if there is enough free memory and true on the 
> other case.
> 
> Please to note mv643xx_eth.c returns also a busy state. So I assume on this 
> case the driver repeats this step up to success
> 
> So in my opinion, this is not an error message but a warning and does not 
> mean corrupted data and I think is should be possible to remove it.
> Is conclusion is correct ?

Well, linearizing an skb only because one fragment is tiny and not
aligned is unfortunate.

This driver should copy the ~7 bytes into temp storage instead.

It would be faster, and would avoid dropping packet when the (possibly
huge) allocation fails.

Strange thing is that txq_submit_tso() can happily present 'tiny frags'
after segmentation is done on a 'big frag', so I wonder if the
has_tiny_unaligned_frags() is really needed.

If this is needed, then a fix in txq_submit_tso() is also needed.