Re: [PATCH] printk: Userspace format enumeration support

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

 



Joe Perches writes:
There are certainly printks which can't be trivially monitored using the printk
format alone, but the vast majority of the ones that are monitored _do_ have
meaningful formats and can be monitored over time. No solution to this is going
to catch every single case, especially when so much of the information can be
generated dyamically, but this patchset still goes a long way to making printk
monitoring more tractable for use cases like the one described in the
changelog.

For the _vast_ majority of printk strings, this can easily be found
and compared using a trivial modification to strings.

There are several issues with your proposed approach that make it unsuitable for use as part of a reliable production environment:

1. It misses printk() formats without KERN_SOH

printk() formats without KERN_SOH are legal and use MESSAGE_LOGLEVEL_DEFAULT. On my test kernel, your proposed patch loses >5% of printk formats -- over 200 messages -- due to this, including critical ones like those about hardware or other errors.

2. Users don't always have the kernel image available

Many of our machines and many of the machines of others like us do not boot using local storage, but instead use PXE or other technologies where the kernel may not be stored during runtime.

As is described in the changelog, it is necessary to be able to vary remediations not only based on what is already in /dev/kmsg, but also to be able to make decisions about our methodology based on what's _supported_ in the running kernel at runtime, and your proposed approach makes this not viable.

3. `KERN_SOH + level' can appear in other places than just printk strings

KERN_SOH is just ASCII '\001' -- it's not distinctive or unique, even when paired with a check for something that looks like a level after it. For this reason, your proposed patch results in a non-trivial amount of non-printk related garbage in its output. For example:

    % binutils/strings -k /tmp/vmlinux | head -5
    3L)s
    3L)s
    c,[]A\
    c(L)c
    d$pL)d$`u4

Fundamentally, one cannot use a tool which just determines whether something is printable to determine semantic intent.

4. strings(1) output cannot differentiate embedded newlines and new formats

The following has exactly the same output from strings(1), but will manifest completely differently at printk() time:

    printk(KERN_ERR "line one\nline two\nline three\n");
    printk("line four\n");

With strings, the hypothetical output would be:*

    3line one\nline two\nline three\nline four\n

* "line four\n" would also be missing with your current -k check.

But this makes it impossible to distinguish between this, compared to:

    printk(KERN_ERR "line one\nline two\n");
    printk("line three\n");
    printk("line four\n");

The originally posted patch _does_ differentiate between these cases, using \0 as a reliable separator. Its outputs are, respectively:

    \0013line one\nline two\nline three\0\nline four\n\0
    \0013line one\nline two\n\0line three\nline four\n\0

This isn't just a theoretical concern -- there are plenty of places which use multiline printks, and we must be able to distinguish between that and _multiple_ printks. Not being able to differentiate cases like these would dramatically reduce the effectiveness of printk enumeration, as we can no longer ascertain which formats will always be used together (for example, in the case of sequences of printks guarded by conditionals, which are all over the place).

5. strings(1) is not contextually aware, and cannot be made to act as if it is

strings has no idea about what it is reading, which is why it is more than happy to output the kind of meaningless output shown in #3. There are plenty of places across the kernel where there might be a sequence of bytes which the strings utility happens to interpret as being semantically meaningful, but in reality just happens to be an unrelated sequence of coincidentally printable bytes that just happens to contain a \001.

I appreciate your willingness to propose other solutions, but for these reasons, the proposed strings(1) patch would not suffice as an interface for printk enumeration.



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux