diff mbox series

[v2,net-next] net: usb: smsc95xx: stop lying about skb->truesize

Message ID 20240508075159.1646031-1-edumazet@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v2,net-next] net: usb: smsc95xx: stop lying about skb->truesize | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 927 this patch: 927
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 1 maintainers not CCed: linux-usb@vger.kernel.org
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 938 this patch: 938
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 41 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-09--06-00 (tests: 1004)

Commit Message

Eric Dumazet May 8, 2024, 7:51 a.m. UTC
Some usb drivers try to set small skb->truesize and break
core networking stacks.

In this patch, I removed one of the skb->truesize override.

I also replaced one skb_clone() by an allocation of a fresh
and small skb, to get minimally sized skbs, like we did
in commit 1e2c61172342 ("net: cdc_ncm: reduce skb truesize
in rx path") and 4ce62d5b2f7a ("net: usb: ax88179_178a:
stop lying about skb->truesize")

v2: leave the skb_trim() game because smsc95xx_rx_csum_offload()
    needs the csum part. (Jakub)
    While we are it, use get_unaligned() in smsc95xx_rx_csum_offload().

Fixes: 2f7ca802bdae ("net: Add SMSC LAN9500 USB2.0 10/100 ethernet adapter driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Cc: UNGLinuxDriver@microchip.com
---
 drivers/net/usb/smsc95xx.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

kernel test robot May 9, 2024, 6 a.m. UTC | #1
Hi Eric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Eric-Dumazet/net-usb-smsc95xx-stop-lying-about-skb-truesize/20240508-155316
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240508075159.1646031-1-edumazet%40google.com
patch subject: [PATCH v2 net-next] net: usb: smsc95xx: stop lying about skb->truesize
config: arc-randconfig-r132-20240509 (https://download.01.org/0day-ci/archive/20240509/202405091310.KvncIecx-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240509/202405091310.KvncIecx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405091310.KvncIecx-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/usb/smsc95xx.c:1815:19: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __wsum [usertype] csum @@     got unsigned short x @@
   drivers/net/usb/smsc95xx.c:1815:19: sparse:     expected restricted __wsum [usertype] csum
   drivers/net/usb/smsc95xx.c:1815:19: sparse:     got unsigned short x
   drivers/net/usb/smsc95xx.c: note: in included file (through include/net/checksum.h, include/linux/skbuff.h, include/net/net_namespace.h, ...):
   arch/arc/include/asm/checksum.h:27:26: sparse: sparse: restricted __wsum degrades to integer
   arch/arc/include/asm/checksum.h:27:36: sparse: sparse: restricted __wsum degrades to integer
   arch/arc/include/asm/checksum.h:29:11: sparse: sparse: bad assignment (-=) to restricted __wsum
   arch/arc/include/asm/checksum.h:30:16: sparse: sparse: restricted __wsum degrades to integer
   arch/arc/include/asm/checksum.h:30:18: sparse: sparse: incorrect type in return expression (different base types) @@     expected restricted __sum16 @@     got unsigned int @@
   arch/arc/include/asm/checksum.h:30:18: sparse:     expected restricted __sum16
   arch/arc/include/asm/checksum.h:30:18: sparse:     got unsigned int

vim +1815 drivers/net/usb/smsc95xx.c

  1810	
  1811	static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
  1812	{
  1813		u16 *csum_ptr = (u16 *)(skb_tail_pointer(skb) - 2);
  1814	
> 1815		skb->csum = get_unaligned(csum_ptr);
  1816		skb->ip_summed = CHECKSUM_COMPLETE;
  1817		skb_trim(skb, skb->len - 2); /* remove csum */
  1818	}
  1819
Eric Dumazet May 9, 2024, 8:25 a.m. UTC | #2
On Thu, May 9, 2024 at 8:01 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Eric,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on net-next/main]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Eric-Dumazet/net-usb-smsc95xx-stop-lying-about-skb-truesize/20240508-155316
> base:   net-next/main
> patch link:    https://lore.kernel.org/r/20240508075159.1646031-1-edumazet%40google.com
> patch subject: [PATCH v2 net-next] net: usb: smsc95xx: stop lying about skb->truesize
> config: arc-randconfig-r132-20240509 (https://download.01.org/0day-ci/archive/20240509/202405091310.KvncIecx-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20240509/202405091310.KvncIecx-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202405091310.KvncIecx-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
> >> drivers/net/usb/smsc95xx.c:1815:19: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __wsum [usertype] csum @@     got unsigned short x @@
>    drivers/net/usb/smsc95xx.c:1815:19: sparse:     expected restricted __wsum [usertype] csum
>    drivers/net/usb/smsc95xx.c:1815:19: sparse:     got unsigned short x
>    drivers/net/usb/smsc95xx.c: note: in included file (through include/net/checksum.h, include/linux/skbuff.h, include/net/net_namespace.h, ...):
>    arch/arc/include/asm/checksum.h:27:26: sparse: sparse: restricted __wsum degrades to integer
>    arch/arc/include/asm/checksum.h:27:36: sparse: sparse: restricted __wsum degrades to integer
>    arch/arc/include/asm/checksum.h:29:11: sparse: sparse: bad assignment (-=) to restricted __wsum
>    arch/arc/include/asm/checksum.h:30:16: sparse: sparse: restricted __wsum degrades to integer
>    arch/arc/include/asm/checksum.h:30:18: sparse: sparse: incorrect type in return expression (different base types) @@     expected restricted __sum16 @@     got unsigned int @@
>    arch/arc/include/asm/checksum.h:30:18: sparse:     expected restricted __sum16
>    arch/arc/include/asm/checksum.h:30:18: sparse:     got unsigned int
>
> vim +1815 drivers/net/usb/smsc95xx.c
>
>   1810
>   1811  static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
>   1812  {
>   1813          u16 *csum_ptr = (u16 *)(skb_tail_pointer(skb) - 2);
>   1814
> > 1815          skb->csum = get_unaligned(csum_ptr);
>   1816          skb->ip_summed = CHECKSUM_COMPLETE;
>   1817          skb_trim(skb, skb->len - 2); /* remove csum */
>   1818  }
>   1819
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki


Please note this is not a new warning. Prior to my change we had :

drivers/net/usb/smsc95xx.c:1813:19: warning: incorrect type in
assignment (different base types)
drivers/net/usb/smsc95xx.c:1813:19:    expected restricted __wsum
[usertype] csum
drivers/net/usb/smsc95xx.c:1813:19:    got unsigned short [usertype]

I will submit a v3, to _also_ fix the sparse warning.
diff mbox series

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 2fa46baa589e5e87e12e145fe46268bdaf9fc219..0311328797c345bfa9a117596c268757808828d1 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1810,9 +1810,11 @@  static int smsc95xx_reset_resume(struct usb_interface *intf)
 
 static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
 {
-	skb->csum = *(u16 *)(skb_tail_pointer(skb) - 2);
+	u16 *csum_ptr = (u16 *)(skb_tail_pointer(skb) - 2);
+
+	skb->csum = get_unaligned(csum_ptr);
 	skb->ip_summed = CHECKSUM_COMPLETE;
-	skb_trim(skb, skb->len - 2);
+	skb_trim(skb, skb->len - 2); /* remove csum */
 }
 
 static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
@@ -1870,25 +1872,22 @@  static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 				if (dev->net->features & NETIF_F_RXCSUM)
 					smsc95xx_rx_csum_offload(skb);
 				skb_trim(skb, skb->len - 4); /* remove fcs */
-				skb->truesize = size + sizeof(struct sk_buff);
 
 				return 1;
 			}
 
-			ax_skb = skb_clone(skb, GFP_ATOMIC);
+			ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
 			if (unlikely(!ax_skb)) {
 				netdev_warn(dev->net, "Error allocating skb\n");
 				return 0;
 			}
 
-			ax_skb->len = size;
-			ax_skb->data = packet;
-			skb_set_tail_pointer(ax_skb, size);
+			skb_put(ax_skb, size);
+			memcpy(ax_skb->data, packet, size);
 
 			if (dev->net->features & NETIF_F_RXCSUM)
 				smsc95xx_rx_csum_offload(ax_skb);
 			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
-			ax_skb->truesize = size + sizeof(struct sk_buff);
 
 			usbnet_skb_return(dev, ax_skb);
 		}