Bug 23065

I don't see the usage anywhere in fortran/module.c.

Never mind, it is indirect with PATH_MAX.

In fact this is a bad use of MAXPATHLEN because we could get a buffer overflow.

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

(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

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.

Well in both the cases in the fortran front-end, really an alloca should be used instead of MAXPATHLEN.

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.

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.

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.

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.

Created attachment 9405 [details]
path_max.diff

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.

Created attachment 9406 [details]
path_max.diff

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.

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

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

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

Fixed.