[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] locking down struct size/layout in remote-protocol.x



This week we noticed that a change to struct remote_error
was causing trouble (new member addition changed the size of
the object being decoded -- bad for the protocol).

In order to ensure that no such changes sneak in again,
I'm planning to do the following.

First, count how many structs must not change size or layout:

    $ grep -E '^struct remote_' src/*/remote_protocol.x|wc -l
    318

Obviously not appropriate to do it manually.
Running this (pdwtags is part of the dwarves package):

    pdwtags src/libvirt_driver_remote_la-remote_protocol.o

prints, among other things, detailed type info for every struct:

    /* 89 */
    struct remote_nonnull_domain {
            remote_nonnull_string      name;                 /*     0     8 */
            remote_uuid                uuid;                 /*     8    16 */
            int                        id;                   /*    24     4 */

            /* size: 32, cachelines: 1, members: 3 */
            /* last cacheline: 32 bytes */

            /* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */

    }; /* size: 32 */

    /* 90 */
    typedef struct remote_nonnull_domain remote_nonnull_domain; /* size: 32 */

    /* 91 */
    struct remote_nonnull_network {
            remote_nonnull_string      name;                 /*     0     8 */
            remote_uuid                uuid;                 /*     8    16 */

            /* size: 24, cachelines: 1, members: 2 */
            /* last cacheline: 24 bytes */
    }; /* size: 24 */

-----------------------------------------------------
A) For each of the above matching /^struct remote_/, print at the
very least this sort of compile-time check:

    verify (sizeof (struct remote_nonnull_domain) == 32);

    verify (sizeof (struct remote_nonnull_network) == 24);

B) Even better, verify member offsets, so that we'd detect
reordered members (which often will not change struct size):

    verify (sizeof (struct remote_nonnull_domain) == 32);
      verify (offsetof (struct remote_nonnull_domain, name) == 0);
      verify (offsetof (struct remote_nonnull_domain, uuid) == 8);
      verify (offsetof (struct remote_nonnull_domain, id) == 24);

    verify (sizeof (struct remote_nonnull_network) == 24);
      verify (offsetof (struct remote_nonnull_network, name) == 0);
      verify (offsetof (struct remote_nonnull_network, uuid) == 16);

C) Might as well verify member sizes as well.

-----------------------------------------------------
Once we have a small script to generate that (RSN), include the generated,
(and version-controlled) file from remote_protocol.c, and we're done.

For reference, here's code to generate A):

    pdwtags src/libvirt_driver_remote_la-remote_protocol.o \
      | perl -0777 -n \
          -e 'foreach my $p (split m!\n\n/\* \d+ \*/\n!)' \
          -e '  { $p =~ m!^struct (remote_\w+).*/\* size: (\d+) \*/!s' \
          -e '      and print "verify (sizeof (struct $1) == $2);\n" }'

And here's a sample of its output:

    verify (sizeof (struct remote_nonnull_domain) == 32);
    verify (sizeof (struct remote_nonnull_network) == 24);
    verify (sizeof (struct remote_nonnull_nwfilter) == 24);
    verify (sizeof (struct remote_nonnull_interface) == 16);
    verify (sizeof (struct remote_nonnull_storage_pool) == 24);
    verify (sizeof (struct remote_nonnull_storage_vol) == 24);
    verify (sizeof (struct remote_nonnull_node_device) == 8);
    ...
    verify (sizeof (struct remote_domain_has_current_snapshot_args) == 40);
    verify (sizeof (struct remote_domain_has_current_snapshot_ret) == 4);
    verify (sizeof (struct remote_domain_snapshot_current_args) == 40);
    verify (sizeof (struct remote_domain_snapshot_current_ret) == 40);
    verify (sizeof (struct remote_domain_revert_to_snapshot_args) == 48);
    verify (sizeof (struct remote_domain_snapshot_delete_args) == 48);
    verify (sizeof (struct remote_message_header) == 24);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]