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
| ||
|
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