BUG: possible NULL pointer dereference in nfs server

[Rick Macklem]

Roman Divacky wrote:
> > > Yea, so long as it includes a comment that states this is done to
> > > work around a stupid compiler bug.
> > 
> > Ugh.
> > 
> > The above has the following bugs:
> > - gross style bugs (lines longer than 80 characters)
> > - large code to do nothing
> > - would be even larger with a comment
> > - cannot actually work around any compiler bug involving abort(),
> > since it
> >    has no effect on production kernels where KASSERT() is null.
> > 
> > >> It is present even in your setup :) Just "objdump -d kernel |
> > >> grep
> > >> ud2" on kernel compiled
> > >> by clang.
> > >>
> > > I actually use gcc, but I believe you. I'll admit I still don't
> > > understand
> > > why having a trap instruction that never gets executed is a bad
> > > thing?
> > 
> > It isn't but trying to link to the noexistent function abort() on
> > arches that don't have any trap instruction is a bad thing.
> >  According
> > the above, this occurs on sparc64.  It must be a gcc bug, since
> > sparc64
> > doesn't have clang.  However, I couldn't get gcc on sparc64 to
> > generate
> > an abort() for *NULL.  Similarly on x86 (gcc is hard to test on
> > x86,
> > since it is broken (not installed) on FreeBSD cluster machines for
> 
> I was testing clang compiled kernel on sparc64.
> 
> The problem is that there's nothing making sure that the NULL pointer
> dereference doesnt happen. So if someone happens to call the function
> with
> wrong flags it will crash.
> 
> Thats why I want to add the KASSERT, to catch possible future cases
> when this happens.
> 
> Unfortunately our KASSERT is not an assert so to remove the actual
> abort/trap/ud2 I have to remove the flag.
> 
> 
> Ok?
> 
If it makes clang for sparc64 happy, I suppose so.
Another possible coding, which is maybe a little less ugly and might
fix the problem is:
change if (new_stp->ls_flags & NFSLCK_OPEN) {
to
       if ((new_stp->ls_flags & NFSLCK_OPEN) != 0 && new_lfpp != NULL) {

rick

> 
> Roman
>