linux-kernel - Re: [PATCH] Cleanup: snprintf() always NUL-terminates: depend on it

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: <CA+55aFw8aMBF5uXekxATymQPTqwZHGJTL-jQx8zDqOwbaft+4g@mail.gmail.com>
Date:	Sun, 4 Jan 2015 11:34:43 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Giel van Schijndel <me@...tis.eu>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Brian King <brking@...ibm.com>,
	"James E.J. Bottomley" <JBottomley@...allels.com>,
	"open list:SCSI SUBSYSTEM" <linux-scsi@...r.kernel.org>
Subject: Re: [PATCH] Cleanup: snprintf() always NUL-terminates: depend on it

On Sun, Jan 4, 2015 at 10:05 AM, Giel van Schijndel <me@...tis.eu> wrote:
> Especially since one very strange piece of code seems to be written in
> such a way that a NUL needs to be placed where a NUL is present already.

Actually, it's worse than that. This:

>         len = snprintf(fname, 99, "%s", buf);
> -       fname[len-1] = '\0';

is complete garbage, since the return value of snprintf() is not the
length of the result, but length of what the result *would* have been.

So if the string doesn't fit in 99 bytes, it will actively corrupt
some random memory after the string. It's not writing zero to what was
already zero, it's corrupting memory.

Anyway, from a quick glance your patches look fine, but you need to
sign off on them. See Documentation/SubmittingPatches.

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