From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752322AbbJEQgr (ORCPT ); Mon, 5 Oct 2015 12:36:47 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:33613 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751569AbbJEQgq (ORCPT ); Mon, 5 Oct 2015 12:36:46 -0400 Date: Mon, 5 Oct 2015 18:36:41 +0200 From: Ingo Molnar To: Linus Torvalds Cc: Alexey Dobriyan , Chris Metcalf , Linux Kernel Mailing List , Peter Zijlstra , Thomas Gleixner , "H. Peter Anvin" , Borislav Petkov Subject: [PATCH] string: Fix strscpy() uninitialized data copy bug Message-ID: <20151005163641.GA11635@gmail.com> References: <20151005162226.GA10993@gmail.com> <20151005162802.GA11474@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151005162802.GA11474@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Ingo Molnar wrote: > A slightly more paranoid version would be: > > c = *(unsigned long *)(src+res); > > if (has_zero(c, &data, &constants)) { > unsigned int zero_pos; > > data = prep_zero_mask(c, data, &constants); data = > create_zero_mask(data); > > zero_pos = find_zero(data); > > /* Clear out undefined data within the final word after > the NUL: */ > memset((void *)&c + zero_pos, 0, sizeof(long)-zero_pos); > > *(unsigned long *)(dest+res) = c; > > return res+zero_pos; > } > *(unsigned long *)(dest+res) = c; > > This would solve any theoretical races in the _target_ buffer: if the target > buffer may be copied to user-space in a racy fashion and we don't ever want it > to have undefined data, then this variant does the tail-zeroing of the final > word in the temporary copy, not in the target buffer. > > Still untested. So the patch below got tested a bit more seriously, with the strscpy() based strlcpy() patch I sent earlier: at least a typical Fedora bootup with a few thousand strlcpy() uses does not crash in any obvious way. Still needs review to make sure I have not missed anything ... Thanks, Ingo ===================> >>From 946bab4d7138e5db53c5f1759e97809ebdf89551 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Mon, 5 Oct 2015 18:30:37 +0200 Subject: [PATCH] string: Fix strscpy() uninitialized data copy bug Alexey Dobriyan noticed that our new strscpy() implementation will copy potentially out of range or uninitialized data from post the end of the source string. Fix this by zeroing out the tail of the final word of the copy. Reported-by: Alexey Dobriyan Signed-off-by: Ingo Molnar --- lib/string.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/string.c b/lib/string.c index 6b89c035df74..548f52b7a145 100644 --- a/lib/string.c +++ b/lib/string.c @@ -177,12 +177,24 @@ ssize_t strscpy(char *dest, const char *src, size_t count) unsigned long c, data; c = *(unsigned long *)(src+res); - *(unsigned long *)(dest+res) = c; + if (has_zero(c, &data, &constants)) { + unsigned int zero_pos; + data = prep_zero_mask(c, data, &constants); data = create_zero_mask(data); + + zero_pos = find_zero(data); + + /* Clear out undefined data within the final word after the NUL (if any): */ + memset((void *)&c + zero_pos, 0, sizeof(long)-zero_pos); + + *(unsigned long *)(dest+res) = c; + return res + find_zero(data); } + *(unsigned long *)(dest+res) = c; + res += sizeof(unsigned long); count -= sizeof(unsigned long); max -= sizeof(unsigned long);