Re: [PATCH] autofs4: Do not potentially dereference NULL pointer returned by fget() in autofs_dev_ioctl_setpipefd()

[Ian Kent <raven@themaw.net>]

On Mon, 2011-01-24 at 21:03 +0100, Jesper Juhl wrote:
> On Mon, 24 Jan 2011, Jesper Juhl wrote:
> 
> > Hi Ian,
> > 
> > On Sat, 18 Dec 2010, Jesper Juhl wrote:
> > 
> > > Hi,
> > > 
> > > In fs/autofs4/dev-ioctl.c::autofs_dev_ioctl_setpipefd() we call fget(), 
> > > which may return NULL, but we do not explicitly test for that NULL return 
> > > so we may end up dereferencing a NULL pointer - bad.
> > > 
> > > When I originally submitted this patch I had chosen EBUSY as the return 
> > > value to use if this happens. Ian Kent was kind enough to explain why that 
> > > would most likely be wrong and why EBADF should most likely be used 
> > > instead. This version of the patch uses EBADF.
> > > 
> > > 
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > >  dev-ioctl.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
> > > index eff9a41..a650d7e 100644
> > > --- a/fs/autofs4/dev-ioctl.c
> > > +++ b/fs/autofs4/dev-ioctl.c
> > > @@ -372,6 +372,10 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
> > >  		return -EBUSY;
> > >  	} else {
> > >  		struct file *pipe = fget(pipefd);
> > > +		if (!pipe) {
> > > +			err = -EBADF;
> > > +			goto out;
> > > +		}
> > >  		if (!pipe->f_op || !pipe->f_op->write) {
> > >  			err = -EPIPE;
> > >  			fput(pipe);
> > > 
> > 
> > It's been more than a month now since I submitted this updated patch 
> > adressing your feedback, but I've not seen any feedback on it.
> > Is it OK? Will you merge it? 
> > 
> Ok, I need to learn to search my mailbox better. I just saw that you did 
> indeed send a reply with an Acked-by: on december 28. 
> 
> Doesn't change the fact that I still need to find someone to actually 
> merge it...
> 

Often patches like this get merged whether I ack them or not.

Right now I'm struggling with the 2.6.38-rc changes since the merge of
vfs-scale together with the vfs-automount patches caused some breakage.

I will post it along with any other patches I end up with if it doesn't
get picked up beforehand.

Ian