| Message ID | 1475175454-3116-2-git-send-email-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016年09月30日 02:57, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > 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). > > Reported-by: Li Qiang <liqiang6-s@360.cn> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > 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;
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
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
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;