All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] autofs4: check dev ioctl size before allocating
@ 2014-03-16  1:40 Sasha Levin
  2014-04-05 15:06 ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2014-03-16  1:40 UTC (permalink / raw)
  To: raven; +Cc: autofs, linux-kernel, Sasha Levin

There wasn't any check of the size passed from userspace before
trying to allocate the memory required.

This meant that userspace might request more space than allowed,
triggering an OOM.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 fs/autofs4/dev-ioctl.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index 3182c0e..86fa7af 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -103,6 +103,9 @@ static struct autofs_dev_ioctl *copy_dev_ioctl(struct autofs_dev_ioctl __user *i
 	if (tmp.size < sizeof(tmp))
 		return ERR_PTR(-EINVAL);
 
+	if (tmp.size > (PATH_MAX + sizeof(tmp)))
+		return ERR_PTR(-E2BIG);
+
 	return memdup_user(in, tmp.size);
 }
 
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] autofs4: check dev ioctl size before allocating
  2014-03-16  1:40 [PATCH] autofs4: check dev ioctl size before allocating Sasha Levin
@ 2014-04-05 15:06 ` Sasha Levin
  2014-04-06  3:03   ` Ian Kent
  0 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2014-04-05 15:06 UTC (permalink / raw)
  To: raven; +Cc: autofs, linux-kernel, Al Viro, linux-fsdevel

Ping? Anyone wants to take this?

On 03/15/2014 09:40 PM, Sasha Levin wrote:
> There wasn't any check of the size passed from userspace before
> trying to allocate the memory required.
> 
> This meant that userspace might request more space than allowed,
> triggering an OOM.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  fs/autofs4/dev-ioctl.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
> index 3182c0e..86fa7af 100644
> --- a/fs/autofs4/dev-ioctl.c
> +++ b/fs/autofs4/dev-ioctl.c
> @@ -103,6 +103,9 @@ static struct autofs_dev_ioctl *copy_dev_ioctl(struct autofs_dev_ioctl __user *i
>  	if (tmp.size < sizeof(tmp))
>  		return ERR_PTR(-EINVAL);
>  
> +	if (tmp.size > (PATH_MAX + sizeof(tmp)))
> +		return ERR_PTR(-E2BIG);
> +
>  	return memdup_user(in, tmp.size);
>  }
>  
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] autofs4: check dev ioctl size before allocating
  2014-04-05 15:06 ` Sasha Levin
@ 2014-04-06  3:03   ` Ian Kent
  2014-04-06  3:11     ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Kent @ 2014-04-06  3:03 UTC (permalink / raw)
  To: Sasha Levin; +Cc: autofs, linux-kernel, Al Viro, linux-fsdevel, akpm

On Sat, 2014-04-05 at 11:06 -0400, Sasha Levin wrote:
> Ping? Anyone wants to take this?

Is this causing a problem for users?
If it is I'll send it to Andrew straight away.

I do have this on my queue but don't have any other patches to send so
it's just sitting there.

If it worries you I'll send it to Andrew now.

> 
> On 03/15/2014 09:40 PM, Sasha Levin wrote:
> > There wasn't any check of the size passed from userspace before
> > trying to allocate the memory required.
> > 
> > This meant that userspace might request more space than allowed,
> > triggering an OOM.
> > 
> > Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> > ---
> >  fs/autofs4/dev-ioctl.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
> > index 3182c0e..86fa7af 100644
> > --- a/fs/autofs4/dev-ioctl.c
> > +++ b/fs/autofs4/dev-ioctl.c
> > @@ -103,6 +103,9 @@ static struct autofs_dev_ioctl *copy_dev_ioctl(struct autofs_dev_ioctl __user *i
> >  	if (tmp.size < sizeof(tmp))
> >  		return ERR_PTR(-EINVAL);
> >  
> > +	if (tmp.size > (PATH_MAX + sizeof(tmp)))
> > +		return ERR_PTR(-E2BIG);
> > +
> >  	return memdup_user(in, tmp.size);
> >  }
> >  
> > 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] autofs4: check dev ioctl size before allocating
  2014-04-06  3:03   ` Ian Kent
@ 2014-04-06  3:11     ` Sasha Levin
  2014-04-06  3:53       ` Ian Kent
  0 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2014-04-06  3:11 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs, linux-kernel, Al Viro, linux-fsdevel, akpm

On 04/05/2014 11:03 PM, Ian Kent wrote:
> On Sat, 2014-04-05 at 11:06 -0400, Sasha Levin wrote:
>> Ping? Anyone wants to take this?
> 
> Is this causing a problem for users?
> If it is I'll send it to Andrew straight away.
> 
> I do have this on my queue but don't have any other patches to send so
> it's just sitting there.
> 
> If it worries you I'll send it to Andrew now.

It's not really a pressing issue since /dev/autofs is accessible only to
root. I also doubt that it causes problems for actual users.

If there are no issues with the patch, I think it's best if you could
ack/sign off on it and if Andrew wants to take it now, he could. If he
doesn't, it could wait for whenever.

I just don't want it getting lost somewhere and never making it upstream.


Thanks,
Sasha

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] autofs4: check dev ioctl size before allocating
  2014-04-06  3:11     ` Sasha Levin
@ 2014-04-06  3:53       ` Ian Kent
  2014-04-07  0:03         ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Kent @ 2014-04-06  3:53 UTC (permalink / raw)
  To: Sasha Levin; +Cc: autofs, linux-kernel, Al Viro, linux-fsdevel, akpm

On Sat, 2014-04-05 at 23:11 -0400, Sasha Levin wrote:
> On 04/05/2014 11:03 PM, Ian Kent wrote:
> > On Sat, 2014-04-05 at 11:06 -0400, Sasha Levin wrote:
> >> Ping? Anyone wants to take this?
> > 
> > Is this causing a problem for users?
> > If it is I'll send it to Andrew straight away.
> > 
> > I do have this on my queue but don't have any other patches to send so
> > it's just sitting there.
> > 
> > If it worries you I'll send it to Andrew now.
> 
> It's not really a pressing issue since /dev/autofs is accessible only to
> root. I also doubt that it causes problems for actual users.
> 
> If there are no issues with the patch, I think it's best if you could
> ack/sign off on it and if Andrew wants to take it now, he could. If he
> doesn't, it could wait for whenever.
> 
> I just don't want it getting lost somewhere and never making it upstream.

OK, but since I'm going to send it I looked at it a bit closer.
Shouldn't the E2BIG be ENAMETOOLONG since it's a file path that's being
passed?

I can change that and send it if you agree with the change.

Ian


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] autofs4: check dev ioctl size before allocating
  2014-04-06  3:53       ` Ian Kent
@ 2014-04-07  0:03         ` Sasha Levin
  0 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2014-04-07  0:03 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs, linux-kernel, Al Viro, linux-fsdevel, akpm

On 04/05/2014 11:53 PM, Ian Kent wrote:
> On Sat, 2014-04-05 at 23:11 -0400, Sasha Levin wrote:
>> On 04/05/2014 11:03 PM, Ian Kent wrote:
>>> On Sat, 2014-04-05 at 11:06 -0400, Sasha Levin wrote:
>>>> Ping? Anyone wants to take this?
>>>
>>> Is this causing a problem for users?
>>> If it is I'll send it to Andrew straight away.
>>>
>>> I do have this on my queue but don't have any other patches to send so
>>> it's just sitting there.
>>>
>>> If it worries you I'll send it to Andrew now.
>>
>> It's not really a pressing issue since /dev/autofs is accessible only to
>> root. I also doubt that it causes problems for actual users.
>>
>> If there are no issues with the patch, I think it's best if you could
>> ack/sign off on it and if Andrew wants to take it now, he could. If he
>> doesn't, it could wait for whenever.
>>
>> I just don't want it getting lost somewhere and never making it upstream.
> 
> OK, but since I'm going to send it I looked at it a bit closer.
> Shouldn't the E2BIG be ENAMETOOLONG since it's a file path that's being
> passed?
> 
> I can change that and send it if you agree with the change.

Looks good to me, thanks!


Thanks,
Sasha


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH] autofs4: check dev ioctl size before allocating
@ 2014-04-07  1:42 Ian Kent
  0 siblings, 0 replies; 7+ messages in thread
From: Ian Kent @ 2014-04-07  1:42 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Sasha Levin, autofs mailing list, Kernel Mailing List

From: Sasha Levin <sasha.levin@oracle.com>

There wasn't any check of the size passed from userspace before
trying to allocate the memory required.

This meant that userspace might request more space than allowed,
triggering an OOM.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs4/dev-ioctl.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index 3182c0e..232e03d 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -103,6 +103,9 @@ static struct autofs_dev_ioctl *copy_dev_ioctl(struct autofs_dev_ioctl __user *i
 	if (tmp.size < sizeof(tmp))
 		return ERR_PTR(-EINVAL);
 
+	if (tmp.size > (PATH_MAX + sizeof(tmp)))
+		return ERR_PTR(-ENAMETOOLONG);
+
 	return memdup_user(in, tmp.size);
 }
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-04-07  1:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-16  1:40 [PATCH] autofs4: check dev ioctl size before allocating Sasha Levin
2014-04-05 15:06 ` Sasha Levin
2014-04-06  3:03   ` Ian Kent
2014-04-06  3:11     ` Sasha Levin
2014-04-06  3:53       ` Ian Kent
2014-04-07  0:03         ` Sasha Levin
2014-04-07  1:42 Ian Kent

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.