Message-ID: <87vbagwjgc.fsf@rasmusvillemoes.dk>
Date:	Fri, 09 Oct 2015 10:10:43 +0200
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Chris Metcalf <cmetcalf@...hip.com>,
	open list <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] string: Improve the generic strlcpy() implementation

On Thu, Oct 08 2015, Ingo Molnar <mingo@...nel.org> wrote:

> * Linus Torvalds <torvalds@...ux-foundation.org> wrote:
>
>> So I really refuse to worry about the snprintf() family of functions wrt this 
>> race. I don't think it was hugely important for strlcpy() either - more of a 
>> "quality of implementation" issue rather than anything fundamental - but for 
>> snprintf and friends it's an almost unavoidable issue because of how snprintf 
>> works.
>>
[snip]
>> 
>> Can we get odd truncated printouts in the (very very very unlikely) case that 
>> the string is being changed? Yes. We just don't care.
>
> I do agree mostly, but I think we should still try to achieve the following two 
> properties, if possible sanely+cheaply+cleanly:
>
>  - the printed string should not contain spurious \0 bytes even if the %s source
>    'races'. [I think this is true currently.]

Sorry, no, that's not true currently.

>  - the return code should correctly represent what snprintf did to the target
>    string. [This might not be the case currently. But I'm not sure!]

It does, in fact, represent "the number of characters, excluding the
trailing nul byte, that would have been written if the output buffer is
big enough" - but in some cases some of those bytes may happen to be
'\0'.

[The %s race is the only way I can see spurious \0, but \0 can also
legitimately be put in the output using %c, or maybe these days also
with some %p extension.]

> Because that's a real concern I think: snprintf() return is used frequently to 
> iterate over buffers, and it should correctly and reliably represent what it did, 
> regardless of what the source buffer does - because snprintf obviously knows what 
> it did to the output buffer, it has full, race-free control over it.
>
> Whether left-alignment and other formatting details were calculated correctly, 
> etc. is a secondary concern and cannot be guaranteed, but we should at least 
> guarantee that we generated a single string, that we did nothing else, and that we 
> correctly returned its length.
>
> Agreed?

No. More precisely, I don't agree with left-alignment etc. being a
secondary concern.

It's hard not to agree with the overall "let's make it more robust if it
can be done sanely+cheaply+cleanly". I was a bit skeptical about whether
those three requirements could be met, since we'd have to do
byte-by-byte traversal of the string, maybe-copying it to the output as
we go along, but then right-alignment would require us to do a memmove,
but not before we've done some complicated bookkeeping
exercise. However, now that I read the source again, it seems that Al
Viro already did that exercise when he added dentry(). So maybe it's
doable without a net increase in LOC.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists