Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenXT build fixes #383

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

jandryuk
Copy link
Contributor

This is an assortment of build fixes from OpenXT, which is built with OpenEmbedded. Some are OpenXT specific and others are generic.

OPEN_XT needs a few fixups to build properly.

The __RD* macros throw a redefinition error with Xen 4.18.

Some fixes to build with -Werror=stringop-truncation.

Individual commits are documented.

jandryuk and others added 5 commits October 19, 2023 12:48
vhd_open_crypto has a redeclaration error:

block-crypto.c: In function ‘vhd_open_crypto’:
block-crypto.c:351:17: error: ‘key’ redeclared as different kind of symbol
  351 |         uint8_t key[MAX_AES_XTS_PLAIN_KEYSIZE / sizeof(uint8_t)] = { 0 };
      |                 ^~~
block-crypto.c:346:52: note: previous definition of ‘key’ with type ‘const uint8_t *’ {aka ‘const unsigned char *’}
  346 | vhd_open_crypto(vhd_context_t *vhd, const uint8_t *key, size_t key_bytes, const char *name)

OPEN_XT populates its key with chain_find_keyed_vhd, while the
non-OPEN_XT version has the key passed in.  Rename the local buffer and
assign key to it to keep things working.

chain_find_keyed_vhd can't accept the const key, so pass keybuf to it.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
GCC complains:

block-crypto.c: In function ‘find_keyfile’:
block-crypto.c:153:32: error: ‘,aes-xts-plain,’ directive output may be truncated writing 15 bytes into a region of size between 0 and 255 [-Werror=format-truncation=]
  153 |                          "%s/%s,aes-xts-plain,%d.key",
      |                                ^~~~~~~~~~~~~~~
block-crypto.c:152:17: note: ‘snprintf’ output 22 or more bytes (assuming 277) into a destination of size 256
  152 |                 snprintf(path, sizeof(path),
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  153 |                          "%s/%s,aes-xts-plain,%d.key",
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  154 |                          keydir, basename, keysize);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~

Increase the size of path.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
findkey() sets keysize to 512 or 256, but an xts case deals with bytes,
better name the variables to indicate whether they contain keysize in
bytes or bits, and make sure we convert chain_find_keyed_vhd to bytes
before calling xts_aes_setkey()

Signed-off-by: Chris Rogers <rogersc@ainfosec.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
OpenXT doesn't pass in the encryption key, but loads it later when
opening the VHD itself.  Therefore it needs to always load the crypto
library to have it available.  Re-arrange to make that the case.

While doing this, swap the if branches, to let the condition be a
positive check.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
We need to include the util.h for the safe_strncpy declararion.

Fixes: 6ffa1d8 "CA-366761: replace use of strncpy with inlined wrapper"
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Copy link
Contributor

@MarkSymsCtx MarkSymsCtx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit but other than it looks fine, apologies for the delay in reviewing it.

drivers/td-blkif.c Outdated Show resolved Hide resolved
tapdisk_xenblkif_evtchn_event_id()
tapdisk_xenblkif_chkrng_event_id()
tapdisk_xenblkif_stoppolling_event_id()

Can all be static inline functions within td-blkif.c

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Move into the header as a proper static inline.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
The __RD* macros are already defined in Xen's io/ring.h which is
included through blkif.h and they have been there for years.  In Xen
4.18 the macros changed for MISRA and now the definitions are out of
sync leading to redefinition errors.  Just remove the local versions and
rely on the io/ring.h ones.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
In the lib/vhd code:
In function 'safe_strncpy',
    inlined from 'vhd_initialize_footer' at libvhd.c:2882:2:
include/util.h:46:17: error: 'strncpy' output truncated before terminating nul copying 3 bytes from a string of the same length [-Werror=stringop-truncation]
   46 |         pdest = strncpy(dest, src, n - 1);

In the lvm code twice:
In function 'safe_strncpy',
    inlined from 'lvm_copy_name' at lvm-util.c:71:2,
    inlined from 'lvm_scan_lvs' at lvm-util.c:289:9:
include/util.h:46:17: error: 'strncpy' output may be truncated copying 255 bytes from a string of length 255 [-Werror=stringop-truncation]
   46 |         pdest = strncpy(dest, src, n - 1);

For VHD, just set the footer bytes individually and avoid the string
ops.

For LVM, switching back to strncpy and providing the full destination
size silences the errors.  strncpy does NUL terminate the string - it
just may truncate.  (safe_strncpy silences many other instances of
stringop-truncation like:
"error: 'strncpy' specified bound 1024 equals destination size", so we
want to keep it.)

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
OpenXT, using OpenEmbedded Dundell, fails to link cbt-util with errors
like:
ld: ./.libs/libcbtutil.a(cbt-util.o): in function `cbt_util_set':
ld: cbt/cbt-util.c:360: undefined reference to `uuid_parse'

libcbtutil.a/cbt-util.c reference the uuid funtions, and main.c doesn't
have any references.  Move the linking to the library to resolve the
issue.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
MarkSymsCtx
MarkSymsCtx previously approved these changes Nov 27, 2023
@MarkSymsCtx
Copy link
Contributor

I'll run this through some tests in our automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants