Messages in this thread | | | From | Rasmus Villemoes <> | Subject | Re: [PATCH] string: Improve the generic strlcpy() implementation | Date | Fri, 09 Oct 2015 10:10:43 +0200 |
| |
On Thu, Oct 08 2015, Ingo Molnar <mingo@kernel.org> wrote:
> * Linus Torvalds <torvalds@linux-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
| |