Skip to content

Commit

Permalink
Patches for glibc to compile with GCC7
Browse files Browse the repository at this point in the history
Signed-off-by: Alexey Neyman <stilor@att.net>
  • Loading branch information
stilor committed May 14, 2017
1 parent e943889 commit 5d792b4
Show file tree
Hide file tree
Showing 40 changed files with 2,223 additions and 0 deletions.
60 changes: 60 additions & 0 deletions patches/glibc/2.12.1/920-fix-rpc_parse-format.patch
@@ -0,0 +1,60 @@
commit 5874510faaf3cbd0bb112aaacab9f225002beed1
Author: Joseph Myers <joseph@codesourcery.com>
Date: Tue Nov 8 23:44:51 2016 +0000

Fix rpcgen buffer overrun (bug 20790).

Building with GCC 7 produces an error building rpcgen:

rpc_parse.c: In function 'get_prog_declaration':
rpc_parse.c:543:25: error: may write a terminating nul past the end of the destination [-Werror=format-length=]
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
~~~~^
rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a destination of size 10
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That buffer overrun is for the case where the .x file declares a
program with a million arguments. The strcpy two lines above can
generate a buffer overrun much more simply for a long argument name.

The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
provides a bound on the buffer size needed, so this patch just changes
the buffer size to MAXLINESIZE to avoid both possible buffer
overruns. A testcase is added that rpcgen does not crash with a
500-character argument name, where it previously crashed.

It would not at all surprise me if there are many other ways of
crashing rpcgen with either valid or invalid input; fuzz testing would
likely find various such bugs, though I don't think they are that
important to fix (rpcgen is not that likely to be used with untrusted
.x files as input). (As well as fuzz-findable bugs there are probably
also issues when various int variables get overflowed on very large
input.) The test infrastructure for rpcgen-not-crashing tests would
need extending if tests are to be added for cases where rpcgen should
produce an error, as opposed to cases where it should succeed.

Tested for x86_64 and x86.

[BZ #20790]
* sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
to MAXLINESIZE.
* sunrpc/bug20790.x: New file.
* sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
variable.
[$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
[$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

diff --git a/sunrpc/rpc_parse.c b/sunrpc/rpc_parse.c
index 1a1df6d8c2..505a6554cf 100644
--- a/sunrpc/rpc_parse.c
+++ b/sunrpc/rpc_parse.c
@@ -521,7 +521,7 @@ static void
get_prog_declaration (declaration * dec, defkind dkind, int num /* arg number */ )
{
token tok;
- char name[10]; /* argument name */
+ char name[MAXLINESIZE]; /* argument name */

if (dkind == DEF_PROGRAM)
{
62 changes: 62 additions & 0 deletions patches/glibc/2.12.1/940-nis-bogus-conditional.patch
@@ -0,0 +1,62 @@
commit f88759ea9bd3c8d8fef28f123ba9767cb0e421a3
Author: Joseph Myers <joseph@codesourcery.com>
Date: Wed Dec 21 23:44:01 2016 +0000

Fix nss_nisplus build with mainline GCC (bug 20978).

glibc build with current mainline GCC fails because
nis/nss_nisplus/nisplus-alias.c contains code

if (name != NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;
}

char buf[strlen (name) + 9 + tablename_len];

producing an error about strlen being called on a pointer that is
always NULL (and a subsequent use of that pointer with a %s format in
snprintf).

As Andreas noted, the bogus conditional comes from a 1997 change:

- if (name == NULL || strlen(name) > 8)
- return NSS_STATUS_NOTFOUND;
- else
+ if (name != NULL || strlen(name) <= 8)

So the intention is clearly to return an error for NULL name.

This patch duly inverts the sense of the conditional. It fixes the
build with GCC mainline, and passes usual glibc testsuite testing for
x86_64. However, I have not tried any actual substantive nisplus
testing, do not have an environment for such testing, and do not know
whether it is possible that strlen (name) or tablename_len might be
large so that the VLA for buf is actually a security issue. However,
if it is a security issue, there are plenty of other similar instances
in the nisplus code (that haven't been hidden by a bogus comparison
with NULL) - and nis_table.c:__create_ib_request uses strdupa on the
string passed to nis_list, so a local fix in the caller wouldn't
suffice anyway (see bug 20987). (Calls to strdupa and other such
macros that use alloca must be considered equally questionable
regarding stack overflow issues as direct calls to alloca and VLA
declarations.)

[BZ #20978]
* nis/nss_nisplus/nisplus-alias.c (_nss_nisplus_getaliasbyname_r):
Compare name == NULL, not name != NULL.

diff --git a/nis/nss_nisplus/nisplus-alias.c b/nis/nss_nisplus/nisplus-alias.c
index 7f698b4e6d..cb5acce01d 100644
--- a/nis/nss_nisplus/nisplus-alias.c
+++ b/nis/nss_nisplus/nisplus-alias.c
@@ -291,7 +291,7 @@ _nss_nisplus_getaliasbyname_r (const char *name, struct aliasent *alias,
return status;
}

- if (name != NULL)
+ if (name == NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;
60 changes: 60 additions & 0 deletions patches/glibc/2.12.2/920-fix-rpc_parse-format.patch
@@ -0,0 +1,60 @@
commit 5874510faaf3cbd0bb112aaacab9f225002beed1
Author: Joseph Myers <joseph@codesourcery.com>
Date: Tue Nov 8 23:44:51 2016 +0000

Fix rpcgen buffer overrun (bug 20790).

Building with GCC 7 produces an error building rpcgen:

rpc_parse.c: In function 'get_prog_declaration':
rpc_parse.c:543:25: error: may write a terminating nul past the end of the destination [-Werror=format-length=]
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
~~~~^
rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a destination of size 10
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That buffer overrun is for the case where the .x file declares a
program with a million arguments. The strcpy two lines above can
generate a buffer overrun much more simply for a long argument name.

The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
provides a bound on the buffer size needed, so this patch just changes
the buffer size to MAXLINESIZE to avoid both possible buffer
overruns. A testcase is added that rpcgen does not crash with a
500-character argument name, where it previously crashed.

It would not at all surprise me if there are many other ways of
crashing rpcgen with either valid or invalid input; fuzz testing would
likely find various such bugs, though I don't think they are that
important to fix (rpcgen is not that likely to be used with untrusted
.x files as input). (As well as fuzz-findable bugs there are probably
also issues when various int variables get overflowed on very large
input.) The test infrastructure for rpcgen-not-crashing tests would
need extending if tests are to be added for cases where rpcgen should
produce an error, as opposed to cases where it should succeed.

Tested for x86_64 and x86.

[BZ #20790]
* sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
to MAXLINESIZE.
* sunrpc/bug20790.x: New file.
* sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
variable.
[$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
[$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

diff --git a/sunrpc/rpc_parse.c b/sunrpc/rpc_parse.c
index 1a1df6d8c2..505a6554cf 100644
--- a/sunrpc/rpc_parse.c
+++ b/sunrpc/rpc_parse.c
@@ -521,7 +521,7 @@ static void
get_prog_declaration (declaration * dec, defkind dkind, int num /* arg number */ )
{
token tok;
- char name[10]; /* argument name */
+ char name[MAXLINESIZE]; /* argument name */

if (dkind == DEF_PROGRAM)
{
62 changes: 62 additions & 0 deletions patches/glibc/2.12.2/940-nis-bogus-conditional.patch
@@ -0,0 +1,62 @@
commit f88759ea9bd3c8d8fef28f123ba9767cb0e421a3
Author: Joseph Myers <joseph@codesourcery.com>
Date: Wed Dec 21 23:44:01 2016 +0000

Fix nss_nisplus build with mainline GCC (bug 20978).

glibc build with current mainline GCC fails because
nis/nss_nisplus/nisplus-alias.c contains code

if (name != NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;
}

char buf[strlen (name) + 9 + tablename_len];

producing an error about strlen being called on a pointer that is
always NULL (and a subsequent use of that pointer with a %s format in
snprintf).

As Andreas noted, the bogus conditional comes from a 1997 change:

- if (name == NULL || strlen(name) > 8)
- return NSS_STATUS_NOTFOUND;
- else
+ if (name != NULL || strlen(name) <= 8)

So the intention is clearly to return an error for NULL name.

This patch duly inverts the sense of the conditional. It fixes the
build with GCC mainline, and passes usual glibc testsuite testing for
x86_64. However, I have not tried any actual substantive nisplus
testing, do not have an environment for such testing, and do not know
whether it is possible that strlen (name) or tablename_len might be
large so that the VLA for buf is actually a security issue. However,
if it is a security issue, there are plenty of other similar instances
in the nisplus code (that haven't been hidden by a bogus comparison
with NULL) - and nis_table.c:__create_ib_request uses strdupa on the
string passed to nis_list, so a local fix in the caller wouldn't
suffice anyway (see bug 20987). (Calls to strdupa and other such
macros that use alloca must be considered equally questionable
regarding stack overflow issues as direct calls to alloca and VLA
declarations.)

[BZ #20978]
* nis/nss_nisplus/nisplus-alias.c (_nss_nisplus_getaliasbyname_r):
Compare name == NULL, not name != NULL.

diff --git a/nis/nss_nisplus/nisplus-alias.c b/nis/nss_nisplus/nisplus-alias.c
index 7f698b4e6d..cb5acce01d 100644
--- a/nis/nss_nisplus/nisplus-alias.c
+++ b/nis/nss_nisplus/nisplus-alias.c
@@ -291,7 +291,7 @@ _nss_nisplus_getaliasbyname_r (const char *name, struct aliasent *alias,
return status;
}

- if (name != NULL)
+ if (name == NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;
60 changes: 60 additions & 0 deletions patches/glibc/2.13/920-fix-rpc_parse-format.patch
@@ -0,0 +1,60 @@
commit 5874510faaf3cbd0bb112aaacab9f225002beed1
Author: Joseph Myers <joseph@codesourcery.com>
Date: Tue Nov 8 23:44:51 2016 +0000

Fix rpcgen buffer overrun (bug 20790).

Building with GCC 7 produces an error building rpcgen:

rpc_parse.c: In function 'get_prog_declaration':
rpc_parse.c:543:25: error: may write a terminating nul past the end of the destination [-Werror=format-length=]
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
~~~~^
rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a destination of size 10
sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That buffer overrun is for the case where the .x file declares a
program with a million arguments. The strcpy two lines above can
generate a buffer overrun much more simply for a long argument name.

The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
provides a bound on the buffer size needed, so this patch just changes
the buffer size to MAXLINESIZE to avoid both possible buffer
overruns. A testcase is added that rpcgen does not crash with a
500-character argument name, where it previously crashed.

It would not at all surprise me if there are many other ways of
crashing rpcgen with either valid or invalid input; fuzz testing would
likely find various such bugs, though I don't think they are that
important to fix (rpcgen is not that likely to be used with untrusted
.x files as input). (As well as fuzz-findable bugs there are probably
also issues when various int variables get overflowed on very large
input.) The test infrastructure for rpcgen-not-crashing tests would
need extending if tests are to be added for cases where rpcgen should
produce an error, as opposed to cases where it should succeed.

Tested for x86_64 and x86.

[BZ #20790]
* sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
to MAXLINESIZE.
* sunrpc/bug20790.x: New file.
* sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
variable.
[$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
[$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

diff --git a/sunrpc/rpc_parse.c b/sunrpc/rpc_parse.c
index 1a1df6d8c2..505a6554cf 100644
--- a/sunrpc/rpc_parse.c
+++ b/sunrpc/rpc_parse.c
@@ -521,7 +521,7 @@ static void
get_prog_declaration (declaration * dec, defkind dkind, int num /* arg number */ )
{
token tok;
- char name[10]; /* argument name */
+ char name[MAXLINESIZE]; /* argument name */

if (dkind == DEF_PROGRAM)
{
62 changes: 62 additions & 0 deletions patches/glibc/2.13/940-nis-bogus-conditional.patch
@@ -0,0 +1,62 @@
commit f88759ea9bd3c8d8fef28f123ba9767cb0e421a3
Author: Joseph Myers <joseph@codesourcery.com>
Date: Wed Dec 21 23:44:01 2016 +0000

Fix nss_nisplus build with mainline GCC (bug 20978).

glibc build with current mainline GCC fails because
nis/nss_nisplus/nisplus-alias.c contains code

if (name != NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;
}

char buf[strlen (name) + 9 + tablename_len];

producing an error about strlen being called on a pointer that is
always NULL (and a subsequent use of that pointer with a %s format in
snprintf).

As Andreas noted, the bogus conditional comes from a 1997 change:

- if (name == NULL || strlen(name) > 8)
- return NSS_STATUS_NOTFOUND;
- else
+ if (name != NULL || strlen(name) <= 8)

So the intention is clearly to return an error for NULL name.

This patch duly inverts the sense of the conditional. It fixes the
build with GCC mainline, and passes usual glibc testsuite testing for
x86_64. However, I have not tried any actual substantive nisplus
testing, do not have an environment for such testing, and do not know
whether it is possible that strlen (name) or tablename_len might be
large so that the VLA for buf is actually a security issue. However,
if it is a security issue, there are plenty of other similar instances
in the nisplus code (that haven't been hidden by a bogus comparison
with NULL) - and nis_table.c:__create_ib_request uses strdupa on the
string passed to nis_list, so a local fix in the caller wouldn't
suffice anyway (see bug 20987). (Calls to strdupa and other such
macros that use alloca must be considered equally questionable
regarding stack overflow issues as direct calls to alloca and VLA
declarations.)

[BZ #20978]
* nis/nss_nisplus/nisplus-alias.c (_nss_nisplus_getaliasbyname_r):
Compare name == NULL, not name != NULL.

diff --git a/nis/nss_nisplus/nisplus-alias.c b/nis/nss_nisplus/nisplus-alias.c
index 7f698b4e6d..cb5acce01d 100644
--- a/nis/nss_nisplus/nisplus-alias.c
+++ b/nis/nss_nisplus/nisplus-alias.c
@@ -291,7 +291,7 @@ _nss_nisplus_getaliasbyname_r (const char *name, struct aliasent *alias,
return status;
}

- if (name != NULL)
+ if (name == NULL)
{
*errnop = EINVAL;
return NSS_STATUS_UNAVAIL;

0 comments on commit 5d792b4

Please sign in to comment.