| Message ID | 1331328437-26852-1-git-send-email-haiyangz@microsoft.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Haiyang Zhang <haiyangz@microsoft.com> Date: Fri, 9 Mar 2012 13:27:16 -0800 > Limiting the memcpy to be the sizeof(struct rndis_message) can truncate > the message if there are Per-Packet-Info or Out-of-Band data. > > In my earlier patch (commit 45326342), the unnecessary kmap_atomic and > kunmap_atomic surrounding this memcpy have been removed because the memory > in the receive buffer is always mapped. This memcpy is not necessary > either. To fix the bug, I removed the memcpy. > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > Reviewed-by: K. Y. Srinivasan <kys@microsoft.com> Well, where the heck is patch 2/2? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Friday, March 09, 2012 4:51 PM > To: Haiyang Zhang > Cc: KY Srinivasan; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org > Subject: Re: [PATCH 1/2] net/hyperv: Fix data corruption in > rndis_filter_receive() > > From: Haiyang Zhang <haiyangz@microsoft.com> > Date: Fri, 9 Mar 2012 13:27:16 -0800 > > > Limiting the memcpy to be the sizeof(struct rndis_message) can > > truncate the message if there are Per-Packet-Info or Out-of-Band data. > > > > In my earlier patch (commit 45326342), the unnecessary kmap_atomic and > > kunmap_atomic surrounding this memcpy have been removed because the > > memory in the receive buffer is always mapped. This memcpy is not > > necessary either. To fix the bug, I removed the memcpy. > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > > Reviewed-by: K. Y. Srinivasan <kys@microsoft.com> > > Well, where the heck is patch 2/2? It's still in the mail server's send queue. This happens sometimes on our mail server. But it should reach you in half hour or so... - Haiyang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
It is very tiring explaining how to submit patches properly, and here you've made a poor submission once again. You haven't indicated what tree you want these changes applied to. You also mixed bug fixes and feature patches into the same patch set. So the patch set isn't appropriate for the 'net' tree, because only the most critical bug fixes can go there now. And this patch set is also not appropriate for the 'net-next' tree because I can only presume you want the important bug fix propagated upstream as fast as possible but it cannot go in with the feature patch since the feature patch is absolutely not appropriate for the 'net' tree right now. I really don't see why it's such a hard thing to simply say where you want patches applied when you submit them. Do you want me to simply guess? How else am I supposed to figure out what you want? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Sunday, March 11, 2012 6:51 PM > To: Haiyang Zhang > Cc: KY Srinivasan; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org > Subject: Re: [PATCH 1/2] net/hyperv: Fix data corruption in > rndis_filter_receive() > > > It is very tiring explaining how to submit patches properly, and here you've > made a poor submission once again. > > You haven't indicated what tree you want these changes applied to. > > You also mixed bug fixes and feature patches into the same patch set. > > So the patch set isn't appropriate for the 'net' tree, because only the most > critical bug fixes can go there now. > > And this patch set is also not appropriate for the 'net-next' tree because I can > only presume you want the important bug fix propagated upstream as fast as > possible but it cannot go in with the feature patch since the feature patch is > absolutely not appropriate for the 'net' tree right now. > > I really don't see why it's such a hard thing to simply say where you want > patches applied when you submit them. Do you want me to simply guess? > How else am I supposed to figure out what you want? I want both patches go to the 'net-next' tree. The patch [1/2] is required by the vlan trunking feature [2/2]. So they should go together. The bug in rndis_filter_receive() does NOT happen until we start to use 'per-packet-data' by the vlan trunking feature, so the bug fix [1/2] isn't urgent to be applied into 'net' tree. And, thanks for the reminder, I will specify the target tree in future submissions. Thanks, - Haiyang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Haiyang Zhang <haiyangz@microsoft.com> Date: Mon, 12 Mar 2012 15:39:02 +0000 > And, thanks for the reminder, I will specify the target tree in future submissions. You still have to formally resubmit these patches, with the proper indication of where the patches go. Just replying to my complaint with an answer won't make your patches get replied, as I've completely dropped this submission. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Monday, March 12, 2012 3:51 PM > To: Haiyang Zhang > Cc: KY Srinivasan; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org > Subject: Re: [PATCH 1/2] net/hyperv: Fix data corruption in > rndis_filter_receive() > > From: Haiyang Zhang <haiyangz@microsoft.com> > Date: Mon, 12 Mar 2012 15:39:02 +0000 > > > And, thanks for the reminder, I will specify the target tree in future > submissions. > > You still have to formally resubmit these patches, with the proper indication > of where the patches go. Just replying to my complaint with an answer > won't make your patches get replied, as I've completely dropped this > submission. Sure, I'm re-submitting them right now. Thanks, - Haiyang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 136efd8..0c3d7d9 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -352,8 +352,7 @@ int rndis_filter_receive(struct hv_device *dev, { struct netvsc_device *net_dev = hv_get_drvdata(dev); struct rndis_device *rndis_dev; - struct rndis_message rndis_msg; - struct rndis_message *rndis_hdr; + struct rndis_message *rndis_msg; struct net_device *ndev; if (!net_dev) @@ -375,46 +374,32 @@ int rndis_filter_receive(struct hv_device *dev, return -ENODEV; } - rndis_hdr = pkt->data; - - /* Make sure we got a valid rndis message */ - if ((rndis_hdr->ndis_msg_type != REMOTE_NDIS_PACKET_MSG) && - (rndis_hdr->msg_len > sizeof(struct rndis_message))) { - netdev_err(ndev, "incoming rndis message buffer overflow " - "detected (got %u, max %zu)..marking it an error!\n", - rndis_hdr->msg_len, - sizeof(struct rndis_message)); - } + rndis_msg = pkt->data; - memcpy(&rndis_msg, rndis_hdr, - (rndis_hdr->msg_len > sizeof(struct rndis_message)) ? - sizeof(struct rndis_message) : - rndis_hdr->msg_len); + dump_rndis_message(dev, rndis_msg); - dump_rndis_message(dev, &rndis_msg); - - switch (rndis_msg.ndis_msg_type) { + switch (rndis_msg->ndis_msg_type) { case REMOTE_NDIS_PACKET_MSG: /* data msg */ - rndis_filter_receive_data(rndis_dev, &rndis_msg, pkt); + rndis_filter_receive_data(rndis_dev, rndis_msg, pkt); break; case REMOTE_NDIS_INITIALIZE_CMPLT: case REMOTE_NDIS_QUERY_CMPLT: case REMOTE_NDIS_SET_CMPLT: /* completion msgs */ - rndis_filter_receive_response(rndis_dev, &rndis_msg); + rndis_filter_receive_response(rndis_dev, rndis_msg); break; case REMOTE_NDIS_INDICATE_STATUS_MSG: /* notification msgs */ - rndis_filter_receive_indicate_status(rndis_dev, &rndis_msg); + rndis_filter_receive_indicate_status(rndis_dev, rndis_msg); break; default: netdev_err(ndev, "unhandled rndis message (type %u len %u)\n", - rndis_msg.ndis_msg_type, - rndis_msg.msg_len); + rndis_msg->ndis_msg_type, + rndis_msg->msg_len); break; }