linux-kernel - Re: [patch] lib: check for strcpy() overflows to fixed length buffers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1507574.Y0KFKOdI6F@vostro.rjw.lan>
Date:	Wed, 30 Apr 2014 21:49:23 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	devel@...ica.org, kernel-hardening@...ts.openwall.com,
	Kees Cook <keescook@...omium.org>,
	Dave Jones <davej@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [patch] lib: check for strcpy() overflows to fixed length buffers

On Wednesday, April 30, 2014 06:08:44 PM Dan Carpenter wrote:
> There are sometimes where we know that we are doing an strcpy() into a
> fixed length buffer.  In those cases, we could verify that the strcpy()
> doesn't overflow.  This patch introduces DEBUG_STRICT_SLOW_STRCPY_CHECKS
> if you want to check for that.  The downside is that it makes strcpy
> slower.
> 
> I introduced __compiletime_size() to make this work.  It returns the
> size of the destination buffer or zero if the size isn't known.  The
> __compiletime_object_size() is similar but if you pass it a struct
> member then it returns the size of everything from there to the end of
> the struct.  Another difference is __compiletime_object_size() returns
> -1 for unknown sizes.
> 
> If you pass a char pointer to __compiletime_size() then it returns zero.
> 
> The strcpy() check ignores buffers with just one byte because people
> often use those for variable length strings at the end of a struct.
> 
> I have tested this patch lightly and created some bugs for it to detect
> but I have not detected any real life overflows.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> 
> diff --git a/include/acpi/platform/acenv.h b/include/acpi/platform/acenv.h
> index e863dd5..5e0fc2b 100644
> --- a/include/acpi/platform/acenv.h
> +++ b/include/acpi/platform/acenv.h

This is an ACPICA header and changes to it need to be submitted to the ACPICA
maintainers (as per MAINTAINERS).  We only get ACPICA changes from the
upstream project (except for really special situations).

> @@ -320,7 +320,7 @@
>  #define ACPI_STRSTR(s1,s2)      strstr((s1), (s2))
>  #define ACPI_STRCHR(s1,c)       strchr((s1), (c))
>  #define ACPI_STRLEN(s)          (acpi_size) strlen((s))
> -#define ACPI_STRCPY(d,s)        (void) strcpy((d), (s))
> +#define ACPI_STRCPY(d,s)        strcpy((d), (s))
>  #define ACPI_STRNCPY(d,s,n)     (void) strncpy((d), (s), (acpi_size)(n))
>  #define ACPI_STRNCMP(d,s,n)     strncmp((d), (s), (acpi_size)(n))
>  #define ACPI_STRCMP(d,s)        strcmp((d), (s))

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/