| Message ID | 1418203056-5365-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi paolo, Will this change affects the migration? I noticed that there is a member 'SuperIOConfig superio_conf' in VT82C686BState. vt82c686 seems only to be used in mips64el target, Do we support migration for mips target? Thanks, zhanghailiang On 2014/12/10 17:17, Paolo Bonzini wrote: > superio_ioport_readb can read the 256th element of the array. > Coverity reports an out-of-bounds write in superio_ioport_writeb, > but it does not show the corresponding out-of-bounds read > because it cannot prove that it can happen. Fix the root > cause of the problem (zhanghailang's patch instead fixes > the logic in superio_ioport_writeb). > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/isa/vt82c686.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index e0c235c..a43e26d 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -36,7 +36,7 @@ > > typedef struct SuperIOConfig > { > - uint8_t config[0xff]; > + uint8_t config[0x100]; > uint8_t index; > uint8_t data; > } SuperIOConfig; >
On 10/12/2014 10:31, zhanghailiang wrote: > Hi paolo, > > Will this change affects the migration? > I noticed that there is a member 'SuperIOConfig superio_conf' in > VT82C686BState. > > vt82c686 seems only to be used in mips64el target, Do we support > migration for mips target? No, there is no VMState for superio_conf. Paolo
On 2014/12/10 17:17, Paolo Bonzini wrote: > superio_ioport_readb can read the 256th element of the array. > Coverity reports an out-of-bounds write in superio_ioport_writeb, > but it does not show the corresponding out-of-bounds read > because it cannot prove that it can happen. Fix the root > cause of the problem (zhanghailang's patch instead fixes > the logic in superio_ioport_writeb). > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/isa/vt82c686.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index e0c235c..a43e26d 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -36,7 +36,7 @@ > > typedef struct SuperIOConfig > { > - uint8_t config[0xff]; > + uint8_t config[0x100]; > uint8_t index; > uint8_t data; > } SuperIOConfig; > Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
10.12.2014 12:17, Paolo Bonzini wrote:
> superio_ioport_readb can read the 256th element of the array.
Is there a legitimate reason for it to access byte index 256?
What is the actual size of superio config memory, 256 or 257?
I don't know, but somehow it looks like it should be 256.
If that's the case, the fix is wrong and superio_ioport_readb()
should be fixed instead...
Thanks,
/mjt
On 11/12/2014 18:55, Michael Tokarev wrote: >> > superio_ioport_readb can read the 256th element of the array. > Is there a legitimate reason for it to access byte index 256? The 256th element is byte index 255. :) > What is the actual size of superio config memory, 256 or 257? It's 256 and the array is sized conf[0xff]. > I don't know, but somehow it looks like it should be 256. That's what the patch does. :) Paolo
10.12.2014 12:17, Paolo Bonzini wrote: > superio_ioport_readb can read the 256th element of the array. > Coverity reports an out-of-bounds write in superio_ioport_writeb, > but it does not show the corresponding out-of-bounds read > because it cannot prove that it can happen. Fix the root > cause of the problem (zhanghailang's patch instead fixes > the logic in superio_ioport_writeb). (Finally) applied to -trivial, thanks! /mjt
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index e0c235c..a43e26d 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -36,7 +36,7 @@ typedef struct SuperIOConfig { - uint8_t config[0xff]; + uint8_t config[0x100]; uint8_t index; uint8_t data; } SuperIOConfig;
superio_ioport_readb can read the 256th element of the array. Coverity reports an out-of-bounds write in superio_ioport_writeb, but it does not show the corresponding out-of-bounds read because it cannot prove that it can happen. Fix the root cause of the problem (zhanghailang's patch instead fixes the logic in superio_ioport_writeb). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/isa/vt82c686.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)