[uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Balbir Singh]

mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
a big overrun bug reported on UML bootup.

Reported-by: Vikas K Managutte <vikki.km@gmail.com>
Reported-by: Sarvesh Kumar Lal Das <skldas@gmail.com>
Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 arch/um/drivers/mconsole_kern.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -puN arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation arch/um/drivers/mconsole_kern.c
--- linux-2.6.28-rc6/arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation	2008-11-29 15:29:04.000000000 +0530
+++ linux-2.6.28-rc6-balbir/arch/um/drivers/mconsole_kern.c	2008-11-29 15:32:23.000000000 +0530
@@ -16,6 +16,8 @@
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/utsname.h>
+#include <linux/socket.h>
+#include <linux/un.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
 #include <asm/uaccess.h>
@@ -785,7 +787,7 @@ static int __init mconsole_init(void)
 	/* long to avoid size mismatch warnings from gcc */
 	long sock;
 	int err;
-	char file[256];
+	char file[UNIX_PATH_MAX];
 
 	if (umid_file_name("mconsole", file, sizeof(file)))
 		return -1;
_

-- 
	Balbir

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Pekka Enberg]

Hi Balbir,

On Sat, Nov 29, 2008 at 12:08 PM, Balbir Singh
<balbir@linux.vnet.ibm.com> wrote:
> mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> a big overrun bug reported on UML bootup.

Maybe mention that ->sun_path in struct sockaddr_un is UNIX_PATH_MAX
long which causes the problem?

> Reported-by: Vikas K Managutte <vikki.km@gmail.com>
> Reported-by: Sarvesh Kumar Lal Das <skldas@gmail.com>
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

> ---
>
>  arch/um/drivers/mconsole_kern.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff -puN arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation arch/um/drivers/mconsole_kern.c
> --- linux-2.6.28-rc6/arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation      2008-11-29 15:29:04.000000000 +0530
> +++ linux-2.6.28-rc6-balbir/arch/um/drivers/mconsole_kern.c     2008-11-29 15:32:23.000000000 +0530
> @@ -16,6 +16,8 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/utsname.h>
> +#include <linux/socket.h>
> +#include <linux/un.h>
>  #include <linux/workqueue.h>
>  #include <linux/mutex.h>
>  #include <asm/uaccess.h>
> @@ -785,7 +787,7 @@ static int __init mconsole_init(void)
>        /* long to avoid size mismatch warnings from gcc */
>        long sock;
>        int err;
> -       char file[256];
> +       char file[UNIX_PATH_MAX];
>
>        if (umid_file_name("mconsole", file, sizeof(file)))
>                return -1;
> _
>
> --
>        Balbir
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Balbir Singh]

* Pekka Enberg <penberg@cs.helsinki.fi> [2008-11-29 12:21:46]:

> Hi Balbir,
> 
> On Sat, Nov 29, 2008 at 12:08 PM, Balbir Singh
> <balbir@linux.vnet.ibm.com> wrote:
> > mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> > the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> > a big overrun bug reported on UML bootup.
> 
> Maybe mention that ->sun_path in struct sockaddr_un is UNIX_PATH_MAX
> long which causes the problem?
>

Yes, the changelog can definitely add that to the changelog, I'll
resend the patch if Jeff asks for it.

 
> > Reported-by: Vikas K Managutte <vikki.km@gmail.com>
> > Reported-by: Sarvesh Kumar Lal Das <skldas@gmail.com>
> > Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> 
> Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

Thanks!

-- 
	Balbir

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Américo Wang]

On Sat, Nov 29, 2008 at 10:08 AM, Balbir Singh
<balbir@linux.vnet.ibm.com> wrote:
>
> mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> a big overrun bug reported on UML bootup.
>
> Reported-by: Vikas K Managutte <vikki.km@gmail.com>
> Reported-by: Sarvesh Kumar Lal Das <skldas@gmail.com>
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> ---
>
>  arch/um/drivers/mconsole_kern.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff -puN arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation arch/um/drivers/mconsole_kern.c
> --- linux-2.6.28-rc6/arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation      2008-11-29 15:29:04.000000000 +0530
> +++ linux-2.6.28-rc6-balbir/arch/um/drivers/mconsole_kern.c     2008-11-29 15:32:23.000000000 +0530
> @@ -16,6 +16,8 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/utsname.h>
> +#include <linux/socket.h>
> +#include <linux/un.h>
>  #include <linux/workqueue.h>
>  #include <linux/mutex.h>
>  #include <asm/uaccess.h>
> @@ -785,7 +787,7 @@ static int __init mconsole_init(void)
>        /* long to avoid size mismatch warnings from gcc */
>        long sock;
>        int err;
> -       char file[256];
> +       char file[UNIX_PATH_MAX];
>


Aha, good spot!

Please also correct the length of mconsole_socket_name.

Anyway,

Reviewed-by: WANG Cong <wangcong@zeuux.org>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Balbir Singh]

* WANG Cong <xiyou.wangcong@gmail.com> [2008-11-29 13:03:42]:

> On Sat, Nov 29, 2008 at 10:08 AM, Balbir Singh
> <balbir@linux.vnet.ibm.com> wrote:
> >
> > mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> > the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> > a big overrun bug reported on UML bootup.
> >
> > Reported-by: Vikas K Managutte <vikki.km@gmail.com>
> > Reported-by: Sarvesh Kumar Lal Das <skldas@gmail.com>
> > Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> > ---
> >
> >  arch/um/drivers/mconsole_kern.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff -puN arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation arch/um/drivers/mconsole_kern.c
> > --- linux-2.6.28-rc6/arch/um/drivers/mconsole_kern.c~fix-uml-buggy-socket-creation      2008-11-29 15:29:04.000000000 +0530
> > +++ linux-2.6.28-rc6-balbir/arch/um/drivers/mconsole_kern.c     2008-11-29 15:32:23.000000000 +0530
> > @@ -16,6 +16,8 @@
> >  #include <linux/slab.h>
> >  #include <linux/syscalls.h>
> >  #include <linux/utsname.h>
> > +#include <linux/socket.h>
> > +#include <linux/un.h>
> >  #include <linux/workqueue.h>
> >  #include <linux/mutex.h>
> >  #include <asm/uaccess.h>
> > @@ -785,7 +787,7 @@ static int __init mconsole_init(void)
> >        /* long to avoid size mismatch warnings from gcc */
> >        long sock;
> >        int err;
> > -       char file[256];
> > +       char file[UNIX_PATH_MAX];
> >
> 
> 
> Aha, good spot!
> 
> Please also correct the length of mconsole_socket_name.
>

I did not fix that with UNIX_PATH_MAX since, mconsole_user.c already includes
sys/un.h and sys/socket.h and I did not want to include kernel headers
linux/socket.h and linux/un.h there. I could have hard-coded those
values to 108, but I just let it be for now. If you feel strongly
about it, it can be done.
 
> Anyway,
> 
> Reviewed-by: WANG Cong <wangcong@zeuux.org>

Thanks!

-- 
	Balbir

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Américo Wang]

On Sun, Nov 30, 2008 at 3:01 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>
> I did not fix that with UNIX_PATH_MAX since, mconsole_user.c already includes
> sys/un.h and sys/socket.h and I did not want to include kernel headers
> linux/socket.h and linux/un.h there. I could have hard-coded those
> values to 108, but I just let it be for now. If you feel strongly
> about it, it can be done.
>

No, you can leave it to me. :)

Thanks.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Andrew Morton]

On Sat, 29 Nov 2008 17:50:39 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> * Pekka Enberg <penberg@cs.helsinki.fi> [2008-11-29 12:21:46]:
> 
> > Hi Balbir,
> > 
> > On Sat, Nov 29, 2008 at 12:08 PM, Balbir Singh
> > <balbir@linux.vnet.ibm.com> wrote:
> > > mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> > > the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> > > a big overrun bug reported on UML bootup.
> > 
> > Maybe mention that ->sun_path in struct sockaddr_un is UNIX_PATH_MAX
> > long which causes the problem?
> >
> 
> Yes, the changelog can definitely add that to the changelog, I'll
> resend the patch if Jeff asks for it.

Jeff's been quiet lately - I'll merge this one.

I also tagged it for -stable, subject to Jeff's OK.  I'm wondering why
this wasn't observed in earlier kernels?


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Américo Wang]

On Tue, Dec 2, 2008 at 8:17 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Sat, 29 Nov 2008 17:50:39 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>
>> * Pekka Enberg <penberg@cs.helsinki.fi> [2008-11-29 12:21:46]:
>>
>> > Hi Balbir,
>> >
>> > On Sat, Nov 29, 2008 at 12:08 PM, Balbir Singh
>> > <balbir@linux.vnet.ibm.com> wrote:
>> > > mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
>> > > the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
>> > > a big overrun bug reported on UML bootup.
>> >
>> > Maybe mention that ->sun_path in struct sockaddr_un is UNIX_PATH_MAX
>> > long which causes the problem?
>> >
>>
>> Yes, the changelog can definitely add that to the changelog, I'll
>> resend the patch if Jeff asks for it.
>
> Jeff's been quiet lately - I'll merge this one.


Yes, indeed.

>
> I also tagged it for -stable, subject to Jeff's OK.  I'm wondering why
> this wasn't observed in earlier kernels?

Maybe because mconsole is not often used for uml? :)

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH][UML] Boot broken due to buffer overrun

[Balbir Singh]

* Andrew Morton <akpm@linux-foundation.org> [2008-12-02 12:17:07]:

> On Sat, 29 Nov 2008 17:50:39 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > * Pekka Enberg <penberg@cs.helsinki.fi> [2008-11-29 12:21:46]:
> > 
> > > Hi Balbir,
> > > 
> > > On Sat, Nov 29, 2008 at 12:08 PM, Balbir Singh
> > > <balbir@linux.vnet.ibm.com> wrote:
> > > > mconsole_init() passed 256 bytes as length in os_create_unix_socket, while
> > > > the sizeof UNIX_PATH_MAX is 108. This patch fixes that problem and avoids
> > > > a big overrun bug reported on UML bootup.
> > > 
> > > Maybe mention that ->sun_path in struct sockaddr_un is UNIX_PATH_MAX
> > > long which causes the problem?
> > >
> > 
> > Yes, the changelog can definitely add that to the changelog, I'll
> > resend the patch if Jeff asks for it.
> 
> Jeff's been quiet lately - I'll merge this one.
> 

Thanks!

> I also tagged it for -stable, subject to Jeff's OK.  I'm wondering why
> this wasn't observed in earlier kernels?
> 

I observed it with the newer toolchain that uses __USE_FORTIFY_LEVEL
and __FORTIFY_SOURCE


-- 
	Balbir

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel