| Message ID | 20230317173606.91426-1-szymon.heidrich@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] net: usb: lan78xx: Limit packet length to skb->len | expand |
Hi Szymon,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master v6.3-rc2 next-20230317]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Szymon-Heidrich/net-usb-lan78xx-Limit-packet-length-to-skb-len/20230318-013659
patch link: https://lore.kernel.org/r/20230317173606.91426-1-szymon.heidrich%40gmail.com
patch subject: [PATCH v2] net: usb: lan78xx: Limit packet length to skb->len
config: i386-randconfig-a005 (https://download.01.org/0day-ci/archive/20230318/202303181127.Phif6jvW-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/17c060ef752f4a1366ed7d8e62ba5f64f654eced
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Szymon-Heidrich/net-usb-lan78xx-Limit-packet-length-to-skb-len/20230318-013659
git checkout 17c060ef752f4a1366ed7d8e62ba5f64f654eced
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/usb/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303181127.Phif6jvW-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/usb/lan78xx.c: In function 'lan78xx_rx':
>> drivers/net/usb/lan78xx.c:3603:25: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
3603 | struct sk_buff *skb2;
| ^~~~~~
vim +3603 drivers/net/usb/lan78xx.c
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3552
ec4c7e12396b1a John Efstathiades 2021-11-18 3553 static int lan78xx_rx(struct lan78xx_net *dev, struct sk_buff *skb,
ec4c7e12396b1a John Efstathiades 2021-11-18 3554 int budget, int *work_done)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3555 {
0dd87266c1337d John Efstathiades 2021-11-18 3556 if (skb->len < RX_SKB_MIN_LEN)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3557 return 0;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3558
ec4c7e12396b1a John Efstathiades 2021-11-18 3559 /* Extract frames from the URB buffer and pass each one to
ec4c7e12396b1a John Efstathiades 2021-11-18 3560 * the stack in a new NAPI SKB.
ec4c7e12396b1a John Efstathiades 2021-11-18 3561 */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3562 while (skb->len > 0) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3563 u32 rx_cmd_a, rx_cmd_b, align_count, size;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3564 u16 rx_cmd_c;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3565 unsigned char *packet;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3566
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3567 rx_cmd_a = get_unaligned_le32(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3568 skb_pull(skb, sizeof(rx_cmd_a));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3569
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3570 rx_cmd_b = get_unaligned_le32(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3571 skb_pull(skb, sizeof(rx_cmd_b));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3572
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3573 rx_cmd_c = get_unaligned_le16(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3574 skb_pull(skb, sizeof(rx_cmd_c));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3575
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3576 packet = skb->data;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3577
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3578 /* get the packet length */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3579 size = (rx_cmd_a & RX_CMD_A_LEN_MASK_);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3580 align_count = (4 - ((size + RXW_PADDING) % 4)) % 4;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3581
17c060ef752f4a Szymon Heidrich 2023-03-17 3582 if (unlikely(size > skb->len)) {
17c060ef752f4a Szymon Heidrich 2023-03-17 3583 netif_dbg(dev, rx_err, dev->net,
17c060ef752f4a Szymon Heidrich 2023-03-17 3584 "size err rx_cmd_a=0x%08x\n",
17c060ef752f4a Szymon Heidrich 2023-03-17 3585 rx_cmd_a);
17c060ef752f4a Szymon Heidrich 2023-03-17 3586 return 0;
17c060ef752f4a Szymon Heidrich 2023-03-17 3587 }
17c060ef752f4a Szymon Heidrich 2023-03-17 3588
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3589 if (unlikely(rx_cmd_a & RX_CMD_A_RED_)) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3590 netif_dbg(dev, rx_err, dev->net,
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3591 "Error rx_cmd_a=0x%08x", rx_cmd_a);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3592 } else {
17c060ef752f4a Szymon Heidrich 2023-03-17 3593 u32 frame_len;
17c060ef752f4a Szymon Heidrich 2023-03-17 3594
17c060ef752f4a Szymon Heidrich 2023-03-17 3595 if (unlikely(size < ETH_FCS_LEN)) {
17c060ef752f4a Szymon Heidrich 2023-03-17 3596 netif_dbg(dev, rx_err, dev->net,
17c060ef752f4a Szymon Heidrich 2023-03-17 3597 "size err rx_cmd_a=0x%08x\n",
17c060ef752f4a Szymon Heidrich 2023-03-17 3598 rx_cmd_a);
17c060ef752f4a Szymon Heidrich 2023-03-17 3599 return 0;
17c060ef752f4a Szymon Heidrich 2023-03-17 3600 }
17c060ef752f4a Szymon Heidrich 2023-03-17 3601
17c060ef752f4a Szymon Heidrich 2023-03-17 3602 frame_len = size - ETH_FCS_LEN;
ec4c7e12396b1a John Efstathiades 2021-11-18 @3603 struct sk_buff *skb2;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3604
ec4c7e12396b1a John Efstathiades 2021-11-18 3605 skb2 = napi_alloc_skb(&dev->napi, frame_len);
ec4c7e12396b1a John Efstathiades 2021-11-18 3606 if (!skb2)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3607 return 0;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3608
ec4c7e12396b1a John Efstathiades 2021-11-18 3609 memcpy(skb2->data, packet, frame_len);
ec4c7e12396b1a John Efstathiades 2021-11-18 3610
ec4c7e12396b1a John Efstathiades 2021-11-18 3611 skb_put(skb2, frame_len);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3612
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3613 lan78xx_rx_csum_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
ec21ecf0aad279 Dave Stevenson 2018-06-25 3614 lan78xx_rx_vlan_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3615
ec4c7e12396b1a John Efstathiades 2021-11-18 3616 /* Processing of the URB buffer must complete once
ec4c7e12396b1a John Efstathiades 2021-11-18 3617 * it has started. If the NAPI work budget is exhausted
ec4c7e12396b1a John Efstathiades 2021-11-18 3618 * while frames remain they are added to the overflow
ec4c7e12396b1a John Efstathiades 2021-11-18 3619 * queue for delivery in the next NAPI polling cycle.
ec4c7e12396b1a John Efstathiades 2021-11-18 3620 */
ec4c7e12396b1a John Efstathiades 2021-11-18 3621 if (*work_done < budget) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3622 lan78xx_skb_return(dev, skb2);
ec4c7e12396b1a John Efstathiades 2021-11-18 3623 ++(*work_done);
ec4c7e12396b1a John Efstathiades 2021-11-18 3624 } else {
ec4c7e12396b1a John Efstathiades 2021-11-18 3625 skb_queue_tail(&dev->rxq_overflow, skb2);
ec4c7e12396b1a John Efstathiades 2021-11-18 3626 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3627 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3628
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3629 skb_pull(skb, size);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3630
ec4c7e12396b1a John Efstathiades 2021-11-18 3631 /* skip padding bytes before the next frame starts */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3632 if (skb->len)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3633 skb_pull(skb, align_count);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3634 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3635
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3636 return 1;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3637 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3638
On Fri, 17 Mar 2023 18:36:06 +0100 Szymon Heidrich wrote: > Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") > Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> I'd drop the Reported-by: tag in this case, since it makes it look like the bot reported the bug you're fixing, rather than merely reported a build issue. When you repost please avoid posting in reply to previous version. Makes the review queue easier to organize for mentainers.
Hi Szymon,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master v6.3-rc2 next-20230317]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Szymon-Heidrich/net-usb-lan78xx-Limit-packet-length-to-skb-len/20230318-013659
patch link: https://lore.kernel.org/r/20230317173606.91426-1-szymon.heidrich%40gmail.com
patch subject: [PATCH v2] net: usb: lan78xx: Limit packet length to skb->len
config: s390-randconfig-r003-20230318 (https://download.01.org/0day-ci/archive/20230318/202303181417.1MZBovse-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/17c060ef752f4a1366ed7d8e62ba5f64f654eced
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Szymon-Heidrich/net-usb-lan78xx-Limit-packet-length-to-skb-len/20230318-013659
git checkout 17c060ef752f4a1366ed7d8e62ba5f64f654eced
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/net/usb/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303181417.1MZBovse-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/net/usb/lan78xx.c:6:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/net/usb/lan78xx.c:6:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/net/usb/lan78xx.c:6:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/net/usb/lan78xx.c:3603:20: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
struct sk_buff *skb2;
^
13 warnings generated.
vim +3603 drivers/net/usb/lan78xx.c
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3552
ec4c7e12396b1a John Efstathiades 2021-11-18 3553 static int lan78xx_rx(struct lan78xx_net *dev, struct sk_buff *skb,
ec4c7e12396b1a John Efstathiades 2021-11-18 3554 int budget, int *work_done)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3555 {
0dd87266c1337d John Efstathiades 2021-11-18 3556 if (skb->len < RX_SKB_MIN_LEN)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3557 return 0;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3558
ec4c7e12396b1a John Efstathiades 2021-11-18 3559 /* Extract frames from the URB buffer and pass each one to
ec4c7e12396b1a John Efstathiades 2021-11-18 3560 * the stack in a new NAPI SKB.
ec4c7e12396b1a John Efstathiades 2021-11-18 3561 */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3562 while (skb->len > 0) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3563 u32 rx_cmd_a, rx_cmd_b, align_count, size;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3564 u16 rx_cmd_c;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3565 unsigned char *packet;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3566
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3567 rx_cmd_a = get_unaligned_le32(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3568 skb_pull(skb, sizeof(rx_cmd_a));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3569
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3570 rx_cmd_b = get_unaligned_le32(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3571 skb_pull(skb, sizeof(rx_cmd_b));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3572
bb448f8a60ea93 Chuhong Yuan 2019-07-19 3573 rx_cmd_c = get_unaligned_le16(skb->data);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3574 skb_pull(skb, sizeof(rx_cmd_c));
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3575
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3576 packet = skb->data;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3577
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3578 /* get the packet length */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3579 size = (rx_cmd_a & RX_CMD_A_LEN_MASK_);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3580 align_count = (4 - ((size + RXW_PADDING) % 4)) % 4;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3581
17c060ef752f4a Szymon Heidrich 2023-03-17 3582 if (unlikely(size > skb->len)) {
17c060ef752f4a Szymon Heidrich 2023-03-17 3583 netif_dbg(dev, rx_err, dev->net,
17c060ef752f4a Szymon Heidrich 2023-03-17 3584 "size err rx_cmd_a=0x%08x\n",
17c060ef752f4a Szymon Heidrich 2023-03-17 3585 rx_cmd_a);
17c060ef752f4a Szymon Heidrich 2023-03-17 3586 return 0;
17c060ef752f4a Szymon Heidrich 2023-03-17 3587 }
17c060ef752f4a Szymon Heidrich 2023-03-17 3588
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3589 if (unlikely(rx_cmd_a & RX_CMD_A_RED_)) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3590 netif_dbg(dev, rx_err, dev->net,
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3591 "Error rx_cmd_a=0x%08x", rx_cmd_a);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3592 } else {
17c060ef752f4a Szymon Heidrich 2023-03-17 3593 u32 frame_len;
17c060ef752f4a Szymon Heidrich 2023-03-17 3594
17c060ef752f4a Szymon Heidrich 2023-03-17 3595 if (unlikely(size < ETH_FCS_LEN)) {
17c060ef752f4a Szymon Heidrich 2023-03-17 3596 netif_dbg(dev, rx_err, dev->net,
17c060ef752f4a Szymon Heidrich 2023-03-17 3597 "size err rx_cmd_a=0x%08x\n",
17c060ef752f4a Szymon Heidrich 2023-03-17 3598 rx_cmd_a);
17c060ef752f4a Szymon Heidrich 2023-03-17 3599 return 0;
17c060ef752f4a Szymon Heidrich 2023-03-17 3600 }
17c060ef752f4a Szymon Heidrich 2023-03-17 3601
17c060ef752f4a Szymon Heidrich 2023-03-17 3602 frame_len = size - ETH_FCS_LEN;
ec4c7e12396b1a John Efstathiades 2021-11-18 @3603 struct sk_buff *skb2;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3604
ec4c7e12396b1a John Efstathiades 2021-11-18 3605 skb2 = napi_alloc_skb(&dev->napi, frame_len);
ec4c7e12396b1a John Efstathiades 2021-11-18 3606 if (!skb2)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3607 return 0;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3608
ec4c7e12396b1a John Efstathiades 2021-11-18 3609 memcpy(skb2->data, packet, frame_len);
ec4c7e12396b1a John Efstathiades 2021-11-18 3610
ec4c7e12396b1a John Efstathiades 2021-11-18 3611 skb_put(skb2, frame_len);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3612
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3613 lan78xx_rx_csum_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
ec21ecf0aad279 Dave Stevenson 2018-06-25 3614 lan78xx_rx_vlan_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3615
ec4c7e12396b1a John Efstathiades 2021-11-18 3616 /* Processing of the URB buffer must complete once
ec4c7e12396b1a John Efstathiades 2021-11-18 3617 * it has started. If the NAPI work budget is exhausted
ec4c7e12396b1a John Efstathiades 2021-11-18 3618 * while frames remain they are added to the overflow
ec4c7e12396b1a John Efstathiades 2021-11-18 3619 * queue for delivery in the next NAPI polling cycle.
ec4c7e12396b1a John Efstathiades 2021-11-18 3620 */
ec4c7e12396b1a John Efstathiades 2021-11-18 3621 if (*work_done < budget) {
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3622 lan78xx_skb_return(dev, skb2);
ec4c7e12396b1a John Efstathiades 2021-11-18 3623 ++(*work_done);
ec4c7e12396b1a John Efstathiades 2021-11-18 3624 } else {
ec4c7e12396b1a John Efstathiades 2021-11-18 3625 skb_queue_tail(&dev->rxq_overflow, skb2);
ec4c7e12396b1a John Efstathiades 2021-11-18 3626 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3627 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3628
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3629 skb_pull(skb, size);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3630
ec4c7e12396b1a John Efstathiades 2021-11-18 3631 /* skip padding bytes before the next frame starts */
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3632 if (skb->len)
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3633 skb_pull(skb, align_count);
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3634 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3635
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3636 return 1;
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3637 }
55d7de9de6c30a Woojung.Huh@microchip.com 2015-07-30 3638
On Fri, Mar 17, 2023 at 06:36:06PM +0100, Szymon Heidrich wrote: > Packet length retrieved from descriptor may be larger than > the actual socket buffer length. In such case the cloned > skb passed up the network stack will leak kernel memory contents. > > Additionally prevent integer underflow when size is less than > ETH_FCS_LEN. > > Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") > Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> the test robot did not report the fact that the packet length needed to be limited :(
On 23/03/2023 17:29, Greg KH wrote: > On Fri, Mar 17, 2023 at 06:36:06PM +0100, Szymon Heidrich wrote: >> Packet length retrieved from descriptor may be larger than >> the actual socket buffer length. In such case the cloned >> skb passed up the network stack will leak kernel memory contents. >> >> Additionally prevent integer underflow when size is less than >> ETH_FCS_LEN. >> >> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") >> Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com> >> Reported-by: kernel test robot <lkp@intel.com> > > the test robot did not report the fact that the packet length needed to > be limited :( > Yes, I removed the Reported-by tag in V3 as suggested by Jakub.
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 068488890..a150711a1 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -3579,11 +3579,27 @@ static int lan78xx_rx(struct lan78xx_net *dev, struct sk_buff *skb, size = (rx_cmd_a & RX_CMD_A_LEN_MASK_); align_count = (4 - ((size + RXW_PADDING) % 4)) % 4; + if (unlikely(size > skb->len)) { + netif_dbg(dev, rx_err, dev->net, + "size err rx_cmd_a=0x%08x\n", + rx_cmd_a); + return 0; + } + if (unlikely(rx_cmd_a & RX_CMD_A_RED_)) { netif_dbg(dev, rx_err, dev->net, "Error rx_cmd_a=0x%08x", rx_cmd_a); } else { - u32 frame_len = size - ETH_FCS_LEN; + u32 frame_len; + + if (unlikely(size < ETH_FCS_LEN)) { + netif_dbg(dev, rx_err, dev->net, + "size err rx_cmd_a=0x%08x\n", + rx_cmd_a); + return 0; + } + + frame_len = size - ETH_FCS_LEN; struct sk_buff *skb2; skb2 = napi_alloc_skb(&dev->napi, frame_len);
Packet length retrieved from descriptor may be larger than the actual socket buffer length. In such case the cloned skb passed up the network stack will leak kernel memory contents. Additionally prevent integer underflow when size is less than ETH_FCS_LEN. Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com> Reported-by: kernel test robot <lkp@intel.com> --- V1 -> V2: Fix ISO C90 forbids mixed declarations and code drivers/net/usb/lan78xx.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)