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.41&content-type=text/x-cvsweb-markup&only_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.22&r2=1.23&only_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 -0000 1.19
> +++ udp6_output.c 23 Aug 2013 19:30:36 -0000
> @@ -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);
