GSoC final progress report (4)

Building Qubes Components

In a custom environment

With qubes-builder/pull/22 merged, we can now set custom environment variables while building the qubes components. This is especially useful if one wants to build a component with CC, CFLAGS, LDFLAGS of one’s own choice.

In order to set these environment variables, one only has to add an entry in the builder config file for setting the environment variables for the specific component(s) using the ENV_ value in the config.

ENV_gui_daemon = "\
    CC=clang \
    CFLAGS=-fstack-protector-strong\ -Wall"

Note: Spaces and newlines need to be escaped, and the -es should be replaced by _es in the component name

The ability to change the build environment along with the ability to change even the git repository and branch for the component now provides quite a bit of flexibility to tweak with the build process.

For example, using a build config like this:

We can set the git prefix, the baseurl, the branch and now even the environment in which we want to build the component. Earlier I used to chroot into the build dir and run certain build commands (different from the usual build) manually. Now though, I usually make the changes in a different branch on my forks of the components and the corresponding changes for the env vars in the config file and build the usual way. This might seem a bit of an overkill, but I feel it is necessary for continuous builds.

These environment variables are also shown in the build-info whenever a user is building qubes like so

Bug found building in a custom environment

While building the gui-daemon locally under Clang, having removed the hardcoded CC=gcc from the Makefile in my fork’s branch of the qubes-gui-daemon repo, and having set its value to clang through the build config, I found a bug in the code which had earlier escaped gcc.

clang -fstack-protector-strong -I../include/ -g -O2 -Wall -Wextra -Werror -fPIC `pkg-config --cflags libconfig` `pkg-config --cflags libnotify` `pkg-config --cflags libpng` `pkg-config --cflags vchan-xen`   -c -o xside.o xside.c
xside.c:2933:13: error: address of array 'g->vmname' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
    if (!g->vmname) {
1 error generated.
<builtin>: recipe for target 'xside.o' failed
make[1]: *** [xside.o] Error 1

It was a small fix which I made in qubes-gui-daemon/pull/14. So yeah, having the ability to set custom env vars has helped already!

Build with all protection flags

A direction which we wanted to take this custom env while building was to ensure all the components be built with all the hardening flags. This was to resolve the qubes-issues/issue/2259. In order to do this, like I have been doing on my forks, we’ll have to:

  • Remove all the hardcoded CC and CFLAGS from the Makefiles like these so that they do not override the values we set for them through the build config file.

  • Set all the protection flags through the config file, and also set default values when they are not set.

  • Make sure the same config is used for the release builds (for more deterministic builds), a different config is used for test builds/devel builds perhaps using clang (it seems to detect bugs better than gcc), and decide which config is used for the continuous builds through travis (possibly separate branches and builds for release versus test/devel builds).

Essentially, we could have a BUILD_TYPE or a RELEASE_BUILD variable in the build config which we could use to decide the above things. I still need to work on this. It shouldn’t take that long, but many tiny things need to be changed in a lot of places (all the makefiles, all the configs) to make it work well.

Under specific static analysis tools, continuously

A part of the static analysis I had discussed with my mentor was to build the qubes components directly under static-analysis tools like LLVM’s scan-build. I have been successfully able to achieve that integration on one of the components: qubes-gui-daemon, under a specific branch on my fork. And it should be easy enough doing the same for other components too, but right now I need to have a mechanism in place to decide when actually to build under scan-build.

Along with locally building under scan-build, the component also builds on travis-ci using scan-build, and so we are able to achieve the goal of a continuous static analysis that we had aimed for.

Bugs found using scan-build

Scan-build has already found 2 bugs in the gui-daemon code, which I will be opening PRs to fix soon.

One of them is type conversion with incompatible size:

png.c:90:12: warning: Result of 'malloc' is converted to a pointer of type 'uint32_t', which is incompatible with sizeof operand type 'long'

    data = malloc(data_size * sizeof(long));

           ^~~~~~             ~~~~~~~~~~~~

1 warning generated.

and the other one is an insignificant warning about never reading from a value stored to a file descriptor

xside.c:2533:9: warning: Value stored to 'fd' is never read

        fd = open(logname, O_WRONLY | O_CREAT | O_TRUNC, 0640); /* stderr */

        ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

1 warning generated.

So yeah, building components under static analysis tools has again paid off almost immediately. And a more complete and thorough integration with all the components will definitely help to find and fix bugs which might otherwise escape manual code review.

A thing I need to do is to centrally collect all these warnings and bugs that are detected using scan-build (and even using clang earlier), and show them clearly after the build (more clearly than to have to look for in the qubes-builder/build-logs/ directory). Ideas to implement this will be appreciated :)

When to build under what?

Right now I have all the mechanisms in place for static-analysis with different CC & CFLAGS, building under scan-build, and making it all continuous using travis-ci. The decision which we need to make now is when to build under what, and how to decide which build path we should follow. Basically,

  • For the release build, the components should have a single config file with all protection flags set.

  • For test/devel builds, we can either build under clang with specific CFLAGS, or we can build under scan-build. I think for the regular test/devel builds we can use scan-build as it would require the least amount of tweaking, but for test/devel builds for which we want more flexibility (like building with certain sanitizers for fuzzing: ASan, UBSan etc.) should build directly under clang with the respective CFLAGS.

Suggestions on how to implement this are welcome :)