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  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, 6 Jan 2021 23:59:52 +0000
From:   Long Li <longli@...rosoft.com>
To:     kernel test robot <lkp@...el.com>,
        Long Li <longli@...uxonhyperv.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "kbuild-all@...ts.01.org" <kbuild-all@...ts.01.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH 2/3] hv_netvsc: Wait for completion on request
 NVSP_MSG4_TYPE_SWITCH_DATA_PATH

> Subject: Re: [PATCH 2/3] hv_netvsc: Wait for completion on request
> NVSP_MSG4_TYPE_SWITCH_DATA_PATH
> 
> Hi Long,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on linus/master] [also build test WARNING on
> v5.11-rc2 next-20210104] [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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-
> scm.com%2Fdocs%2Fgit-format-
> patch&amp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3d454eb468
> b85fb08d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> 37455042608743102%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=
> 90AgH9HlZumRZ4UNC4uD2WIRpZ6ZEnvIdOKOfzYcXpI%3D&amp;reserved=0]
> 
> url:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2F0day-ci%2Flinux%2Fcommits%2FLong-Li%2Fhv_netvsc-Check-VF-
> datapath-when-sending-traffic-to-VF%2F20210106-
> 092237&amp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3d454eb46
> 8b85fb08d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
> 637455042608753098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata
> =vtVJ8pXIOxIYeKdaqT9pD1%2BEuOM3wz4yqsHh8uWsGP4%3D&amp;reserv
> ed=0
> base:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git&
> amp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3d454eb468b85fb0
> 8d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6374550
> 42608753098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=FXMG
> CFODFoq3KLklxr17iVHiq%2FWmJ3c0fM7vIZRfNmc%3D&amp;reserved=0
> e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62
> config: i386-allyesconfig (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
>         #
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2F0day-
> ci%2Flinux%2Fcommit%2F8c92b5574da1b0c2aee3eab7da2c4dad8d92572c&a
> mp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3d454eb468b85fb08
> d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63745504
> 2608753098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MMXkQ
> KENGpyfW0NJs2khBSKTuBExFSZaWHgWyyIj6UU%3D&amp;reserved=0
>         git remote add linux-review
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2F0day-
> ci%2Flinux&amp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3d454e
> b468b85fb08d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0
> %7C637455042608753098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;s
> data=uge6PX2NyAe%2BjRtvgOhR5xzN2ltBctZXeZwn0hoYco0%3D&amp;reser
> ved=0
>         git fetch --no-tags linux-review Long-Li/hv_netvsc-Check-VF-datapath-
> when-sending-traffic-to-VF/20210106-092237
>         git checkout 8c92b5574da1b0c2aee3eab7da2c4dad8d92572c
>         # save the attached .config to linux build tree
>         make W=1 ARCH=i386
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/net/hyperv/netvsc.c: In function 'netvsc_send_completion':
> >> drivers/net/hyperv/netvsc.c:778:14: warning: cast to pointer from
> >> integer of different size [-Wint-to-pointer-cast]
>      778 |   pkt_rqst = (struct nvsp_message *)cmd_rqst;
>          |              ^

I think this warning can be safely ignored.

When sending packets over vmbus, the address is passed as u64 and stored internally as u64 in vmbus_next_request_id(). Passing a 32 bit address will not lose any data. Later the address is retrieved from vmbus_request_addr() as a u64. Again, it will not lose data when casting to a 32 bit address.

This method of storing and retrieving addresses are used throughout other hyper-v drivers. If we want to not to trigger this warning, I suggest making a patch to convert all those usages in all hyper-v drivers.

Thanks,
Long

> 
> 
> vim +778 drivers/net/hyperv/netvsc.c
> 
>    757
>    758	static void netvsc_send_completion(struct net_device *ndev,
>    759					   struct netvsc_device *net_device,
>    760					   struct vmbus_channel
> *incoming_channel,
>    761					   const struct vmpacket_descriptor
> *desc,
>    762					   int budget)
>    763	{
>    764		const struct nvsp_message *nvsp_packet =
> hv_pkt_data(desc);
>    765		u32 msglen = hv_pkt_datalen(desc);
>    766		struct nvsp_message *pkt_rqst;
>    767		u64 cmd_rqst;
>    768
>    769		/* First check if this is a VMBUS completion without data
> payload */
>    770		if (!msglen) {
>    771			cmd_rqst =
> vmbus_request_addr(&incoming_channel->requestor,
>    772						      (u64)desc->trans_id);
>    773			if (cmd_rqst == VMBUS_RQST_ERROR) {
>    774				netdev_err(ndev, "Invalid transaction id\n");
>    775				return;
>    776			}
>    777
>  > 778			pkt_rqst = (struct nvsp_message *)cmd_rqst;
>    779			switch (pkt_rqst->hdr.msg_type) {
>    780			case NVSP_MSG4_TYPE_SWITCH_DATA_PATH:
>    781				complete(&net_device->channel_init_wait);
>    782				break;
>    783
>    784			default:
>    785				netdev_err(ndev, "Unexpected VMBUS
> completion!!\n");
>    786			}
>    787			return;
>    788		}
>    789
>    790		/* Ensure packet is big enough to read header fields */
>    791		if (msglen < sizeof(struct nvsp_message_header)) {
>    792			netdev_err(ndev, "nvsp_message length too
> small: %u\n", msglen);
>    793			return;
>    794		}
>    795
>    796		switch (nvsp_packet->hdr.msg_type) {
>    797		case NVSP_MSG_TYPE_INIT_COMPLETE:
>    798			if (msglen < sizeof(struct nvsp_message_header) +
>    799					sizeof(struct
> nvsp_message_init_complete)) {
>    800				netdev_err(ndev, "nvsp_msg length too
> small: %u\n",
>    801					   msglen);
>    802				return;
>    803			}
>    804			fallthrough;
>    805
>    806		case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
>    807			if (msglen < sizeof(struct nvsp_message_header) +
>    808					sizeof(struct
> nvsp_1_message_send_receive_buffer_complete)) {
>    809				netdev_err(ndev, "nvsp_msg1 length too
> small: %u\n",
>    810					   msglen);
>    811				return;
>    812			}
>    813			fallthrough;
>    814
>    815		case NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE:
>    816			if (msglen < sizeof(struct nvsp_message_header) +
>    817					sizeof(struct
> nvsp_1_message_send_send_buffer_complete)) {
>    818				netdev_err(ndev, "nvsp_msg1 length too
> small: %u\n",
>    819					   msglen);
>    820				return;
>    821			}
>    822			fallthrough;
>    823
>    824		case NVSP_MSG5_TYPE_SUBCHANNEL:
>    825			if (msglen < sizeof(struct nvsp_message_header) +
>    826					sizeof(struct
> nvsp_5_subchannel_complete)) {
>    827				netdev_err(ndev, "nvsp_msg5 length too
> small: %u\n",
>    828					   msglen);
>    829				return;
>    830			}
>    831			/* Copy the response back */
>    832			memcpy(&net_device->channel_init_pkt,
> nvsp_packet,
>    833			       sizeof(struct nvsp_message));
>    834			complete(&net_device->channel_init_wait);
>    835			break;
>    836
>    837		case NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE:
>    838			netvsc_send_tx_complete(ndev, net_device,
> incoming_channel,
>    839						desc, budget);
>    840			break;
>    841
>    842		default:
>    843			netdev_err(ndev,
>    844				   "Unknown send completion type %d
> received!!\n",
>    845				   nvsp_packet->hdr.msg_type);
>    846		}
>    847	}
>    848
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fhyperkitty%2Flist%2Fkbuild-
> all%40lists.01.org&amp;data=04%7C01%7Clongli%40microsoft.com%7C695cf3
> d454eb468b85fb08d8b1fb3ddd%7C72f988bf86f141af91ab2d7cd011db47%7C1
> %7C0%7C637455042608753098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&
> amp;sdata=AKWfmJrn1C%2BwaqX6wlu95HcPys9K0ju%2FlC%2Bu3O20jAg%3
> D&amp;reserved=0

Powered by blists - more mailing lists