Discussion:
canonicalize-lgpl: support paths of length > 2 GB
(too old to reply)
Bruno Haible
2016-10-14 00:57:09 UTC
Permalink
Raw Message
Hi,

If pathconf (name, _PC_PATH_MAX) returns a PATH_MAX value > 2 GB,
the readlink call in canonicalize-lgpl.c line 292 may return a length > 2 GB,
therefore the implicit cast (assignment) to 'int' will produce a wrong value.

If you agree with this patch, it'd be a good idea to propagate it into glibc
(it's the file stdlib/canonicalize.c there).

Bruno
Pádraig Brady
2016-10-14 12:38:17 UTC
Permalink
Raw Message
Post by Bruno Haible
Hi,
If pathconf (name, _PC_PATH_MAX) returns a PATH_MAX value > 2 GB,
the readlink call in canonicalize-lgpl.c line 292 may return a length > 2 GB,
therefore the implicit cast (assignment) to 'int' will produce a wrong value.
If you agree with this patch, it'd be a good idea to propagate it into glibc
(it's the file stdlib/canonicalize.c there).
There is a later cast to (long int) that would
similarly truncate large values on LLP64 systems.
How about something like this as well?

@@ -311,7 +312,7 @@ __realpath (const char *name, char *resolved)
}

len = strlen (end);
- if ((long int) (n + len) >= path_max)
+ if (SIZE_MAX - len <= n || path_max <= n + len)
{
freea (buf);
__set_errno (ENAMETOOLONG);

thanks,
Pádraig
Bruno Haible
2016-10-15 10:10:52 UTC
Permalink
Raw Message
Hi Pádraig,
Post by Pádraig Brady
There is a later cast to (long int) that would
similarly truncate large values on LLP64 systems.
How about something like this as well?
@@ -311,7 +312,7 @@ __realpath (const char *name, char *resolved)
}
len = strlen (end);
- if ((long int) (n + len) >= path_max)
+ if (SIZE_MAX - len <= n || path_max <= n + len)
{
freea (buf);
__set_errno (ENAMETOOLONG);
Thanks, also for the integer overflow check (useful also for the LP64
platforms). Here is the proposed revised patch.

Bruno
Pádraig Brady
2016-10-15 10:23:17 UTC
Permalink
Raw Message
Post by Bruno Haible
Hi Pádraig,
Thanks, also for the integer overflow check (useful also for the LP64
platforms). Here is the proposed revised patch.
- if ((long int) (n + len) >= path_max)
+ /* Check that n + len + 1 doesn't overflow and is <= path_max. */
+ if (n >= SIZE_MAX - len || n + len >= path_max)
+1

thanks
Bruno Haible
2016-10-15 12:52:18 UTC
Permalink
Raw Message
+1
OK. Pushed.

Bruno
Tom G. Christensen
2016-10-20 15:02:48 UTC
Permalink
Raw Message
Post by Bruno Haible
+1
OK. Pushed.
Unfortunately this broke the build on CentOS 3 and 4.

Here's the error from a CentOS 4 host:
gcc -std=gnu99 -DHAVE_CONFIG_H -DEXEEXT=\"\" -DEXEEXT=\"\" -DNO_XMALLOC
-DEXEEXT=\"\" -I. -I.. -DGNULIB_STRICT_CHECKING=1 -fvisi
bility=hidden -g -O2 -MT canonicalize-lgpl.o -MD -MP -MF $depbase.Tpo -c
-o canonicalize-lgpl.o canonicalize-lgpl.c &&\
mv -f $depbase.Tpo $depbase.Po
canonicalize-lgpl.c: In function `rpl_realpath':
canonicalize-lgpl.c:315: error: `SIZE_MAX' undeclared (first use in this
function)
canonicalize-lgpl.c:315: error: (Each undeclared identifier is reported
only once
canonicalize-lgpl.c:315: error: for each function it appears in.)
make[4]: *** [canonicalize-lgpl.o] Error 1

The full log is available here:
http://jupiterrise.com/autobuild/gnulib/log-201610200648492729791.txt

-tgc
Padraig Brady
2016-10-20 18:32:54 UTC
Permalink
Raw Message
Post by Tom G. Christensen
Unfortunately this broke the build on CentOS 3 and 4.
Thanks for testing.
The attached should address that.
Tom G. Christensen
2016-10-20 19:21:51 UTC
Permalink
Raw Message
Post by Padraig Brady
Post by Tom G. Christensen
Unfortunately this broke the build on CentOS 3 and 4.
Thanks for testing.
The attached should address that.
Yes, that fixed it for me.

-tgc
Bruno Haible
2016-10-20 19:48:57 UTC
Permalink
Raw Message
Post by Padraig Brady
The attached should address that.
Thanks Pádraig. Looks fine.

Bruno

Loading...