Re: [Qemu-devel] [PATCH 1/2] net: pcnet: check rx/tx descriptor ring length
On 2016年09月30日 13:36, P J P wrote: Hello Jason, +-- On Fri, 30 Sep 2016, Jason Wang wrote --+ | On 2016年09月30日 02:57, P J P wrote: | > The AMD PC-Net II emulator has set of control and status(CSR) | > registers. Of these, CSR76 and CSR78 hold receive and transmit | > descriptor ring length respectively. This ring length could range | > from 1 to 65535. Setting ring length to zero leads to an infinite | > loop in pcnet_rdra_addr. Add check to avoid it. | | In this case, we only need to protect RCVRL I believe? (since XMTRL were not | used). XMTRL is not used in this case, but could be prone to similar issues. For ex. static void pcnet_transmit(PCNetState *s) { int count = CSR_XMTRL(s) - 1; ... if (count--) goto txagain; } If CSR_XMTRL is set to zero(0), 'count' would never reach zero and function would continue to jump to 'txagain'. Applied and tweak the commit log by mentioning pcnet_transmit() too. Thanks Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH 1/2] net: pcnet: check rx/tx descriptor ring length
Hello Jason, +-- On Fri, 30 Sep 2016, Jason Wang wrote --+ | On 2016年09月30日 02:57, P J P wrote: | > The AMD PC-Net II emulator has set of control and status(CSR) | > registers. Of these, CSR76 and CSR78 hold receive and transmit | > descriptor ring length respectively. This ring length could range | > from 1 to 65535. Setting ring length to zero leads to an infinite | > loop in pcnet_rdra_addr. Add check to avoid it. | | In this case, we only need to protect RCVRL I believe? (since XMTRL were not | used). XMTRL is not used in this case, but could be prone to similar issues. For ex. static void pcnet_transmit(PCNetState *s) { int count = CSR_XMTRL(s) - 1; ... if (count--) goto txagain; } If CSR_XMTRL is set to zero(0), 'count' would never reach zero and function would continue to jump to 'txagain'. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH 1/2] net: pcnet: check rx/tx descriptor ring length
On 2016年09月30日 02:57, P J P wrote: From: Prasad J PanditThe AMD PC-Net II emulator has set of control and status(CSR) registers. Of these, CSR76 and CSR78 hold receive and transmit descriptor ring length respectively. This ring length could range from 1 to 65535. Setting ring length to zero leads to an infinite loop in pcnet_rdra_addr. Add check to avoid it. In this case, we only need to protect RCVRL I believe? (since XMTRL were not used). Reported-by: Li Qiang Signed-off-by: Prasad J Pandit --- hw/net/pcnet.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 198a01f..3078de8 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1429,8 +1429,11 @@ static void pcnet_csr_writew(PCNetState *s, uint32_t rap, uint32_t new_value) case 47: /* POLLINT */ case 72: case 74: +break; case 76: /* RCVRL */ case 78: /* XMTRL */ +val = (val > 0) ? val : 512; +break; case 112: if (CSR_STOP(s) || CSR_SPND(s)) break;
[Qemu-devel] [PATCH 1/2] net: pcnet: check rx/tx descriptor ring length
From: Prasad J PanditThe AMD PC-Net II emulator has set of control and status(CSR) registers. Of these, CSR76 and CSR78 hold receive and transmit descriptor ring length respectively. This ring length could range from 1 to 65535. Setting ring length to zero leads to an infinite loop in pcnet_rdra_addr. Add check to avoid it. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit --- hw/net/pcnet.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 198a01f..3078de8 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1429,8 +1429,11 @@ static void pcnet_csr_writew(PCNetState *s, uint32_t rap, uint32_t new_value) case 47: /* POLLINT */ case 72: case 74: +break; case 76: /* RCVRL */ case 78: /* XMTRL */ +val = (val > 0) ? val : 512; +break; case 112: if (CSR_STOP(s) || CSR_SPND(s)) break; -- 2.5.5