Bug 23065 - MAXPATHLEN usage in fortran/{scanner,module}.c
Summary: MAXPATHLEN usage in fortran/{scanner,module}.c
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.0.2
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 21824
  Show dependency treegraph
 
Reported: 2005-07-25 20:34 UTC by Thomas Schwinge
Modified: 2005-08-19 16:47 UTC (History)
4 users (show)

See Also:
Host: i386-pc-gnu
Target: i386-pc-gnu
Build: i386-pc-gnu
Known to work:
Known to fail:
Last reconfirmed: 2005-07-25 20:43:32


Attachments
path_max.diff (1.18 KB, text/plain)
2005-08-01 18:26 UTC, Steve Kargl
Details
path_max.diff (1.18 KB, text/plain)
2005-08-01 18:26 UTC, Steve Kargl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schwinge 2005-07-25 20:34:26 UTC
Please do not use MAXPATHLEN, it is a arbitrary limit, and is not
defined on GNU.  Currently this makes building gcc-4.0 fail on GNU since 
fortran/module.c assumes that MAXPATHLEN is defined.  Please do not use these 
kind of limits in GNU programs.

Not having MAXPATHLEN is perfectly compliant with POSIX, same goes for
MAXHOSTNAMELEN, PATH_MAX etc.
Comment 1 Andrew Pinski 2005-07-25 20:40:50 UTC
I don't see the usage anywhere in fortran/module.c.
Comment 2 Andrew Pinski 2005-07-25 20:43:32 UTC
Never mind, it is indirect with PATH_MAX.

In fact this is a bad use of MAXPATHLEN because we could get a buffer overflow.
Comment 3 kargl 2005-07-25 21:41:49 UTC
So, GNU does not follow IEEE Std 1003.1, 2004?

gfortran.h contains

#include <limits.h>
#ifndef PATH_MAX               /* This is defined in a IEEE Std 1003.1, 2004 */
# include <sys/param.h>
# define PATH_MAX MAXPATHLEN
#endif
Comment 4 Thomas Schwinge 2005-08-01 11:07:33 UTC
(In reply to comment #3)
> So, GNU does not follow IEEE Std 1003.1, 2004?

We do follow IEEE Std 1003.1, 2004 here, the standard does not mandate that
PATH_MAX _must_ be defined.
If no limit exist on how long a file name can be, then one does not need to
define PATH_MAX.
And this is the case for the GNU system.

See also here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21706
Comment 5 kargl 2005-08-01 16:43:27 UTC
Posix says:

  Pathname Variable Values

  The values in the following list may be constants within an
  implementation or may vary from one pathname to another. For
  example, file systems or directories may have different characteristics.

  A definition of one of the values shall be omitted from the <limits.h>
  header on specific implementations where the corresponding value is equal
  to or greater than the stated minimum, but where the value can vary
  depending on the file to which it is applied. The actual value supported
  for a specific pathname shall be provided by the pathconf() function.

  {PATH_MAX}
    Maximum number of bytes in a pathname, including the terminating
    null character.
    Minimum Acceptable Value: {_POSIX_PATH_MAX}

So, does GNU define _POSIX_PATH_MAX?

Does GNU support pathconf()?

I read the other thread where it is suggested that a non-portable
GNU extension should be used.  The gfortran source is fairly clean
from such kludges, and I would oppose the introduction of one.
Comment 6 Andrew Pinski 2005-08-01 16:46:17 UTC
Well in both the cases in the fortran front-end, really an alloca should be used instead of MAXPATHLEN.
Comment 7 Alfred M. Szmidt 2005-08-01 17:48:08 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

   So, does GNU define _POSIX_PATH_MAX?

No.

   Does GNU support pathconf()?

Yes.

   I read the other thread where it is suggested that a non-portable
   GNU extension should be used.  The gfortran source is fairly clean
   from such kludges, and I would oppose the introduction of one.

In this case using getcwd(NULL, 0) (and it is easy to make this
portable), isn't neeed.  But there is nothing "kludgy" about GNU
programs using GNU extentions.

In either case, I have a patch in the brew that should fix all
MAXPATHLEN/PATH_MAX issues in the whole GCC tree, which I will post as
soon as I have tested it (should be in few days).

Happy hacking.
Comment 8 Steve Kargl 2005-08-01 18:13:52 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

On Mon, Aug 01, 2005 at 05:48:10PM -0000, ams at gnu dot org wrote:
> 
> Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c
> 
>    So, does GNU define _POSIX_PATH_MAX?
> 
> No.

Then GNU isn't POSIX compliant.

> 
>    Does GNU support pathconf()?
> 
> Yes.

Use pathconf instead of ...

> 
>    I read the other thread where it is suggested that a non-portable
>    GNU extension should be used.  The gfortran source is fairly clean
>    from such kludges, and I would oppose the introduction of one.
> 
> In this case using getcwd(NULL, 0) (and it is easy to make this
> portable), isn't neeed.  But there is nothing "kludgy" about GNU
> programs using GNU extentions.

this ugly hack.

> In either case, I have a patch in the brew that should fix all
> MAXPATHLEN/PATH_MAX issues in the whole GCC tree, which I will post as
> soon as I have tested it (should be in few days).

Don't bother with gfortran.  I've regression testing a patch
that uses alloca as suggested by Andrew.

Comment 9 Alfred M. Szmidt 2005-08-01 18:24:30 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

   >    So, does GNU define _POSIX_PATH_MAX?
   > 
   > No.

   Then GNU isn't POSIX compliant.

Sorry, I meant yes.  We do define _POSIX_PATH_MAX.  My brain failed to
communicate this to my fingers.

(As for GNU being POSIX compliant, we are POSIX compliant where it
makes sense)

   > 
   >    Does GNU support pathconf()?
   > 
   > Yes.

   Use pathconf instead of ...

   > 
   >    I read the other thread where it is suggested that a non-portable
   >    GNU extension should be used.  The gfortran source is fairly clean
   >    from such kludges, and I would oppose the introduction of one.
   > 
   > In this case using getcwd(NULL, 0) (and it is easy to make this
   > portable), isn't neeed.  But there is nothing "kludgy" about GNU
   > programs using GNU extentions.

   this ugly hack.

This isn't a ugly hack.  GNU programs should use GNU extentions where
possible.

   Don't bother with gfortran.  I've regression testing a patch that
   uses alloca as suggested by Andrew.

Thanks.
Comment 10 Steve Kargl 2005-08-01 18:26:11 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

On Mon, Aug 01, 2005 at 04:46:17PM -0000, pinskia at gcc dot gnu dot org wrote:
> 
> Well in both the cases in the fortran front-end, really an alloca
> should be used instead of MAXPATHLEN.
> 

The attached patches uses alloca to remove the use of PATH_MAX
from gfortran.  It also fixes one other nearby hardcoded buffer.

This has been bubblestrapped and regression tested on amd64-*-freebsd.

2005-08-01  Steven G. Kargl  <kargls@comcast.net>

	PR fortran/23065
	* gfortran.h: Remove PATH_MAX definition.
	* module.c (write_module,gfc_dump_module): Use alloca to allocate buffers.
	* scanner.s (gfc_release_include_path,form_from_filename): Ditto.

Comment 11 Steve Kargl 2005-08-01 18:26:12 UTC
Created attachment 9405 [details]
path_max.diff
Comment 12 Steve Kargl 2005-08-01 18:26:32 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

On Mon, Aug 01, 2005 at 04:46:17PM -0000, pinskia at gcc dot gnu dot org wrote:
> 
> Well in both the cases in the fortran front-end, really an alloca
> should be used instead of MAXPATHLEN.
> 

The attached patches uses alloca to remove the use of PATH_MAX
from gfortran.  It also fixes one other nearby hardcoded buffer.

This has been bubblestrapped and regression tested on amd64-*-freebsd.

2005-08-01  Steven G. Kargl  <kargls@comcast.net>

	PR fortran/23065
	* gfortran.h: Remove PATH_MAX definition.
	* module.c (write_module,gfc_dump_module): Use alloca to allocate buffers.
	* scanner.s (gfc_release_include_path,form_from_filename): Ditto.

Comment 13 Steve Kargl 2005-08-01 18:26:32 UTC
Created attachment 9406 [details]
path_max.diff
Comment 14 Andrew Pinski 2005-08-01 18:34:28 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c


On Aug 1, 2005, at 2:26 PM, Steve Kargl wrote:

> On Mon, Aug 01, 2005 at 04:46:17PM -0000, pinskia at gcc dot gnu dot 
> org wrote:
>>
>> Well in both the cases in the fortran front-end, really an alloca
>> should be used instead of MAXPATHLEN.
>>
>
> The attached patches uses alloca to remove the use of PATH_MAX
> from gfortran.  It also fixes one other nearby hardcoded buffer.

This what I was thinking of.

This also fixes a buffer overflow that could have happened in 
gfc_dump_module.

Thanks,
Andrew Pinski

PS  scanner.s should be scanner.c in the ChangeLog.

Comment 15 Tobias Schlüter 2005-08-14 22:02:32 UTC
Subject: Re:  MAXPATHLEN usage in fortran/{scanner,module}.c

Steve Kargl wrote:
> The attached patches uses alloca to remove the use of PATH_MAX
> from gfortran.  It also fixes one other nearby hardcoded buffer.
> 
> This has been bubblestrapped and regression tested on amd64-*-freebsd.
> 
> 2005-08-01  Steven G. Kargl  <kargls@comcast.net>
> 
> 	PR fortran/23065
> 	* gfortran.h: Remove PATH_MAX definition.
> 	* module.c (write_module,gfc_dump_module): Use alloca to allocate buffers.
> 	* scanner.s (gfc_release_include_path,form_from_filename): Ditto.
                  ^
typo.

> --- 1131,1142 ----
>     const char *fileext;
>     int i;
>   
> !   /* Find end of file name.  Note, filename is either a NULL pointer or
> !      a NUL terminated string.  */
>     i = 0;
> !   while (filename[i] != '\0')
>       i++;
>   
>     /* Find last period.  */
>     while (i >= 0 && (filename[i] != '.'))
>       i--;

If filename ever were a NULL pointer this would break, but it won't ever be
because gfc_post_options explicitly sets it to the empty string if no filename
is supplied on the command line.

The patch is ok if you remove the if in the beginning of gfc_new_file, i.e. change
  try
  gfc_new_file (const char *filename, gfc_source_form form)
  {
    try result;

    if (filename != NULL)
      {
        gfc_source_file = gfc_getmem (strlen (filename) + 1);
        strcpy (gfc_source_file, filename);
      }
    else
      gfc_source_file = NULL;

to gcc_assert(filename) plus the if part.

thanks,
- Tobi


Comment 16 CVS Commits 2005-08-19 09:05:11 UTC
Subject: Bug 23065

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	tobi@gcc.gnu.org	2005-08-19 09:05:03

Modified files:
	gcc/fortran    : ChangeLog gfortran.h module.c scanner.c 

Log message:
	2005-08-19  Steven G. Kargl  <kargls@comcast.net>
	
	PR fortran/23065
	* gfortran.h: Remove PATH_MAX definition.
	* module.c (write_module, gfc_dump_module): Use alloca to allocate
	buffers.
	* scanner.s (gfc_release_include_path, form_from_filename): Ditto.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/ChangeLog.diff?cvsroot=gcc&r1=1.526&r2=1.527
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/gfortran.h.diff?cvsroot=gcc&r1=1.80&r2=1.81
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/module.c.diff?cvsroot=gcc&r1=1.34&r2=1.35
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/scanner.c.diff?cvsroot=gcc&r1=1.23&r2=1.24

Comment 17 CVS Commits 2005-08-19 15:50:52 UTC
Subject: Bug 23065

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	tobi@gcc.gnu.org	2005-08-19 15:50:43

Modified files:
	gcc/fortran    : ChangeLog gfortran.h module.c scanner.c 

Log message:
	2005-08-19  Steven G. Kargl  <kargls@comcast.net>
	
	PR fortran/23065
	* gfortran.h: Remove PATH_MAX definition.
	* module.c (write_module, gfc_dump_module): Use alloca to allocate
	buffers.
	* scanner.c (gfc_release_include_path, form_from_filename): Ditto.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.335.2.108&r2=1.335.2.109
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/gfortran.h.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.58.2.14&r2=1.58.2.15
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/module.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.31.2.1&r2=1.31.2.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/scanner.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.16.10.5&r2=1.16.10.6

Comment 18 Tobias Schlüter 2005-08-19 15:51:19 UTC
Fixed.