On 2018-04-05 03:45, Andrew Morton wrote:
> On Wed, 4 Apr 2018 18:25:16 -0700 Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:
>
>> From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
>>
>> If the reiserfs mount option's journal name contains a '%' character,
>> it can lead to a WARN_ONCE() in lib/vsprintf.c::format_decode(),
>> saying: "Please remove unsupported %/ in format string."
>> That's OK until panic_on_warn is set, at which point it's dead, Jim.
>>
>> To placate this situation, check the journal name string for a '%'
>> character and return an error if one is found. Also print a warning
>> (one that won't panic the kernel) about the invalid journal name (e.g.):
>>
>> reiserfs: journal device name is invalid: %/file0
>>
>> (In this example, the caller app specified the journal device name as
>> "%/file0".)
>>
>
> Well, that is a valid filename and we should support it...
>
> Isn't the bug in journal_init_dev()?
Urgh. At first I was about to reply that the real bug was in reiserfs.h
for failing to annotate __reiserfs_warning with __printf(). But digging
into it, it turns out that it implements its own printf extensions, so
that's obviously a non-starter. Now, one thing is that some of those
extension clash with existing standard modifiers (%z and %h, so if
someone adds a correct %zu thing to print a size_t in reiserfs things
will break). But, and I hope I'm wrong about this and just hasn't had
enough coffee, this seems completely broken:
while ((k = is_there_reiserfs_struct(fmt1, &what)) != NULL) {
*k = 0;
p += vsprintf(p, fmt1, args);
switch (what) {
case 'k':
sprintf_le_key(p, va_arg(args, struct
reiserfs_key *));
break;
On architectures where va_list is a typedef for a one-element array of
some struct (x86-64), that works ok, because the vsprintf call can and
does update the args metadata. But when args is just a pointer into the
stack (i386), we don't know how much vsprintf consumed, and end up
consuming the same arguments again - only this time we may interpret
some random integer as a struct pointer...
A minimal program showing the difference:
#include <stdio.h>
#include <stdarg.h>
void f(const char *dummy, ...)
{
va_list ap;
int i;
va_start(ap, dummy);
for (i = 0; i < 5; ++i) {
vprintf("%d\n", ap);
printf("%d\n", va_arg(ap, int));
}
va_end(ap);
}
int main(int argc, char *argv[])
{
f("bla", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
return 0;
}
Compiling for native (x86-64), this produces $(seq 10). But with -m32,
one gets 1,1,2,2,3,3,4,4,5,5.
Assuming reiserfs (at least its debugging infrastructure) isn't broken
on a bunch of architectures, I'm obviously missing something
fundamental. Please enlighten me.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
