Project

General

Profile

Actions

Bug #11866

closed

Use -fstack-protector-strong when available

Added by John Levon almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

In illumos#5922, parts of the system were built with -fstack-protector. At that time, it was noted that the actual thing we wanted was -fstack-protector-strong - let's do that.

Actions #1

Updated by John Levon almost 4 years ago

Original notes from https://smartos.org/bugview/OS-7620 by Robert Mustacchi:

To test this, I put together a set of four different tests between on
haswell, a Intel(r) Xeon(r) CPU E3-1270 v3 @ 3.50GHz. I went into the
BIOS and made sure that TurboBoost was disabled so as to have a
consistent set of data.
Experimental Setup

I put together a set of four tests that were designed to fill in
different bits of information:

1. Calling stat in a tight loop on a file
2. Calling gethrvtime in a tight loop
3. Pinging another node over a back to back 25 GbE interface using the
qede driver
4. Testing some soft-real time processes by using a USB audio adapter to
play a converted mp3 to au file using the song 光 by Utada Hikaru.

To set these up, I first created a processor set, disabled interrupts on
that processor, and then bound my shell to it using the following
commands. Each test was performed a series of 10 times while bound to
the shell, with the exceptio of the audio test which was just done using
a separate session without being bound or with interrupts disabled. Set
up and running was as follows:

# psrset -c 1
# psradm -i 1
# psrset -b 1 $$
for i in $(seq 1 10); do ./gethrvtime 50000000 > vtime.out.$i; done
for i in $(seq 1 10); do ../stat ../file 5000000 > stat.out.$i; done
for i in $(seq 1 10); do ping -I 0.02 -sn 10.0.0.2 56 1000 > ping.out.$i; done

Note, for each test I ran it manually a few times to make sure that it
was good to go. The ping test was performed with haswell running on the
IP address 10.0.0.1/24 using qede1 with a QSFP28 cable to another system
using the same platform with the IP address 10.0.0.2/24.

The source code for the two programs is as follows with their Makefile.
These were built as 32-bit programs using gcc 4.7.3 and were built at
optimization -O0. For more details, see the Makefile:

# cat gethrvtime.c
/*
 * Perform a lot of hrtime calls in a row and see what happens.
 */

#include <err.h>
#include <stdlib.h>
#include <unistd.h>
#include <limits.h>
#include <sys/time.h>
#include <stdio.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>

int
main(int argc, const char *argv[])
{
    long long count, i;
    const char *errstr;
    hrtime_t start, end;

    if (argc != 2) {
        errx(EXIT_FAILURE, "missing args");
    }

    count = strtonum(argv[1], 0, LONG_MAX, &errstr);
    if (errstr != NULL) {
        errx(EXIT_FAILURE, "bad number of iterations: %s", errstr);
    }

    if ((count % 10000) != 0) {
        errx(EXIT_FAILURE, "count should be a multiple of 1000");
    }

    start = gethrtime();
    for (i = 0; i < count ; i++) {
        if (i != 0 && (i % 10000) == 0) {
            end = gethrtime();
            printf("%lld\n", end - start);
            start = gethrtime();
        }

        (void) gethrvtime();
    }

    end = gethrtime();
    printf("%lld\n", end - start);

    return (0);
}
# cat Makefile
#
#
#

CC = gcc
CFLAGS = -Wall -Wextra -O0
PROGS = stat gethrvtime

all: $(PROGS)

%: %.c
    $(CC) -o $@ $(CFLAGS) $<
# stat.c
/*
 * Perform a lot of stat calls in a row and see what happens.
 */

#include <err.h>
#include <stdlib.h>
#include <unistd.h>
#include <limits.h>
#include <sys/time.h>
#include <stdio.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>

int
main(int argc, const char *argv[])
{
    long long count, i;
    const char *errstr;
    hrtime_t start, end;

    if (argc != 3) {
        errx(EXIT_FAILURE, "missing args");
    }

    count = strtonum(argv[2], 0, LONG_MAX, &errstr);
    if (errstr != NULL) {
        errx(EXIT_FAILURE, "bad number of iterations: %s", errstr);
    }

    if ((count % 1000) != 0) {
        errx(EXIT_FAILURE, "count should be a multiple of 1000");
    }

    start = gethrtime();
    for (i = 0; i < count ; i++) {
        struct stat st;

        if (i != 0 && (i % 1000) == 0) {
            end = gethrtime();
            printf("%lld\n", end - start);
            start = gethrtime();
        }

        if (stat(argv[1], &st) != 0) {
            err(EXIT_FAILURE, "failed to stat %s", argv[1]);
        }
    }

    end = gethrtime();
    printf("%lld\n", end - start);

    return (0);
}

Experimental Results

I've tarred up all of the results and attached them to this ticket.
Doing a basic analysis found that most of the data was in the noise of
one another. While there were slight differences in these paths, it was
hard to attribute them. I also found no discernible difference in the
USB audio test.

To try and discern the differences, I first quantized all the data. It
all was bucketed to the same rough bucket, which meant that there
weren't any gross outliers. This was still true even when dividing the
time. I had trouble finding good software to feed into an arbitrary
equivalent of DTrace's lquantize and unfortunately my attempt to
brute force DTrace to do so didn't end well. Through some manual
inspection I mostly determined that it felt like it was all pretty
similar. While measurements such as the average aren't great or useful,
when the average of all the data is pretty tight and the before and
after are similar, it's hard to feel like this is creating a notable
difference.

Using the qede driver had an interesting side effect of determining
an issue in the driver. Though we've worked around it, which is partly
why some of the networking stack numbers weren't rerun a second time.

Actions #2

Updated by John Levon almost 4 years ago

I also did some basic sanity testing of this under illumos-gate

Actions #3

Updated by Electric Monk almost 4 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit b12258b69ac245658b0ca5ae070b3ff004186148

commit  b12258b69ac245658b0ca5ae070b3ff004186148
Author: Robert Mustacchi <rm@joyent.com>
Date:   2019-10-29T16:22:06.000Z

    11866 Use -fstack-protector-strong when available
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Reviewed by: John Levon <john.levon@joyent.com>
    Reviewed by: Andy Fiddaman <andy@omniosce.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF