[Crash-utility] Re: [RFC] Patch for fixing 'files' command: TAKE 4

[Dave Anderson]

Rachita Kothiyal wrote:

> On Tue, Jan 03, 2006 at 04:28:36PM -0500, Dave Anderson wrote:
> > Dave Anderson wrote:
> >
> > > This patch-to-your-patch seems to fix it:
> > >
> > > RCS file: /nfs/projects/cvs/crash/filesys.c,v
> > > retrieving revision 1.45
> > > diff -r1.45 filesys.c
> > > 2015c2015,2016
> > > <       ulong files_struct_addr, fdtable_addr;
> > > ---
> > > >       ulong files_struct_addr;
> > > >       ulong fdtable_addr = 0;
> > > 2153c2154,2155
> > > <       if (!fdtable_addr || !files_struct_addr || max_fdset == 0 || max_fds == 0) {
> > > ---
> > > >       if ((VALID_MEMBER(files_struct_fdt) && !fdtable_addr) ||
> > > >           !files_struct_addr || max_fdset == 0 || max_fds == 0) {
> > >
> > > With fdtable_addr uninitialized, it only gets past the "if" statement
> > > above on a pre-2.6.14 kernel if there were non-zero garbage left there
> > > on the stack.  If there happened to be a zero there, the "if" path is
> > > taken inadvertently, and it just prints "No open files".
> > >
> > > But do you agree with my logic of the enclosed checking of the validity first
> > > and then the fdtable_addr?  Seems right.
> > >
> > > Dave
> >
> > Hi Rachita,
> >
> > AFAICT, if we're running 2.6.14 with fdtables, it doesn't appear
> > that there's any possibility of fdtable_addr ever being zero?
> > It begins life in alloc_files() pointing to the embedded fdtable
> > in the files_struct, and potentially can be reassigned to a
> > replacement fdtable allocated from expand_fdtable().  But I don't
> > see anywhere that it could be cleared.
> >
> > For that matter, my original check for a NULL files_struct_addr
> > also looks to be impossible, even in the case of kernel threads.
> > Maybe back in the 2.2 timeframe it could be NULL -- I just don't
> > remember...
> >
> > In any case, I prefer to leave the code as it is above -- just to
> > be absolutely safe.
> >
> Hi Dave
>
> You are right. We need to do the validity check on files_struct_fdt
> before we check for fdtable_addr.
>
> I was wondering if you want me to regenerate the patch with this check..
>

No -- that's fine.  I'm taking your patch as it was, plus the changes
above applied on top.

Thanks,
  Dave