Re: [Bug binutils/136] New: BFD crashes when cached file can't be reopened

[Nick Clifton]

Hi Mikulas,

> BFD keeps cache of files and has only few files open at a time. When the file is
> not in open-descriptor cache, BFD reopens it. BFD crashes trying to work with
> NULL FILE * pointer, when reopening file fails.
>
> A lot of code in bfd expects that bfd_cache_lookup always returns non-zero FILE
> * pointer. However bfd_cache_lookup may call bfd_cache_lookup_worker and
> bfd_cache_lookup_worker may return NULL if file could not be reopened.
>
> You should either modify bfd_cache_lookup_worker to abort instead of returning
> NULL or check for a non-null pointer at each call to bfd_cache_lookup.
>  
You mean like the attached (and applied) patch ?

Thanks for bringing this to our attention.  I have applied the attached 
patch along with this ChangeLog entry:

2004-05-05  Nick Clifton  <address@hidden>

   PR/136
   * cache.c (bfd_cache_lookup_worker): Call abort() rather than
   returning NULL as most users of this function do not check its
   return value.

Cheers
 Nick


Index: bfd/cache.c
===================================================================
RCS file: /cvs/src/src/bfd/cache.c,v
retrieving revision 1.13
diff -c -3 -p -r1.13 cache.c
*** bfd/cache.c 22 Apr 2004 16:17:31 -0000      1.13
--- bfd/cache.c 5 May 2004 15:29:43 -0000
*************** DESCRIPTION
*** 438,444 ****
        quick answer.  Find a file descriptor for @var{abfd}.  If
        necessary, it open it.  If there are already more than
        <<BFD_CACHE_MAX_OPEN>> files open, it tries to close one first, to
!       avoid running out of file descriptors.
  */
  
  FILE *
--- 438,445 ----
        quick answer.  Find a file descriptor for @var{abfd}.  If
        necessary, it open it.  If there are already more than
        <<BFD_CACHE_MAX_OPEN>> files open, it tries to close one first, to
!       avoid running out of file descriptors.  It will abort rather than
!       returning NULL if it is unable to (re)open the @var{abfd}.
  */
  
  FILE *
*************** bfd_cache_lookup_worker (bfd *abfd)
*** 461,472 ****
      }
    else
      {
!       if (bfd_open_file (abfd) == NULL)
!       return NULL;
!       if (abfd->where != (unsigned long) abfd->where)
!       return NULL;
!       if (real_fseek ((FILE *) abfd->iostream, abfd->where, SEEK_SET) != 0)
!       return NULL;
      }
  
    return (FILE *) abfd->iostream;
--- 462,471 ----
      }
    else
      {
!       if (bfd_open_file (abfd) == NULL
!         || abfd->where != (unsigned long) abfd->where
!         || real_fseek ((FILE *) abfd->iostream, abfd->where, SEEK_SET) != 0)
!       abort ();
      }
  
    return (FILE *) abfd->iostream;