[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