| Message ID | 20190214201939.494-4-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ccid-card-passthru: check buffer size parameter | expand |
Would it be better to have some description? On Thu, Feb 14, 2019 at 09:19:33PM +0100, Philippe Mathieu-Daudé wrote: >Suggested-by: Paolo Bonzini <pbonzini@redhat.com> >Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >--- > hw/usb/ccid-card-passthru.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c >index 1676b5fc05..0c44b38fc2 100644 >--- a/hw/usb/ccid-card-passthru.c >+++ b/hw/usb/ccid-card-passthru.c >@@ -270,7 +270,7 @@ static void ccid_card_vscard_read(void *opaque, const uint8_t *buf, int size) > VSCMsgHeader *hdr; > > assert(size <= VSCARD_IN_SIZE - card->vscard_in_pos); >- assert(card->vscard_in_hdr < VSCARD_IN_SIZE); >+ assert(card->vscard_in_hdr < card->vscard_in_pos); > memcpy(card->vscard_in_data + card->vscard_in_pos, buf, size); > card->vscard_in_pos += size; > hdr = (VSCMsgHeader *)(card->vscard_in_data + card->vscard_in_hdr); >-- >2.20.1 >
Hi On Thu, Feb 14, 2019 at 9:20 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/usb/ccid-card-passthru.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c > index 1676b5fc05..0c44b38fc2 100644 > --- a/hw/usb/ccid-card-passthru.c > +++ b/hw/usb/ccid-card-passthru.c > @@ -270,7 +270,7 @@ static void ccid_card_vscard_read(void *opaque, const uint8_t *buf, int size) > VSCMsgHeader *hdr; > > assert(size <= VSCARD_IN_SIZE - card->vscard_in_pos); > - assert(card->vscard_in_hdr < VSCARD_IN_SIZE); > + assert(card->vscard_in_hdr < card->vscard_in_pos); I think you need "<=". card->vscard_in_hdr is updated in the loop and may reach card->vscard_in_pos: while ((card->vscard_in_pos - card->vscard_in_hdr >= sizeof(VSCMsgHeader)) &&(card->vscard_in_pos - card->vscard_in_hdr >= sizeof(VSCMsgHeader) + ntohl(hdr->length))) { ... card->vscard_in_hdr += hdr->length + sizeof(VSCMsgHeader); ... } if "hdr->length + sizeof(VSCMsgHeader)" == "card->vscard_in_pos - card->vscard_in_hdr", card->vscard_in_hdr == card->vscard_in_pos after the loop. I think the existing assert() could also stay. > memcpy(card->vscard_in_data + card->vscard_in_pos, buf, size); > card->vscard_in_pos += size; > hdr = (VSCMsgHeader *)(card->vscard_in_data + card->vscard_in_hdr); > -- > 2.20.1 >
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c index 1676b5fc05..0c44b38fc2 100644 --- a/hw/usb/ccid-card-passthru.c +++ b/hw/usb/ccid-card-passthru.c @@ -270,7 +270,7 @@ static void ccid_card_vscard_read(void *opaque, const uint8_t *buf, int size) VSCMsgHeader *hdr; assert(size <= VSCARD_IN_SIZE - card->vscard_in_pos); - assert(card->vscard_in_hdr < VSCARD_IN_SIZE); + assert(card->vscard_in_hdr < card->vscard_in_pos); memcpy(card->vscard_in_data + card->vscard_in_pos, buf, size); card->vscard_in_pos += size; hdr = (VSCMsgHeader *)(card->vscard_in_data + card->vscard_in_hdr);
Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/usb/ccid-card-passthru.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)