viosock: fix sending large packet sg list

[Jing118]

If the first element in SgList is not 4KB, there will be 17 element in SgList for 64KB data, which will cause a stack buffer overrun BSOD. This issue can be reproduced by sending more then 64KB data in one send().

From #957

For the TX fix, it is a bad practice to allocate memory on stack for SG in any case. I suggest to make a real fix that will allocate memory properly (also with consideration of probable usage of IOMMU) and not workaround.

SG is only used to pass addr and length to virtqueue_add(), so I don't understand why it's a bad practice to allocate memory on stack for SG. I also checked other virtio drivers and found some of them use stack memory as well.

[YanVugenfirer]

If the first element in SgList is not 4KB, there will be 17 element in SgList for 64KB data, which will cause a stack buffer overrun BSOD. This issue can be reproduced by sending more than 64KB data in one send().

But there are already 17 elements. Because

#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE       (1024 * 64)
#define VIOSOCK_DMA_TX_PAGES BYTES_TO_PAGES(VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)

VIOSOCK_SG_DESC sg[VIOSOCK_DMA_TX_PAGES + 1];

Can you please explain how for example 64K+3pages of data will be transmitted?

Could be that we should check that he transmitted data not over VIRTIO_VSOCK_MAX_PKT_BUF_SIZE and fragment or reject transmission?

From #957

For the TX fix, it is a bad practice to allocate memory on stack for SG in any case. I suggest to make a real fix that will allocate memory properly (also with consideration of probable usage of IOMMU) and not workaround.

SG is only used to pass addr and length to virtqueue_add(), so I don't understand why it's a bad practice to allocate memory on stack for SG. I also checked other virtio drivers and found some of them use stack memory as well. You are right. My confusion, this is an intermediate array and the data copied in add_buf

Please add all explanations to commit message.

[Jing118]

Sorry if I was not clear, before there were 17 elements in sgList, 1 for the header and 16 for the data. So if data is not 4KB aligned, 64KB data will need 17 elements, resulting in 18 elements in total (sg[0] is the header):

In VIOSockWrite() we have limited the maximum length can be sent in one send() call to VIRTIO_VSOCK_MAX_PKT_BUF_SIZE, so there will not be more than 64KB data in VIOSockTxPktInsert().