Re: udp6 fix for possible memory corruption

[Alexander Bluhm]

On Fri, Aug 23, 2013 at 12:47:10PM -0700, Loganaden Velvindron wrote:
 Hi,
 
 From NetBSD:
 
 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c?rev=1.41content-type=text/x-cvsweb-markuponly_with_tag=MAIN
 
 Under some circumstances, udp6_output() would call ip6_clearpktopts()
 with an uninitialized struct ip6_pktopts on the stack, opt.
 ip6_clearpktopts(opt, ...) could dereference dangling pointers,
 leading to memory corruption or a crash.  Now, udp6_output() calls
 ip6_clearpktopts(opt, ...) only if opt was initialized. Thanks to
 Clement LECIGNE for reporting this bug.
 
 I checked openbsd source code and it seems that the issue is present
 as well.

Yes, the release path looks wrong.  OK bluhm@

 
 Tentative diff:
 
 Index: udp6_output.c
 ===
 RCS file: /cvs/src/sys/netinet6/udp6_output.c,v
 retrieving revision 1.19
 diff -u -p -r1.19 udp6_output.c
 --- udp6_output.c 28 Mar 2013 16:45:16 -  1.19
 +++ udp6_output.c 23 Aug 2013 19:30:36 -
 @@ -119,7 +119,8 @@ udp6_output(struct in6pcb *in6p, struct 
   struct  in6_addr *laddr, *faddr;
   u_short fport;
   int error = 0;
 - struct ip6_pktopts *optp, opt;
 + struct ip6_pktopts *optp = NULL; 
 + struct ip6_pktopts opt;
   int priv;
   int af, hlen;
   int flags;
 @@ -284,7 +285,8 @@ release:
  
  releaseopt:
   if (control) {
 - ip6_clearpktopts(opt, -1);
 + if (optp == opt)
 + ip6_clearpktopts(opt, -1);
   m_freem(control);
   }
   return (error);

Re: udp6 fix for possible memory corruption

[Stefan Sperling]

On Fri, Aug 23, 2013 at 12:47:10PM -0700, Loganaden Velvindron wrote:
 Hi,
 
 From NetBSD:
 
 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c?rev=1.41content-type=text/x-cvsweb-markuponly_with_tag=MAIN
 
 Under some circumstances, udp6_output() would call ip6_clearpktopts()
 with an uninitialized struct ip6_pktopts on the stack, opt.
 ip6_clearpktopts(opt, ...) could dereference dangling pointers,
 leading to memory corruption or a crash.  Now, udp6_output() calls
 ip6_clearpktopts(opt, ...) only if opt was initialized. Thanks to
 Clement LECIGNE for reporting this bug.
 
 I checked openbsd source code and it seems that the issue is present
 as well.

No, this doesn't apply to OpenBSD.

In OpenBSD, udp6_output() starts off with a call to ip6_setpktopts()
if control is not NULL. This means that opt will always be initalised
before being cleaned up.

NetBSD does not initialise opt at the start of udp6_output().
Rather, they initalise it after an if (addr6) { ... } block of
code which in case of error can jump to the code that tries to
free opt (via 'goto release;'), without opt having been initalised.
That's what the extra if (optp == opt) is supposed to prevent.

The problem was introduced in NetBSD in this revision:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c.diff?r1=1.22r2=1.23only_with_tag=MAIN

 Tentative diff:
 
 Index: udp6_output.c
 ===
 RCS file: /cvs/src/sys/netinet6/udp6_output.c,v
 retrieving revision 1.19
 diff -u -p -r1.19 udp6_output.c
 --- udp6_output.c 28 Mar 2013 16:45:16 -  1.19
 +++ udp6_output.c 23 Aug 2013 19:30:36 -
 @@ -119,7 +119,8 @@ udp6_output(struct in6pcb *in6p, struct 
   struct  in6_addr *laddr, *faddr;
   u_short fport;
   int error = 0;
 - struct ip6_pktopts *optp, opt;
 + struct ip6_pktopts *optp = NULL; 
 + struct ip6_pktopts opt;
   int priv;
   int af, hlen;
   int flags;
 @@ -284,7 +285,8 @@ release:
  
  releaseopt:
   if (control) {
 - ip6_clearpktopts(opt, -1);
 + if (optp == opt)
 + ip6_clearpktopts(opt, -1);
   m_freem(control);
   }
   return (error);

udp6 fix for possible memory corruption

[Loganaden Velvindron]

Hi,

From NetBSD:

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c?rev=1.41content-type=text/x-cvsweb-markuponly_with_tag=MAIN

Under some circumstances, udp6_output() would call ip6_clearpktopts()
with an uninitialized struct ip6_pktopts on the stack, opt.
ip6_clearpktopts(opt, ...) could dereference dangling pointers,
leading to memory corruption or a crash.  Now, udp6_output() calls
ip6_clearpktopts(opt, ...) only if opt was initialized. Thanks to
Clement LECIGNE for reporting this bug.

I checked openbsd source code and it seems that the issue is present
as well.

Tentative diff:

Index: udp6_output.c
===
RCS file: /cvs/src/sys/netinet6/udp6_output.c,v
retrieving revision 1.19
diff -u -p -r1.19 udp6_output.c
--- udp6_output.c   28 Mar 2013 16:45:16 -  1.19
+++ udp6_output.c   23 Aug 2013 19:30:36 -
@@ -119,7 +119,8 @@ udp6_output(struct in6pcb *in6p, struct 
struct  in6_addr *laddr, *faddr;
u_short fport;
int error = 0;
-   struct ip6_pktopts *optp, opt;
+   struct ip6_pktopts *optp = NULL; 
+   struct ip6_pktopts opt;
int priv;
int af, hlen;
int flags;
@@ -284,7 +285,8 @@ release:
 
 releaseopt:
if (control) {
-   ip6_clearpktopts(opt, -1);
+   if (optp == opt)
+   ip6_clearpktopts(opt, -1);
m_freem(control);
}
return (error);