Re: Build problems in kdd.c with xen-4.14.0-rc4
On Fri, Jul 03, 2020 at 07:54:57PM +0100, Tim Deegan wrote:
> Hi Michael,
>
> Thanks for ther report!
>
> At 23:21 +0100 on 30 Jun (1593559296), Michael Young wrote:
> > I get the following errors when trying to build xen-4.14.0-rc4
> >
> > kdd.c: In function 'kdd_tx':
> > kdd.c:754:15: error: array subscript 16 is above array bounds of
> > 'uint8_t[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
> >754 | s->txb[len++] = 0xaa;
> >| ~~^~~
> > kdd.c:82:17: note: while referencing 'txb'
> > 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling
> > area for tx */
> >| ^~~
> > kdd.c: In function 'kdd_break':
> > kdd.c:819:19: error: array subscript 16 is above array bounds of
> > 'uint8_t[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
>
> Oh dear. The fix for the last kdd bug seems to have gone wrong
> somewhere. The patch I posted has:
>
> -uint8_t txb[sizeof (kdd_hdr) + 65536]; /* Marshalling area for tx
> */
> +uint8_t txb[sizeof (kdd_pkt)]; /* Marshalling area for tx
> */
>
> but as applied in master it's:
>
> -uint8_t txb[sizeof (kdd_hdr) + 65536]; /* Marshalling area for tx
> */
> +uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area for tx
> */
>
> i.e. the marshalling area is only large enough for a header and GCC
> is correctly complaining about that.
>
> Wei, it looks like you committed this patch - can you figure out what
> happened to it please?
>
My bad. The mail I saved did not apply cleanly so I manually recreated
your patch.
Thanks for letting me know. I will send a patch to fix the issue.
Wei.
> Cheers,
>
> Tim.
Re: Build problems in kdd.c with xen-4.14.0-rc4
Hi Michael,
Thanks for ther report!
At 23:21 +0100 on 30 Jun (1593559296), Michael Young wrote:
> I get the following errors when trying to build xen-4.14.0-rc4
>
> kdd.c: In function 'kdd_tx':
> kdd.c:754:15: error: array subscript 16 is above array bounds of
> 'uint8_t[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
>754 | s->txb[len++] = 0xaa;
>| ~~^~~
> kdd.c:82:17: note: while referencing 'txb'
> 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
> for tx */
>| ^~~
> kdd.c: In function 'kdd_break':
> kdd.c:819:19: error: array subscript 16 is above array bounds of
> 'uint8_t[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
Oh dear. The fix for the last kdd bug seems to have gone wrong
somewhere. The patch I posted has:
-uint8_t txb[sizeof (kdd_hdr) + 65536]; /* Marshalling area for tx */
+uint8_t txb[sizeof (kdd_pkt)]; /* Marshalling area for tx */
but as applied in master it's:
-uint8_t txb[sizeof (kdd_hdr) + 65536]; /* Marshalling area for tx */
+uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area for tx */
i.e. the marshalling area is only large enough for a header and GCC
is correctly complaining about that.
Wei, it looks like you committed this patch - can you figure out what
happened to it please?
Cheers,
Tim.
Re: Build problems in kdd.c with xen-4.14.0-rc4
Am Fri, 3 Jul 2020 14:23:14 +0100 schrieb Paul Durrant : > That doesn't look quite right. That might be true. I do not debug windows, and it makes gcc happy... Olaf pgp2PK2BM9w6V.pgp Description: Digitale Signatur von OpenPGP
RE: Build problems in kdd.c with xen-4.14.0-rc4
> -Original Message-
> From: Xen-devel On Behalf Of Olaf
> Hering
> Sent: 02 July 2020 19:38
> To: Michael Young
> Cc: xen-devel@lists.xenproject.org; Tim Deegan
> Subject: Re: Build problems in kdd.c with xen-4.14.0-rc4
>
> On Tue, Jun 30, Michael Young wrote:
>
> > I get the following errors when trying to build xen-4.14.0-rc4
>
> This happens to work for me.
>
> Olaf
>
> ---
> tools/debugger/kdd/kdd.c | 8
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -742,25 +742,25 @@ static void kdd_tx(kdd_state *s)
> int i;
>
> /* Fix up the checksum before we send */
> for (i = 0; i < s->txp.h.len; i++)
> sum += s->txp.payload[i];
> s->txp.h.sum = sum;
>
> kdd_log_pkt(s, "TX", &s->txp);
>
> len = s->txp.h.len + sizeof (kdd_hdr);
> if (s->txp.h.dir == KDD_DIR_PKT)
> /* Append the mysterious 0xaa byte to each packet */
> -s->txb[len++] = 0xaa;
> +s->txp.payload[len++] = 0xaa;
That doesn't look quite right. I think you need [len++ - sizeof(kdd_hdr)] there.
>
> (void) blocking_write(s->fd, s->txb, len);
> }
>
>
> /* Send an acknowledgement to the client */
> static void kdd_send_ack(kdd_state *s, uint32_t id, uint16_t type)
> {
> s->txp.h.dir = KDD_DIR_ACK;
> s->txp.h.type = type;
> s->txp.h.len = 0;
> s->txp.h.id = id;
> @@ -775,25 +775,25 @@ static void kdd_send_cmd(kdd_state *s, uint32_t
> subtype, size_t extra)
> s->txp.h.type = KDD_PKT_CMD;
> s->txp.h.len = sizeof (kdd_cmd) + extra;
> s->txp.h.id = (s->next_id ^= 1);
> s->txp.h.sum = 0;
> s->txp.cmd.subtype = subtype;
> kdd_tx(s);
> }
>
> /* Cause the client to print a string */
> static void kdd_send_string(kdd_state *s, char *fmt, ...)
> {
> uint32_t len = 0x - sizeof (kdd_msg);
> -char *buf = (char *) s->txb + sizeof (kdd_hdr) + sizeof (kdd_msg);
> +char *buf = (char *) &s->txp + sizeof (kdd_hdr) + sizeof (kdd_msg);
> va_list ap;
>
> va_start(ap, fmt);
> len = vsnprintf(buf, len, fmt, ap);
> va_end(ap);
>
> s->txp.h.dir = KDD_DIR_PKT;
> s->txp.h.type = KDD_PKT_MSG;
> s->txp.h.len = sizeof (kdd_msg) + len;
> s->txp.h.id = (s->next_id ^= 1);
> s->txp.h.sum = 0;
> s->txp.msg.subtype = KDD_MSG_PRINT;
> @@ -807,25 +807,25 @@ static void kdd_break(kdd_state *s)
> {
> uint16_t ilen;
> KDD_LOG(s, "Break\n");
>
> if (s->running)
> kdd_halt(s->guest);
> s->running = 0;
>
> {
> unsigned int i;
> /* XXX debug pattern */
> for (i = 0; i < 0x100 ; i++)
> -s->txb[sizeof (kdd_hdr) + i] = i;
> +s->txp.payload[sizeof (kdd_hdr) + i] = i;
Again, drop the sizeof(kdd_hdr) here I think.
Paul
> }
>
> /* Send a state-change message to the client so it knows we've stopped */
> s->txp.h.dir = KDD_DIR_PKT;
> s->txp.h.type = KDD_PKT_STC;
> s->txp.h.len = sizeof (kdd_stc);
> s->txp.h.id = (s->next_id ^= 1);
> s->txp.stc.subtype = KDD_STC_STOP;
> s->txp.stc.stop.cpu = s->cpuid;
> s->txp.stc.stop.ncpus = kdd_count_cpus(s->guest);
> s->txp.stc.stop.kthread = 0; /* Let the debugger figure it out */
> s->txp.stc.stop.status = KDD_STC_STATUS_BREAKPOINT;
RE: Build problems in kdd.c with xen-4.14.0-rc4
> -Original Message-
> From: Michael Young
> Sent: 03 July 2020 10:49
> To: p...@xen.org
> Cc: xen-devel@lists.xenproject.org; 'Tim Deegan'
> Subject: RE: Build problems in kdd.c with xen-4.14.0-rc4
>
> On Fri, 3 Jul 2020, Paul Durrant wrote:
>
> >> -Original Message-
> >> From: Xen-devel On Behalf Of
> >> Michael Young
> >> Sent: 30 June 2020 23:22
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Tim Deegan
> >> Subject: Build problems in kdd.c with xen-4.14.0-rc4
> >>
> >> I get the following errors when trying to build xen-4.14.0-rc4
> >>
> >> kdd.c: In function 'kdd_tx':
> >> kdd.c:754:15: error: array subscript 16 is above array bounds of
> >> 'uint8_t[16]' {aka 'unsigned
> >> char[16]'} [-Werror=array-bounds]
> >>754 | s->txb[len++] = 0xaa;
> >>| ~~^~~
> >> kdd.c:82:17: note: while referencing 'txb'
> >> 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling
> >> area for tx */
> >>| ^~~
> >> kdd.c: In function 'kdd_break':
> >> kdd.c:819:19: error: array subscript 16 is above array bounds of
> >> 'uint8_t[16]' {aka 'unsigned
> >> char[16]'} [-Werror=array-bounds]
> >>819 | s->txb[sizeof (kdd_hdr) + i] = i;
> >>| ~~^~
> >> kdd.c:82:17: note: while referencing 'txb'
> >> 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling
> >> area for tx */
> >>| ^~~
> >> In file included from /usr/include/stdio.h:867,
> >> from kdd.c:36:
> >> In function 'vsnprintf',
> >> inlined from 'kdd_send_string' at kdd.c:791:11:
> >> /usr/include/bits/stdio2.h:80:10: error: '__builtin___vsnprintf_chk'
> >> specified bound 65519 exceeds
> >> destination size 0 [-Werror=stringop-overflow=]
> >> 80 | return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL
> >> - 1,
> >>|
> >> ^
> >> 81 | __bos (__s), __fmt, __ap);
> >>| ~
> >> cc1: all warnings being treated as errors
> >> make[4]: ***
> >> [/builddir/build/BUILD/xen-4.14.0-rc4/tools/debugger/kdd/../../../tools/Rules.mk:216:
> >> kdd.o] Error 1
> >>
> >> The first two array-bounds errors seem to be a result of the
> >>
> >> kdd: stop using [0] arrays to access packet contents
> >>
> >> patch at
> >> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=3471cafbdda35eacf04670881dd2aee2558b4f08
> >>
> >> which reduced the size of txb from
> >> sizeof (kdd_hdr) + 65536
> >> to
> >> sizeof (kdd_hdr)
> >> which means the code now tries to write beyond the end of txb in both
> >> cases.
> >>
> >
> > Sorry not to get back to you sooner. Which compiler are you using?
> >
> > Paul
>
> This was with gcc-10.1.1-1.fc32.x86_64
> Full build logs are (at the moment) at
> https://download.copr.fedorainfracloud.org/results/myoung/xentest/fedora-32-x86_64/01515056-xen/
>
Ok, I have an older compiler. Does this patch fix it for you?
---8<---
diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 866532f0c7..a7d0976ea4 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -79,11 +79,11 @@ typedef struct {
/* State of the debugger stub */
typedef struct {
union {
-uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area for tx */
+uint8_t txb[sizeof (kdd_pkt)]; /* Marshalling area for tx */
kdd_pkt txp; /* Also readable as a packet structure */
};
union {
-uint8_t rxb[sizeof (kdd_hdr)]; /* Marshalling area for rx */
+uint8_t rxb[sizeof (kdd_pkt)]; /* Marshalling area for rx */
kdd_pkt rxp; /* Also readable as a packet structure */
};
unsigned int cur; /* Offset into rx where we'll put the next byte */
---8<---
Paul
> Michael Young
RE: Build problems in kdd.c with xen-4.14.0-rc4
On Fri, 3 Jul 2020, Paul Durrant wrote:
-Original Message-
From: Xen-devel On Behalf Of Michael
Young
Sent: 30 June 2020 23:22
To: xen-devel@lists.xenproject.org
Cc: Tim Deegan
Subject: Build problems in kdd.c with xen-4.14.0-rc4
I get the following errors when trying to build xen-4.14.0-rc4
kdd.c: In function 'kdd_tx':
kdd.c:754:15: error: array subscript 16 is above array bounds of 'uint8_t[16]'
{aka 'unsigned
char[16]'} [-Werror=array-bounds]
754 | s->txb[len++] = 0xaa;
| ~~^~~
kdd.c:82:17: note: while referencing 'txb'
82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
for tx */
| ^~~
kdd.c: In function 'kdd_break':
kdd.c:819:19: error: array subscript 16 is above array bounds of 'uint8_t[16]'
{aka 'unsigned
char[16]'} [-Werror=array-bounds]
819 | s->txb[sizeof (kdd_hdr) + i] = i;
| ~~^~
kdd.c:82:17: note: while referencing 'txb'
82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
for tx */
| ^~~
In file included from /usr/include/stdio.h:867,
from kdd.c:36:
In function 'vsnprintf',
inlined from 'kdd_send_string' at kdd.c:791:11:
/usr/include/bits/stdio2.h:80:10: error: '__builtin___vsnprintf_chk' specified
bound 65519 exceeds
destination size 0 [-Werror=stringop-overflow=]
80 | return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^
81 | __bos (__s), __fmt, __ap);
| ~
cc1: all warnings being treated as errors
make[4]: ***
[/builddir/build/BUILD/xen-4.14.0-rc4/tools/debugger/kdd/../../../tools/Rules.mk:216:
kdd.o] Error 1
The first two array-bounds errors seem to be a result of the
kdd: stop using [0] arrays to access packet contents
patch at
http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=3471cafbdda35eacf04670881dd2aee2558b4f08
which reduced the size of txb from
sizeof (kdd_hdr) + 65536
to
sizeof (kdd_hdr)
which means the code now tries to write beyond the end of txb in both
cases.
Sorry not to get back to you sooner. Which compiler are you using?
Paul
This was with gcc-10.1.1-1.fc32.x86_64
Full build logs are (at the moment) at
https://download.copr.fedorainfracloud.org/results/myoung/xentest/fedora-32-x86_64/01515056-xen/
Michael Young
RE: Build problems in kdd.c with xen-4.14.0-rc4
> -Original Message-
> From: Xen-devel On Behalf Of Michael
> Young
> Sent: 30 June 2020 23:22
> To: xen-devel@lists.xenproject.org
> Cc: Tim Deegan
> Subject: Build problems in kdd.c with xen-4.14.0-rc4
>
> I get the following errors when trying to build xen-4.14.0-rc4
>
> kdd.c: In function 'kdd_tx':
> kdd.c:754:15: error: array subscript 16 is above array bounds of
> 'uint8_t[16]' {aka 'unsigned
> char[16]'} [-Werror=array-bounds]
>754 | s->txb[len++] = 0xaa;
>| ~~^~~
> kdd.c:82:17: note: while referencing 'txb'
> 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
> for tx */
>| ^~~
> kdd.c: In function 'kdd_break':
> kdd.c:819:19: error: array subscript 16 is above array bounds of
> 'uint8_t[16]' {aka 'unsigned
> char[16]'} [-Werror=array-bounds]
>819 | s->txb[sizeof (kdd_hdr) + i] = i;
>| ~~^~
> kdd.c:82:17: note: while referencing 'txb'
> 82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
> for tx */
>| ^~~
> In file included from /usr/include/stdio.h:867,
> from kdd.c:36:
> In function 'vsnprintf',
> inlined from 'kdd_send_string' at kdd.c:791:11:
> /usr/include/bits/stdio2.h:80:10: error: '__builtin___vsnprintf_chk'
> specified bound 65519 exceeds
> destination size 0 [-Werror=stringop-overflow=]
> 80 | return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL -
> 1,
>|
> ^
> 81 | __bos (__s), __fmt, __ap);
>| ~
> cc1: all warnings being treated as errors
> make[4]: ***
> [/builddir/build/BUILD/xen-4.14.0-rc4/tools/debugger/kdd/../../../tools/Rules.mk:216:
> kdd.o] Error 1
>
> The first two array-bounds errors seem to be a result of the
>
> kdd: stop using [0] arrays to access packet contents
>
> patch at
> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=3471cafbdda35eacf04670881dd2aee2558b4f08
>
> which reduced the size of txb from
> sizeof (kdd_hdr) + 65536
> to
> sizeof (kdd_hdr)
> which means the code now tries to write beyond the end of txb in both
> cases.
>
Sorry not to get back to you sooner. Which compiler are you using?
Paul
> Michael Young
Re: Build problems in kdd.c with xen-4.14.0-rc4
On Tue, Jun 30, Michael Young wrote:
> I get the following errors when trying to build xen-4.14.0-rc4
This happens to work for me.
Olaf
---
tools/debugger/kdd/kdd.c | 8
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -742,25 +742,25 @@ static void kdd_tx(kdd_state *s)
int i;
/* Fix up the checksum before we send */
for (i = 0; i < s->txp.h.len; i++)
sum += s->txp.payload[i];
s->txp.h.sum = sum;
kdd_log_pkt(s, "TX", &s->txp);
len = s->txp.h.len + sizeof (kdd_hdr);
if (s->txp.h.dir == KDD_DIR_PKT)
/* Append the mysterious 0xaa byte to each packet */
-s->txb[len++] = 0xaa;
+s->txp.payload[len++] = 0xaa;
(void) blocking_write(s->fd, s->txb, len);
}
/* Send an acknowledgement to the client */
static void kdd_send_ack(kdd_state *s, uint32_t id, uint16_t type)
{
s->txp.h.dir = KDD_DIR_ACK;
s->txp.h.type = type;
s->txp.h.len = 0;
s->txp.h.id = id;
@@ -775,25 +775,25 @@ static void kdd_send_cmd(kdd_state *s, uint32_t subtype,
size_t extra)
s->txp.h.type = KDD_PKT_CMD;
s->txp.h.len = sizeof (kdd_cmd) + extra;
s->txp.h.id = (s->next_id ^= 1);
s->txp.h.sum = 0;
s->txp.cmd.subtype = subtype;
kdd_tx(s);
}
/* Cause the client to print a string */
static void kdd_send_string(kdd_state *s, char *fmt, ...)
{
uint32_t len = 0x - sizeof (kdd_msg);
-char *buf = (char *) s->txb + sizeof (kdd_hdr) + sizeof (kdd_msg);
+char *buf = (char *) &s->txp + sizeof (kdd_hdr) + sizeof (kdd_msg);
va_list ap;
va_start(ap, fmt);
len = vsnprintf(buf, len, fmt, ap);
va_end(ap);
s->txp.h.dir = KDD_DIR_PKT;
s->txp.h.type = KDD_PKT_MSG;
s->txp.h.len = sizeof (kdd_msg) + len;
s->txp.h.id = (s->next_id ^= 1);
s->txp.h.sum = 0;
s->txp.msg.subtype = KDD_MSG_PRINT;
@@ -807,25 +807,25 @@ static void kdd_break(kdd_state *s)
{
uint16_t ilen;
KDD_LOG(s, "Break\n");
if (s->running)
kdd_halt(s->guest);
s->running = 0;
{
unsigned int i;
/* XXX debug pattern */
for (i = 0; i < 0x100 ; i++)
-s->txb[sizeof (kdd_hdr) + i] = i;
+s->txp.payload[sizeof (kdd_hdr) + i] = i;
}
/* Send a state-change message to the client so it knows we've stopped */
s->txp.h.dir = KDD_DIR_PKT;
s->txp.h.type = KDD_PKT_STC;
s->txp.h.len = sizeof (kdd_stc);
s->txp.h.id = (s->next_id ^= 1);
s->txp.stc.subtype = KDD_STC_STOP;
s->txp.stc.stop.cpu = s->cpuid;
s->txp.stc.stop.ncpus = kdd_count_cpus(s->guest);
s->txp.stc.stop.kthread = 0; /* Let the debugger figure it out */
s->txp.stc.stop.status = KDD_STC_STATUS_BREAKPOINT;
signature.asc
Description: PGP signature
Build problems in kdd.c with xen-4.14.0-rc4
I get the following errors when trying to build xen-4.14.0-rc4
kdd.c: In function 'kdd_tx':
kdd.c:754:15: error: array subscript 16 is above array bounds of 'uint8_t[16]'
{aka 'unsigned char[16]'} [-Werror=array-bounds]
754 | s->txb[len++] = 0xaa;
| ~~^~~
kdd.c:82:17: note: while referencing 'txb'
82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
for tx */
| ^~~
kdd.c: In function 'kdd_break':
kdd.c:819:19: error: array subscript 16 is above array bounds of 'uint8_t[16]'
{aka 'unsigned char[16]'} [-Werror=array-bounds]
819 | s->txb[sizeof (kdd_hdr) + i] = i;
| ~~^~
kdd.c:82:17: note: while referencing 'txb'
82 | uint8_t txb[sizeof (kdd_hdr)]; /* Marshalling area
for tx */
| ^~~
In file included from /usr/include/stdio.h:867,
from kdd.c:36:
In function 'vsnprintf',
inlined from 'kdd_send_string' at kdd.c:791:11:
/usr/include/bits/stdio2.h:80:10: error: '__builtin___vsnprintf_chk' specified
bound 65519 exceeds destination size 0 [-Werror=stringop-overflow=]
80 | return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^
81 | __bos (__s), __fmt, __ap);
| ~
cc1: all warnings being treated as errors
make[4]: ***
[/builddir/build/BUILD/xen-4.14.0-rc4/tools/debugger/kdd/../../../tools/Rules.mk:216:
kdd.o] Error 1
The first two array-bounds errors seem to be a result of the
kdd: stop using [0] arrays to access packet contents
patch at
http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=3471cafbdda35eacf04670881dd2aee2558b4f08
which reduced the size of txb from
sizeof (kdd_hdr) + 65536
to
sizeof (kdd_hdr)
which means the code now tries to write beyond the end of txb in both
cases.
Michael Young
