linux-kernel - Re: [PATCH 36/38] vfs: Add a sample program for the new mount API [ver #10]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <18974.1533031913@warthog.procyon.org.uk>
Date:   Tue, 31 Jul 2018 11:11:53 +0100
From:   David Howells <dhowells@...hat.com>
To:     Pavel Machek <pavel@....cz>
Cc:     dhowells@...hat.com, "Theodore Y. Ts'o" <tytso@....edu>,
        Matthew Wilcox <willy@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 36/38] vfs: Add a sample program for the new mount API [ver #10]

Pavel Machek <pavel@....cz> wrote:

> Proposal is "message %s foo %s\0param 1\0param2\0", only strings
> allowed.

I think that's too strict and you will need to allow integer values, IP
addresses and possibly other things also.  It could certainly have a limited
set (e.g. no kernel pointers).

You also haven't proposed what you think the internal interface should look
like.

> That's simple enough, yet allows translations.

Yes and no.  From the point of view of translating it, yes, it's easier, but
from the point of view of actually writing these things in the kernel, it's
not.

So, currently, I have, say:

	cg_invalf(fc, "option or name mismatch, new: 0x%x \"%s\", old: 0x%x \"%s\"", ...);

Now I can split that to separate out the option change error from the option
change error:

	cg_invalf(fc, "Name mismatch, new: \"%s\", old: \"%s\"", ...);
	cg_invalf(fc, "Option mismatch, new: 0x%x, old: 0x%x", ...);

but we still need to communicate to the logging function how many arguments
we're actually passing.  Now, I wouldn't consider this a fast-path, so
actually counting the arguments encoded in the format string is probably fine
- otherwise we have to actually pass the number of arguments in, e.g.:

	cg_invalf(fc, "Option mismatch, new: 0x%x, old: 0x%x", 2, ...);

and then the kernel has to render the whole lot into 

Another example, in ext4 we have:

	printk(KERN_NOTICE "EXT4-fs (%s): last error at time %u: %.*s:%d",

so we could do something like:

	infof(fc, "EXT4-fs (%s): last error at time %u: %.*s:%d", ...);

on mount.  But the use of "%.*s" is a pain for your interface.  We can't pass
qualifiers like "*" to userspace, so either the caller would have to copy the
string first or the logging routines would have to edit the format string.
Though I suppose we could leave the editing to userspace.

David