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

[Rachita Kothiyal]

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..

Thanks
Rachita