Skip to content

Commit 505e3f0

Browse files
andrea-parrikuba-moo
andrea-parri
authored and
kuba-moo
committed
hv_netvsc: Add (more) validation for untrusted Hyper-V values
For additional robustness in the face of Hyper-V errors or malicious behavior, validate all values that originate from packets that Hyper-V has sent to the guest. Ensure that invalid values cannot cause indexing off the end of an array, or subvert an existing validation via integer overflow. Ensure that outgoing packets do not have any leftover guest memory that has not been zeroed out. Reported-by: Juan Vazquez <[email protected]> Signed-off-by: Andrea Parri (Microsoft) <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent a98c0c4 commit 505e3f0

File tree

4 files changed

+136
-62
lines changed

4 files changed

+136
-62
lines changed

‎drivers/net/hyperv/netvsc.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,7 @@ static inline int netvsc_send_pkt(
918918
int ret;
919919
u32 ring_avail = hv_get_avail_to_write_percent(&out_channel->outbound);
920920

921+
memset(&nvmsg, 0, sizeof(struct nvsp_message));
921922
nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
922923
if (skb)
923924
rpkt->channel_type = 0; /* 0 is RMC_DATA */
@@ -1337,7 +1338,7 @@ static void netvsc_send_table(struct net_device *ndev,
13371338
sizeof(union nvsp_6_message_uber);
13381339

13391340
/* Boundary check for all versions */
1340-
if (offset > msglen - count * sizeof(u32)) {
1341+
if (msglen < count * sizeof(u32) || offset > msglen - count * sizeof(u32)) {
13411342
netdev_err(ndev, "Received send-table offset too big:%u\n",
13421343
offset);
13431344
return;

‎drivers/net/hyperv/netvsc_bpf.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,
3737
if (!prog)
3838
goto out;
3939

40+
/* Ensure that the below memcpy() won't overflow the page buffer. */
41+
if (len > ndev->mtu + ETH_HLEN) {
42+
act = XDP_DROP;
43+
goto out;
44+
}
45+
4046
/* allocate page buffer for data */
4147
page = alloc_page(GFP_ATOMIC);
4248
if (!page) {

‎drivers/net/hyperv/netvsc_drv.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,16 @@ void netvsc_linkstatus_callback(struct net_device *net,
761761
if (indicate->status == RNDIS_STATUS_LINK_SPEED_CHANGE) {
762762
u32 speed;
763763

764+
/* Validate status_buf_offset */
765+
if (indicate->status_buflen < sizeof(speed) ||
766+
indicate->status_buf_offset < sizeof(*indicate) ||
767+
resp->msg_len - RNDIS_HEADER_SIZE < indicate->status_buf_offset ||
768+
resp->msg_len - RNDIS_HEADER_SIZE - indicate->status_buf_offset
769+
< indicate->status_buflen) {
770+
netdev_err(net, "invalid rndis_indicate_status packet\n");
771+
return;
772+
}
773+
764774
speed = *(u32 *)((void *)indicate
765775
+ indicate->status_buf_offset) / 10000;
766776
ndev_ctx->speed = speed;
@@ -866,8 +876,14 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
866876
*/
867877
if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
868878
csum_info->receive.ip_checksum_succeeded &&
869-
skb->protocol == htons(ETH_P_IP))
879+
skb->protocol == htons(ETH_P_IP)) {
880+
/* Check that there is enough space to hold the IP header. */
881+
if (skb_headlen(skb) < sizeof(struct iphdr)) {
882+
kfree_skb(skb);
883+
return NULL;
884+
}
870885
netvsc_comp_ipcsum(skb);
886+
}
871887

872888
/* Do L4 checksum offload if enabled and present. */
873889
if (csum_info && (net->features & NETIF_F_RXCSUM)) {

‎drivers/net/hyperv/rndis_filter.c

Lines changed: 111 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -131,66 +131,84 @@ static void dump_rndis_message(struct net_device *netdev,
131131
{
132132
switch (rndis_msg->ndis_msg_type) {
133133
case RNDIS_MSG_PACKET:
134-
netdev_dbg(netdev, "RNDIS_MSG_PACKET (len %u, "
135-
"data offset %u data len %u, # oob %u, "
136-
"oob offset %u, oob len %u, pkt offset %u, "
137-
"pkt len %u\n",
138-
rndis_msg->msg_len,
139-
rndis_msg->msg.pkt.data_offset,
140-
rndis_msg->msg.pkt.data_len,
141-
rndis_msg->msg.pkt.num_oob_data_elements,
142-
rndis_msg->msg.pkt.oob_data_offset,
143-
rndis_msg->msg.pkt.oob_data_len,
144-
rndis_msg->msg.pkt.per_pkt_info_offset,
145-
rndis_msg->msg.pkt.per_pkt_info_len);
134+
if (rndis_msg->msg_len - RNDIS_HEADER_SIZE >= sizeof(struct rndis_packet)) {
135+
const struct rndis_packet *pkt = &rndis_msg->msg.pkt;
136+
netdev_dbg(netdev, "RNDIS_MSG_PACKET (len %u, "
137+
"data offset %u data len %u, # oob %u, "
138+
"oob offset %u, oob len %u, pkt offset %u, "
139+
"pkt len %u\n",
140+
rndis_msg->msg_len,
141+
pkt->data_offset,
142+
pkt->data_len,
143+
pkt->num_oob_data_elements,
144+
pkt->oob_data_offset,
145+
pkt->oob_data_len,
146+
pkt->per_pkt_info_offset,
147+
pkt->per_pkt_info_len);
148+
}
146149
break;
147150

148151
case RNDIS_MSG_INIT_C:
149-
netdev_dbg(netdev, "RNDIS_MSG_INIT_C "
150-
"(len %u, id 0x%x, status 0x%x, major %d, minor %d, "
151-
"device flags %d, max xfer size 0x%x, max pkts %u, "
152-
"pkt aligned %u)\n",
153-
rndis_msg->msg_len,
154-
rndis_msg->msg.init_complete.req_id,
155-
rndis_msg->msg.init_complete.status,
156-
rndis_msg->msg.init_complete.major_ver,
157-
rndis_msg->msg.init_complete.minor_ver,
158-
rndis_msg->msg.init_complete.dev_flags,
159-
rndis_msg->msg.init_complete.max_xfer_size,
160-
rndis_msg->msg.init_complete.
161-
max_pkt_per_msg,
162-
rndis_msg->msg.init_complete.
163-
pkt_alignment_factor);
152+
if (rndis_msg->msg_len - RNDIS_HEADER_SIZE >=
153+
sizeof(struct rndis_initialize_complete)) {
154+
const struct rndis_initialize_complete *init_complete =
155+
&rndis_msg->msg.init_complete;
156+
netdev_dbg(netdev, "RNDIS_MSG_INIT_C "
157+
"(len %u, id 0x%x, status 0x%x, major %d, minor %d, "
158+
"device flags %d, max xfer size 0x%x, max pkts %u, "
159+
"pkt aligned %u)\n",
160+
rndis_msg->msg_len,
161+
init_complete->req_id,
162+
init_complete->status,
163+
init_complete->major_ver,
164+
init_complete->minor_ver,
165+
init_complete->dev_flags,
166+
init_complete->max_xfer_size,
167+
init_complete->max_pkt_per_msg,
168+
init_complete->pkt_alignment_factor);
169+
}
164170
break;
165171

166172
case RNDIS_MSG_QUERY_C:
167-
netdev_dbg(netdev, "RNDIS_MSG_QUERY_C "
168-
"(len %u, id 0x%x, status 0x%x, buf len %u, "
169-
"buf offset %u)\n",
170-
rndis_msg->msg_len,
171-
rndis_msg->msg.query_complete.req_id,
172-
rndis_msg->msg.query_complete.status,
173-
rndis_msg->msg.query_complete.
174-
info_buflen,
175-
rndis_msg->msg.query_complete.
176-
info_buf_offset);
173+
if (rndis_msg->msg_len - RNDIS_HEADER_SIZE >=
174+
sizeof(struct rndis_query_complete)) {
175+
const struct rndis_query_complete *query_complete =
176+
&rndis_msg->msg.query_complete;
177+
netdev_dbg(netdev, "RNDIS_MSG_QUERY_C "
178+
"(len %u, id 0x%x, status 0x%x, buf len %u, "
179+
"buf offset %u)\n",
180+
rndis_msg->msg_len,
181+
query_complete->req_id,
182+
query_complete->status,
183+
query_complete->info_buflen,
184+
query_complete->info_buf_offset);
185+
}
177186
break;
178187

179188
case RNDIS_MSG_SET_C:
180-
netdev_dbg(netdev,
181-
"RNDIS_MSG_SET_C (len %u, id 0x%x, status 0x%x)\n",
182-
rndis_msg->msg_len,
183-
rndis_msg->msg.set_complete.req_id,
184-
rndis_msg->msg.set_complete.status);
189+
if (rndis_msg->msg_len - RNDIS_HEADER_SIZE + sizeof(struct rndis_set_complete)) {
190+
const struct rndis_set_complete *set_complete =
191+
&rndis_msg->msg.set_complete;
192+
netdev_dbg(netdev,
193+
"RNDIS_MSG_SET_C (len %u, id 0x%x, status 0x%x)\n",
194+
rndis_msg->msg_len,
195+
set_complete->req_id,
196+
set_complete->status);
197+
}
185198
break;
186199

187200
case RNDIS_MSG_INDICATE:
188-
netdev_dbg(netdev, "RNDIS_MSG_INDICATE "
189-
"(len %u, status 0x%x, buf len %u, buf offset %u)\n",
190-
rndis_msg->msg_len,
191-
rndis_msg->msg.indicate_status.status,
192-
rndis_msg->msg.indicate_status.status_buflen,
193-
rndis_msg->msg.indicate_status.status_buf_offset);
201+
if (rndis_msg->msg_len - RNDIS_HEADER_SIZE >=
202+
sizeof(struct rndis_indicate_status)) {
203+
const struct rndis_indicate_status *indicate_status =
204+
&rndis_msg->msg.indicate_status;
205+
netdev_dbg(netdev, "RNDIS_MSG_INDICATE "
206+
"(len %u, status 0x%x, buf len %u, buf offset %u)\n",
207+
rndis_msg->msg_len,
208+
indicate_status->status,
209+
indicate_status->status_buflen,
210+
indicate_status->status_buf_offset);
211+
}
194212
break;
195213

196214
default:
@@ -246,11 +264,20 @@ static void rndis_set_link_state(struct rndis_device *rdev,
246264
{
247265
u32 link_status;
248266
struct rndis_query_complete *query_complete;
267+
u32 msg_len = request->response_msg.msg_len;
268+
269+
/* Ensure the packet is big enough to access its fields */
270+
if (msg_len - RNDIS_HEADER_SIZE < sizeof(struct rndis_query_complete))
271+
return;
249272

250273
query_complete = &request->response_msg.msg.query_complete;
251274

252275
if (query_complete->status == RNDIS_STATUS_SUCCESS &&
253-
query_complete->info_buflen == sizeof(u32)) {
276+
query_complete->info_buflen >= sizeof(u32) &&
277+
query_complete->info_buf_offset >= sizeof(*query_complete) &&
278+
msg_len - RNDIS_HEADER_SIZE >= query_complete->info_buf_offset &&
279+
msg_len - RNDIS_HEADER_SIZE - query_complete->info_buf_offset
280+
>= query_complete->info_buflen) {
254281
memcpy(&link_status, (void *)((unsigned long)query_complete +
255282
query_complete->info_buf_offset), sizeof(u32));
256283
rdev->link_state = link_status != 0;
@@ -343,7 +370,8 @@ static void rndis_filter_receive_response(struct net_device *ndev,
343370
*/
344371
static inline void *rndis_get_ppi(struct net_device *ndev,
345372
struct rndis_packet *rpkt,
346-
u32 rpkt_len, u32 type, u8 internal)
373+
u32 rpkt_len, u32 type, u8 internal,
374+
u32 ppi_size)
347375
{
348376
struct rndis_per_packet_info *ppi;
349377
int len;
@@ -359,7 +387,8 @@ static inline void *rndis_get_ppi(struct net_device *ndev,
359387
return NULL;
360388
}
361389

362-
if (rpkt->per_pkt_info_len > rpkt_len - rpkt->per_pkt_info_offset) {
390+
if (rpkt->per_pkt_info_len < sizeof(*ppi) ||
391+
rpkt->per_pkt_info_len > rpkt_len - rpkt->per_pkt_info_offset) {
363392
netdev_err(ndev, "Invalid per_pkt_info_len: %u\n",
364393
rpkt->per_pkt_info_len);
365394
return NULL;
@@ -381,8 +410,15 @@ static inline void *rndis_get_ppi(struct net_device *ndev,
381410
continue;
382411
}
383412

384-
if (ppi->type == type && ppi->internal == internal)
413+
if (ppi->type == type && ppi->internal == internal) {
414+
/* ppi->size should be big enough to hold the returned object. */
415+
if (ppi->size - ppi->ppi_offset < ppi_size) {
416+
netdev_err(ndev, "Invalid ppi: size %u ppi_offset %u\n",
417+
ppi->size, ppi->ppi_offset);
418+
continue;
419+
}
385420
return (void *)((ulong)ppi + ppi->ppi_offset);
421+
}
386422
len -= ppi->size;
387423
ppi = (struct rndis_per_packet_info *)((ulong)ppi + ppi->size);
388424
}
@@ -461,13 +497,16 @@ static int rndis_filter_receive_data(struct net_device *ndev,
461497
return NVSP_STAT_FAIL;
462498
}
463499

464-
vlan = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, IEEE_8021Q_INFO, 0);
500+
vlan = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, IEEE_8021Q_INFO, 0, sizeof(*vlan));
465501

466-
csum_info = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, TCPIP_CHKSUM_PKTINFO, 0);
502+
csum_info = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, TCPIP_CHKSUM_PKTINFO, 0,
503+
sizeof(*csum_info));
467504

468-
hash_info = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, NBL_HASH_VALUE, 0);
505+
hash_info = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, NBL_HASH_VALUE, 0,
506+
sizeof(*hash_info));
469507

470-
pktinfo_id = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, RNDIS_PKTINFO_ID, 1);
508+
pktinfo_id = rndis_get_ppi(ndev, rndis_pkt, rpkt_len, RNDIS_PKTINFO_ID, 1,
509+
sizeof(*pktinfo_id));
471510

472511
data = (void *)msg + data_offset;
473512

@@ -522,9 +561,6 @@ int rndis_filter_receive(struct net_device *ndev,
522561
struct net_device_context *net_device_ctx = netdev_priv(ndev);
523562
struct rndis_message *rndis_msg = data;
524563

525-
if (netif_msg_rx_status(net_device_ctx))
526-
dump_rndis_message(ndev, rndis_msg);
527-
528564
/* Validate incoming rndis_message packet */
529565
if (buflen < RNDIS_HEADER_SIZE || rndis_msg->msg_len < RNDIS_HEADER_SIZE ||
530566
buflen < rndis_msg->msg_len) {
@@ -533,6 +569,9 @@ int rndis_filter_receive(struct net_device *ndev,
533569
return NVSP_STAT_FAIL;
534570
}
535571

572+
if (netif_msg_rx_status(net_device_ctx))
573+
dump_rndis_message(ndev, rndis_msg);
574+
536575
switch (rndis_msg->ndis_msg_type) {
537576
case RNDIS_MSG_PACKET:
538577
return rndis_filter_receive_data(ndev, net_dev, nvchan,
@@ -567,6 +606,7 @@ static int rndis_filter_query_device(struct rndis_device *dev,
567606
u32 inresult_size = *result_size;
568607
struct rndis_query_request *query;
569608
struct rndis_query_complete *query_complete;
609+
u32 msg_len;
570610
int ret = 0;
571611

572612
if (!result)
@@ -634,8 +674,19 @@ static int rndis_filter_query_device(struct rndis_device *dev,
634674

635675
/* Copy the response back */
636676
query_complete = &request->response_msg.msg.query_complete;
677+
msg_len = request->response_msg.msg_len;
678+
679+
/* Ensure the packet is big enough to access its fields */
680+
if (msg_len - RNDIS_HEADER_SIZE < sizeof(struct rndis_query_complete)) {
681+
ret = -1;
682+
goto cleanup;
683+
}
637684

638-
if (query_complete->info_buflen > inresult_size) {
685+
if (query_complete->info_buflen > inresult_size ||
686+
query_complete->info_buf_offset < sizeof(*query_complete) ||
687+
msg_len - RNDIS_HEADER_SIZE < query_complete->info_buf_offset ||
688+
msg_len - RNDIS_HEADER_SIZE - query_complete->info_buf_offset
689+
< query_complete->info_buflen) {
639690
ret = -1;
640691
goto cleanup;
641692
}

0 commit comments

Comments
 (0)