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

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

[Jesper Juhl]

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.

A comment in fget() says "File object ref couldn't be taken" when that 
function returns NULL, so I guess EBUSY is the proper error to return from 
autofs_dev_ioctl_setpipefd() when this happens, but I'm far from sure 
about this, so I'd like some feedback before this patch is merged.


Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 dev-ioctl.c |    4 ++++
 1 file changed, 4 insertions(+)

 compile tested only.

diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index eff9a41..ab551ee 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 = -EBUSY;
+			goto out;
+		}
 		if (!pipe->f_op || !pipe->f_op->write) {
 			err = -EPIPE;
 			fput(pipe);



-- 
Jesper Juhl <jj@chaosbits.net>            http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.

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

[Ian Kent]

On Mon, 2010-12-13 at 00:02 +0100, 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.

Oops, thanks for this.

> 
> A comment in fget() says "File object ref couldn't be taken" when that 
> function returns NULL, so I guess EBUSY is the proper error to return from 
> autofs_dev_ioctl_setpipefd() when this happens, but I'm far from sure 
> about this, so I'd like some feedback before this patch is merged.

Not sure EBUSY is the one to use.

What is happening here is that user space has opened a file handle it
thinks is against an autofs mount point (which may actually be covered
by another mount, hence the need for an ioctl) and is asking the kernel
to set it in the super block info struct so it can be used for control
operations. By the look of fget() a NULL return would happen if the file
table was full (ENFILE) or the file handle had already been closed
(probably EBADF) so perhaps EBADF would be a better choice.

> 
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  dev-ioctl.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
>  compile tested only.
> 
> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
> index eff9a41..ab551ee 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 = -EBUSY;
> +			goto out;
> +		}
>  		if (!pipe->f_op || !pipe->f_op->write) {
>  			err = -EPIPE;
>  			fput(pipe);
> 
> 
> 

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

[Jesper Juhl]

On Wed, 15 Dec 2010, Ian Kent wrote:

> On Mon, 2010-12-13 at 00:02 +0100, 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.
> 
> Oops, thanks for this.
> 
> > 
> > A comment in fget() says "File object ref couldn't be taken" when that 
> > function returns NULL, so I guess EBUSY is the proper error to return from 
> > autofs_dev_ioctl_setpipefd() when this happens, but I'm far from sure 
> > about this, so I'd like some feedback before this patch is merged.
> 
> Not sure EBUSY is the one to use.
> 
> What is happening here is that user space has opened a file handle it
> thinks is against an autofs mount point (which may actually be covered
> by another mount, hence the need for an ioctl) and is asking the kernel
> to set it in the super block info struct so it can be used for control
> operations. By the look of fget() a NULL return would happen if the file
> table was full (ENFILE) or the file handle had already been closed
> (probably EBADF) so perhaps EBADF would be a better choice.
> 
Ok. That makes sense.
I'll re-spin the patch with that change and re-submit it soon.

-- 
Jesper Juhl <jj@chaosbits.net>            http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [autofs] Autofs SMBFS no write in files

[Ian Kent]

On Tue, 2010-12-28 at 19:51 +0100, Issa wrote:
> Hello,
> 
> 
> Autofs accés smbfs impossible d'écrire sur le disque ?
> Autofs smbfs no writing ?

A bit off topic but is there some reason you need to use smbfs.
It's deprecated and I expect it will go away some time soon.
You should try using CIFS if you have a reasonably recent (or even not
so recent should be ok) kernel.

> 
> Im usint autofs with ubuntu 10.10
> Im using autofs like this
> 
> 
> sudo nano /etc/auto.master
> 
> 
> +auto.master
> /mnt/smb       /etc/auto.auto   --timeout=60 --ghost
> et le fichier sudo nano /etc/auto.auto
> 
> 
> #directory name         option for mount                                                  device to mount
> win1            -fstype=smbfs,rw,credentials=/etc/smb.auth         ://win1/docs/
> Authentication files :
> sudo nano /etc/smb.auth
> 
> 
> username=users1
> password=motDePasse
> domain=windowsDomaine
> 
> now with this i can access only in read
> /mnt/smb/win1
> 
> My question how add acess to write ?
> 
> 
> because i can write for the moment.
> 
> 
> thanks
> 
> 
> _______________________________________________
> autofs mailing list
> autofs@linux.kernel.org
> http://linux.kernel.org/mailman/listinfo/autofs