On Thu, 2023-03-09 at 19:56 +0100, Hans-Peter Nilsson wrote:
> It's not obvious to me whether considered best to include or
> exclude these tests that depend on structure layout details.
> If excluding, the obvious alternative to this patch is then
> to add a top one-liner (to dg-skip-if the test for
> default_packed targets or a similar excluding expression).
> I'm fine either way, just suggesting the following, which
> handles the cris-elf test-case failures I see for these
> tests, and causes no change in results for native
> x86_64-pc-linux-gnu.
Thanks for looking at this.
How about a third option: can the structs be explicitly marked as being
packed, by adding __attribute__((__packed__)) to the various structs?
The tests are all about detecting problems with padding bits, and
presumably we can have padding bits on all targets if we explicitly ask
for them.
Does that make for a simpler patch?
Dave
>
> Beware that some of the tests have lines with trailing
> whitespace. Where lines are changed in this patch, the
> trailing whitespace is removed.
>
> Ok to commit?
>
> -- >8 --
> It's a judgement call whether to just skip some of these
> tests rather than trying to match messages depending on the
> layout of structures, but better include than exclude.
>
> * gcc.dg/plugin/infoleak-2.c,
> gcc.dg/plugin/infoleak-CVE-2011-1078-1.c,
> gcc.dg/plugin/infoleak-CVE-2011-1078-2.c,
> gcc.dg/plugin/infoleak-CVE-2017-18549-1.c,
> gcc.dg/plugin/infoleak-CVE-2017-18550-1.c,
> gcc.dg/plugin/infoleak-antipatterns-1.c,
> gcc.dg/plugin/infoleak-fixit-1.c: Handle default_packed
> targets.
> ---
> gcc/testsuite/gcc.dg/plugin/infoleak-2.c | 13 ++++++++---
> --
> .../gcc.dg/plugin/infoleak-CVE-2011-1078-1.c | 10 +++++-----
> .../gcc.dg/plugin/infoleak-CVE-2011-1078-2.c | 10 +++++-----
> .../gcc.dg/plugin/infoleak-CVE-2017-18549-1.c | 10 +++++-----
> .../gcc.dg/plugin/infoleak-CVE-2017-18550-1.c | 7 ++++---
> .../gcc.dg/plugin/infoleak-antipatterns-1.c | 10 +++++-----
> gcc/testsuite/gcc.dg/plugin/infoleak-fixit-1.c | 10 ++++++----
> 7 files changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-2.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-2.c
> index 252f8f25918a..4ba484b3c6be 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-2.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-2.c
> @@ -18,16 +18,19 @@ struct st
> int b:1; /* { dg-message "field 'b' is uninitialized \\(1 bit\\)"
> "field" } */
> /* { dg-message "padding after field 'b' is uninitialized
> \\(7 bits\\)" "padding" { target *-*-* } .-1 } */
> u8 d; /* { dg-message "field 'd' is uninitialized \\(1 byte\\)"
> } */
> - int c:7; /* { dg-message "padding after field 'c' is uninitialized
> \\(9 bits\\)" } */
> - u16 e; /* { dg-message "padding after field 'e' is uninitialized
> \\(2 bytes\\)" } */
> + int c:7; /* { dg-message "padding after field 'c' is uninitialized
> \\(9 bits\\)" "padding" { target { ! default_packed } } } */
> + /* { dg-message "padding after field 'c' is uninitialized
> \\(1 bit\\)" "padding" { target default_packed } .-1 } */
> + u16 e; /* { dg-message "padding after field 'e' is uninitialized
> \\(2 bytes\\)" "padding" { target { ! default_packed } } } */
> };
>
> void test (void __user *dst, u16 v)
> {
> struct st s; /* { dg-message "region created on stack here"
> "where" } */
> - /* { dg-message "capacity: 12 bytes" "capacity" { target *-*-* }
> .-1 } */
> - /* { dg-message "suggest forcing zero-initialization by providing
> a '\\{0\\}' initializer" "fix-it" { target *-*-* } .-2 } */
> + /* { dg-message "capacity: 12 bytes" "capacity" { target { !
> default_packed } } .-1 } */
> + /* { dg-message "capacity: 9 bytes" "capacity" { target
> default_packed } .-2 } */
> + /* { dg-message "suggest forcing zero-initialization by providing
> a '\\{0\\}' initializer" "fix-it" { target *-*-* } .-3 } */
> s.e = v;
> copy_to_user(dst, &s, sizeof (struct st)); /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" "warning" } */
> - /* { dg-message "10 bytes are uninitialized" "note how much" {
> target *-*-* } .-1 } */
> + /* { dg-message "10 bytes are uninitialized" "note how much" {
> target { ! default_packed } } .-1 } */
> + /* { dg-message "7 bytes are uninitialized" "note how much" {
> target default_packed } .-2 } */
> }
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-1.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-1.c
> index 3616fbe176b3..9269b911b22f 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-1.c
> @@ -51,7 +51,7 @@ struct socket {
>
> struct sco_conninfo {
> __u16 hci_handle;
> - __u8 dev_class[3]; /* { dg-message "padding after field
> 'dev_class' is uninitialized \\(1 byte\\)" } */
> + __u8 dev_class[3]; /* { dg-message "padding after field
> 'dev_class' is uninitialized \\(1 byte\\)" "padding" { target { !
> default_packed } } } */
> };
>
> struct sco_conn {
> @@ -83,8 +83,8 @@ static int sco_sock_getsockopt_old_broken(struct
> socket *sock, int optname, char
> {
> struct sock *sk = sock->sk;
> /* [...snip...] */
> - struct sco_conninfo cinfo; /* { dg-message "region created on
> stack here" "where" } */
> - /* { dg-message "capacity: 6
> bytes" "capacity" { target *-*-* } .-1 } */
> + struct sco_conninfo cinfo; /* { dg-message "region created on
> stack here" "where" { target { ! default_packed } } } */
> + /* { dg-message "capacity: 6
> bytes" "capacity" { target { ! default_packed } } .-1 } */
> /* Note: 40 bits of fields, padded to 48. */
>
> int len, err = 0;
> @@ -101,8 +101,8 @@ static int sco_sock_getsockopt_old_broken(struct
> socket *sock, int optname, char
> memcpy(cinfo.dev_class, sco_pi(sk)->conn->hcon-
> >dev_class, 3);
>
> len = min_t(unsigned int, len, sizeof(cinfo));
> - if (copy_to_user(optval, (char *)&cinfo, len)) /* {
> dg-warning "potential exposure of sensitive information by copying
> uninitialized data from stack" "warning" { target *-*-* } } */
> - /* { dg-message "1 byte is uninitialized"
> "how much note" { target *-*-* } .-1 } */
> + if (copy_to_user(optval, (char *)&cinfo, len)) /* {
> dg-warning "potential exposure of sensitive information by copying
> uninitialized data from stack" "warning" { target { ! default_packed
> } } } */
> + /* { dg-message "1 byte is uninitialized"
> "how much note" { target { ! default_packed } } .-1 } */
> err = -1;
>
> /* [...snip...] */
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-2.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-2.c
> index 2096bda71798..d5f598b0878e 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-2.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2011-1078-2.c
> @@ -15,22 +15,22 @@ typedef unsigned short __u16;
>
> struct sco_conninfo {
> __u16 hci_handle;
> - __u8 dev_class[3]; /* { dg-message "padding after field
> 'dev_class' is uninitialized \\(1 byte\\)" } */
> + __u8 dev_class[3]; /* { dg-message "padding after field
> 'dev_class' is uninitialized \\(1 byte\\)" "padding" { target { !
> default_packed } } } */
> };
>
> /* Adapted from sco_sock_getsockopt_old in net/bluetooth/sco.c. */
>
> int test_1 (char __user *optval, const struct sco_conninfo *in)
> {
> - struct sco_conninfo cinfo; /* { dg-message "region created on
> stack here" "where" } */
> - /* { dg-message "capacity: 6
> bytes" "capacity" { target *-*-* } .-1 } */
> + struct sco_conninfo cinfo; /* { dg-message "region created on
> stack here" "where" { target { ! default_packed } } } */
> + /* { dg-message "capacity: 6
> bytes" "capacity" { target { ! default_packed } } .-1 } */
> /* Note: 40 bits of fields, padded to 48. */
>
> cinfo.hci_handle = in->hci_handle;
> memcpy(cinfo.dev_class, in->dev_class, 3);
>
> - copy_to_user(optval, &cinfo, sizeof(cinfo)); /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" "warning" } */
> - /* { dg-message "1 byte is uninitialized" "how much note" {
> target *-*-* } .-1 } */
> + copy_to_user(optval, &cinfo, sizeof(cinfo)); /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" "warning" { target { ! default_packed } } } */
> + /* { dg-message "1 byte is uninitialized" "how much note" {
> target { ! default_packed } } .-1 } */
> }
>
> int test_2 (char __user *optval, const struct sco_conninfo *in)
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18549-1.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18549-1.c
> index 8a1c816cc1b5..8fcf9a904a2a 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18549-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18549-1.c
> @@ -35,7 +35,7 @@ struct aac_srb_reply
> __le32 scsi_status;
> __le32 data_xfer_length;
> __le32 sense_data_size;
> - u8 sense_data[AAC_SENSE_BUFFERSIZE]; /* { dg-
> message "padding after field 'sense_data' is uninitialized \\(2
> bytes\\)" } */
> + u8 sense_data[AAC_SENSE_BUFFERSIZE]; /* { dg-
> message "padding after field 'sense_data' is uninitialized \\(2
> bytes\\)" "padding" { target { ! default_packed } } } */
> };
>
> #define ST_OK 0
> @@ -50,8 +50,8 @@ static int aac_send_raw_srb(/* [...snip...] */
>
> /* [...snip...] */
>
> - struct aac_srb_reply reply; /* { dg-message "region created
> on stack here" "memspace message" } */
> - /* { dg-message "capacity: 52 bytes" "capacity message" {
> target *-*-* } .-1 } */
> + struct aac_srb_reply reply; /* { dg-message "region created
> on stack here" "memspace message" { target { ! default_packed } } }
> */
> + /* { dg-message "capacity: 52 bytes" "capacity message" {
> target { ! default_packed } } .-1 } */
>
> reply.status = ST_OK;
>
> @@ -65,8 +65,8 @@ static int aac_send_raw_srb(/* [...snip...] */
>
> /* [...snip...] */
>
> - if (copy_to_user(user_reply, &reply, /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" } */
> - /* { dg-message "2 bytes
> are uninitialized" "note how much" { target *-*-* } .-1 } */
> + if (copy_to_user(user_reply, &reply, /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" "padding" { target { ! default_packed } } } */
> + /* { dg-message "2 bytes
> are uninitialized" "note how much" { target { ! default_packed } } .-
> 1 } */
> sizeof(struct aac_srb_reply))) {
> /* [...snip...] */
> }
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18550-1.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18550-1.c
> index 4272da96bab0..a7361f937401 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18550-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-CVE-2017-18550-1.c
> @@ -31,7 +31,7 @@ struct aac_hba_info {
> u8 driver_name[50]; /* { dg-message "field 'driver_name'
> is uninitialized \\(50 bytes\\)" } */
> u8 adapter_number;
> u8 system_io_bus_number;
> - u8 device_number; /* { dg-message "padding after field
> 'device_number' is uninitialized \\(3 bytes\\)" } */
> + u8 device_number; /* { dg-message "padding after field
> 'device_number' is uninitialized \\(3 bytes\\)" "padding" { target {
> ! default_packed } } } */
> u32 function_number;
> u32 vendor_id;
> u32 device_id;
> @@ -108,7 +108,8 @@ struct pci_bus {
> static int aac_get_hba_info(struct aac_dev *dev, void __user *arg)
> {
> struct aac_hba_info hbainfo; /* { dg-message "region created
> on stack here" "memspace message" } */
> - /* { dg-message "capacity: 200 bytes" "capacity message" {
> target *-*-* } .-1 } */
> + /* { dg-message "capacity: 200 bytes" "capacity message" {
> target { ! default_packed } } .-1 } */
> + /* { dg-message "capacity: 194 bytes" "capacity message" {
> target default_packed } .-2 } */
>
> hbainfo.adapter_number = (u8) dev->id;
> hbainfo.system_io_bus_number = dev->pdev->bus->number;
> @@ -121,7 +122,7 @@ static int aac_get_hba_info(struct aac_dev *dev,
> void __user *arg)
> hbainfo.sub_system_id = dev->pdev-
> >subsystem_device;
>
> if (copy_to_user(arg, &hbainfo, sizeof(struct aac_hba_info)))
> { /* { dg-warning "potential exposure of sensitive information by
> copying uninitialized data from stack" "warning" } */
> - /* { dg-message "177 bytes are uninitialized" "how
> much" { target *-*-* } .-1 } */
> + /* { dg-message "177 bytes are uninitialized" "how
> much" { target { ! default_packed } } .-1 } */
> /* [...snip...] */
> }
>
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-antipatterns-1.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-antipatterns-1.c
> index 500845364388..d9db2b3bdebc 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-antipatterns-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-antipatterns-1.c
> @@ -90,21 +90,21 @@ int infoleak_heap_missing_a_field(void __user
> *dst, u32 v)
>
> struct infoleak_3
> {
> - u8 a; /* { dg-message "padding after field 'a' is uninitialized
> \\(3 bytes\\)" } */
> + u8 a; /* { dg-message "padding after field 'a' is uninitialized
> \\(3 bytes\\)" "padding" { target { ! default_packed } } } */
> /* padding here */
> u32 b;
> };
>
> int infoleak_stack_padding(void __user *dst, u8 p, u32 q)
> {
> - struct infoleak_3 st; /* { dg-message "region created on stack
> here" "where" } */
> - /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-
> 1 } */
> + struct infoleak_3 st; /* { dg-message "region created on stack
> here" "where" { target { ! default_packed } } } */
> + /* { dg-message "capacity: 8 bytes" "capacity" { target { !
> default_packed } } .-1 } */
>
> st.a = p;
> st.b = q;
> /* No initialization of padding. */
> - if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential
> exposure of sensitive information by copying uninitialized data from
> stack" "warning" } */
> - /* { dg-message "3 bytes are uninitialized" "note how much" {
> target *-*-* } .-1 } */
> + if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential
> exposure of sensitive information by copying uninitialized data from
> stack" "warning" { target { ! default_packed } } } */
> + /* { dg-message "3 bytes are uninitialized" "note how much" {
> target { ! default_packed } } .-1 } */
> return -EFAULT;
> return 0;
> }
> diff --git a/gcc/testsuite/gcc.dg/plugin/infoleak-fixit-1.c
> b/gcc/testsuite/gcc.dg/plugin/infoleak-fixit-1.c
> index 6961b44f76b9..192c9e7802ad 100644
> --- a/gcc/testsuite/gcc.dg/plugin/infoleak-fixit-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/infoleak-fixit-1.c
> @@ -11,16 +11,18 @@ typedef unsigned int u32;
>
> struct st
> {
> - u8 i; /* { dg-message "padding after field 'i' is uninitialized
> \\(3 bytes\\)" } */
> + u8 i; /* { dg-message "padding after field 'i' is uninitialized
> \\(3 bytes\\)" "padding" { target { ! default_packed } } } */
> u32 j; /* { dg-message "field 'j' is uninitialized \\(4 bytes\\)"
> } */
> };
>
> void test (void __user *dst, u8 a)
> {
> struct st s; /* { dg-message "region created on stack here"
> "where" } */
> - /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-
> 1 } */
> - /* { dg-message "suggest forcing zero-initialization by providing
> a '.0.' initializer" "fix-it hint" { target *-*-* } .-2 } */
> + /* { dg-message "capacity: 8 bytes" "capacity" { target { !
> default_packed } } .-1 } */
> + /* { dg-message "capacity: 5 bytes" "capacity" { target
> default_packed } .-2 } */
> + /* { dg-message "suggest forcing zero-initialization by providing
> a '.0.' initializer" "fix-it hint" { target *-*-* } .-3 } */
> s.i = a;
> copy_to_user(dst, &s, sizeof (struct st)); /* { dg-warning
> "potential exposure of sensitive information by copying uninitialized
> data from stack" "warning" } */
> - /* { dg-message "7 bytes are uninitialized" "note how much" {
> target *-*-* } .-1 } */
> + /* { dg-message "7 bytes are uninitialized" "note how much" {
> target { ! default_packed } } .-1 } */
> + /* { dg-message "4 bytes are uninitialized" "note how much" {
> target default_packed } .-2 } */
> }
