diff mbox series

[2/3] stdlib: Enforce PATH_MAX on allocated realpath buffer

Message ID 20200810204856.2111211-2-adhemerval.zanella@linaro.org
State New
Headers show
Series [1/3] stdlib: Use fixed buffer size for realpath (BZ #26241) | expand

Commit Message

Adhemerval Zanella Aug. 10, 2020, 8:48 p.m. UTC
It removes the buffer resize for sizes larger than PATH_MAX in the
case where the 'resolved' buffer is not specified.  This allow assume
realpath limit is PATH_MAX for all cases.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 stdlib/canonicalize.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

Comments

Florian Weimer Aug. 11, 2020, 8:26 a.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> It removes the buffer resize for sizes larger than PATH_MAX in the
> case where the 'resolved' buffer is not specified.  This allow assume
> realpath limit is PATH_MAX for all cases.

Is that actually true, in the sense that the full path cannot be
longer than PATH_MAX?

I don't think Linux has such a restriction.  One cannot open such
files directly, but they can exist.
Andreas Schwab Aug. 11, 2020, 9:48 a.m. UTC | #2
On Aug 10 2020, Adhemerval Zanella via Libc-alpha wrote:

> It removes the buffer resize for sizes larger than PATH_MAX in the
> case where the 'resolved' buffer is not specified.  This allow assume
> realpath limit is PATH_MAX for all cases.

"allows to assume" or "assumes".

Andreas.
Andreas Schwab Aug. 11, 2020, 9:54 a.m. UTC | #3
On Aug 11 2020, Florian Weimer wrote:

> I don't think Linux has such a restriction.  One cannot open such
> files directly, but they can exist.

You can open them with a relative name as long as cwd is nearer than
PATH_MAX (or use openat with such a directory handle).

Andreas.
Florian Weimer Aug. 11, 2020, 10:24 a.m. UTC | #4
* Andreas Schwab:

> On Aug 11 2020, Florian Weimer wrote:
>
>> I don't think Linux has such a restriction.  One cannot open such
>> files directly, but they can exist.
>
> You can open them with a relative name as long as cwd is nearer than
> PATH_MAX (or use openat with such a directory handle).

Yes, that's what I meant: one cannot use the full path directly.  It
has to be split up in some way.

I still think realpath should report the full path for them, and not
give up with an error.  Similar for getcwd.
Adhemerval Zanella Aug. 11, 2020, 3:05 p.m. UTC | #5
On 11/08/2020 07:24, Florian Weimer wrote:
> * Andreas Schwab:
> 
>> On Aug 11 2020, Florian Weimer wrote:
>>
>>> I don't think Linux has such a restriction.  One cannot open such
>>> files directly, but they can exist.
>>
>> You can open them with a relative name as long as cwd is nearer than
>> PATH_MAX (or use openat with such a directory handle).
> 
> Yes, that's what I meant: one cannot use the full path directly.  It
> has to be split up in some way.
> 
> I still think realpath should report the full path for them, and not
> give up with an error.  Similar for getcwd.
> 

The only issue I have with this approach is realpath has different semantic
regarding maximum pathname returned whether you pass a 'resolved' buffer
(which assume PATH_MAX).
Paul Eggert Aug. 11, 2020, 3:37 p.m. UTC | #6
On 8/11/20 8:05 AM, Adhemerval Zanella via Libc-alpha wrote:
> The only issue I have with this approach is realpath has different semantic
> regarding maximum pathname returned whether you pass a 'resolved' buffer
> (which assume PATH_MAX).

If realpath can successfully return a file name longer than PATH_MAX now, that 
suggests that it should continue to do so.
Florian Weimer Aug. 11, 2020, 6:29 p.m. UTC | #7
* Adhemerval Zanella via Libc-alpha:

> On 11/08/2020 07:24, Florian Weimer wrote:
>> * Andreas Schwab:
>> 
>>> On Aug 11 2020, Florian Weimer wrote:
>>>
>>>> I don't think Linux has such a restriction.  One cannot open such
>>>> files directly, but they can exist.
>>>
>>> You can open them with a relative name as long as cwd is nearer than
>>> PATH_MAX (or use openat with such a directory handle).
>> 
>> Yes, that's what I meant: one cannot use the full path directly.  It
>> has to be split up in some way.
>> 
>> I still think realpath should report the full path for them, and not
>> give up with an error.  Similar for getcwd.
>> 
>
> The only issue I have with this approach is realpath has different semantic
> regarding maximum pathname returned whether you pass a 'resolved' buffer
> (which assume PATH_MAX).

This is just our attempt to avoid the inherent buffer overflows in
this interface.  We had to do something similar for readdir_r.
diff mbox series

Patch

diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index 554ba221e4..91c30e38be 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -122,36 +122,16 @@  __realpath (const char *name, char *resolved)
 	}
       else
 	{
-	  size_t new_size;
-
 	  if (dest[-1] != '/')
 	    *dest++ = '/';
 
 	  if (dest + (end - start) >= rpath_limit)
 	    {
-	      ptrdiff_t dest_offset = dest - rpath;
-	      char *new_rpath;
-
-	      if (resolved)
-		{
-		  __set_errno (ENAMETOOLONG);
-		  if (dest > rpath + 1)
-		    dest--;
-		  *dest = '\0';
-		  goto error;
-		}
-	      new_size = rpath_limit - rpath;
-	      if (end - start + 1 > PATH_MAX)
-		new_size += end - start + 1;
-	      else
-		new_size += PATH_MAX;
-	      new_rpath = (char *) realloc (rpath, new_size);
-	      if (new_rpath == NULL)
-		goto error;
-	      rpath = new_rpath;
-	      rpath_limit = rpath + new_size;
-
-	      dest = rpath + dest_offset;
+	      __set_errno (ENAMETOOLONG);
+	      if (dest > rpath + 1)
+		dest--;
+	      *dest = '\0';
+	      goto error;
 	    }
 
 	  dest = __mempcpy (dest, start, end - start);