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

[PATCH] 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.

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);



-- 
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: [PATCH] autofs4: Do not potentially dereference NULL pointer returned by fget() in autofs_dev_ioctl_setpipefd()

[Ian Kent]

On Sat, 2010-12-18 at 22:43 +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.
> 
> 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>
Acked-by: Ian Kent <raven@themaw.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);
> 
> 
> 

Autofs SMBFS no write in files

[Issa]

[-- Attachment #1.1: Type: text/plain, Size: 770 bytes --]

Hello,


      Autofs accés smbfs no way to write in smbfs share ?

Autofs smbfs no writing ?

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


[-- Attachment #1.2: Type: text/html, Size: 1480 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Autofs SMBFS no write in files

[Carter, Joel]

[-- Attachment #1.1: Type: text/plain, Size: 1231 bytes --]

Hey there.

 

Who is the owner and what are the permissions of your /mnt/smb/win1 mount point after autofs kicks in? Can you write to it using root (or using sudo, same thing)?

 

Joel.

 

From: autofs-bounces@linux.kernel.org [mailto:autofs-bounces@linux.kernel.org] On Behalf Of Issa
Sent: January-02-11 4:53 AM
To: Jesper Juhl
Cc: autofs@linux.kernel.org; linux-kernel@vger.kernel.org; Ian Kent
Subject: [autofs] Autofs SMBFS no write in files

 

Hello,


Autofs accés smbfs no way to write in smbfs share ?


Autofs smbfs no writing ?

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


[-- Attachment #1.2: Type: text/html, Size: 5878 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Autofs SMBFS no write in files

[Issa]

[-- Attachment #1.1: Type: text/plain, Size: 1523 bytes --]

Hello thanks for u answer,


yes i need to make sudo for write in .





2011/1/5 Carter, Joel <JoelC@trailerwizards.com>

> Hey there.
>
>
>
> Who is the owner and what are the permissions of your /mnt/smb/win1 mount
> point after autofs kicks in? Can you write to it using root (or using sudo,
> same thing)?
>
>
>
> Joel.
>
>
>
> *From:* autofs-bounces@linux.kernel.org [mailto:
> autofs-bounces@linux.kernel.org] *On Behalf Of *Issa
> *Sent:* January-02-11 4:53 AM
> *To:* Jesper Juhl
> *Cc:* autofs@linux.kernel.org; linux-kernel@vger.kernel.org; Ian Kent
> *Subject:* [autofs] Autofs SMBFS no write in files
>
>
>
> Hello,
> Autofs accés smbfs no way to write in smbfs share ?
>
> Autofs smbfs no writing ?
>
> 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
>



-- 
Cordialement
Yassin Chouchane

[-- Attachment #1.2: Type: text/html, Size: 3358 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

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

[Jesper Juhl]

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? 

/Jesper


-- 
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: [PATCH] autofs4: Do not potentially dereference NULL pointer returned by fget() in autofs_dev_ioctl_setpipefd()

[Jesper Juhl]

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

-- 
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: [PATCH] autofs4: Do not potentially dereference NULL pointer returned by fget() in autofs_dev_ioctl_setpipefd()

[Ian Kent]

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

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

Autofs SMBFS no write in files

[Issa]

[-- Attachment #1.1: Type: text/plain, Size: 773 bytes --]

Hello,


      Autofs accés smbfs impossible d'écrire sur le disque ?

Autofs smbfs no writing ?

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


[-- Attachment #1.2: Type: text/html, Size: 1376 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs