From owner-freebsd-current@FreeBSD.ORG Sun Nov 27 16:56:36 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 060F21065672 for ; Sun, 27 Nov 2011 16:56:36 +0000 (UTC) (envelope-from davide.italiano@gmail.com) Received: from mail-vw0-f54.google.com (mail-vw0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 899CD8FC1C for ; Sun, 27 Nov 2011 16:56:35 +0000 (UTC) Received: by vbbfr13 with SMTP id fr13so421769vbb.13 for ; Sun, 27 Nov 2011 08:56:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=t3W7imdStAu9sFkDVoZTG5cEDafRndLd7v4MSy2IcNs=; b=k/Bm9akvJ/OYW9OP330ML7eMiUajr5/VntIlj1yIwWzG1GU+mOiowvSC7RG0v125Zh px1flSiLfyTpFbgjXW4AaMJt7MZ4rv1PIegi1kyHZv481JEn2XfsxKk7YiclSC64WD0P QCufaMO+wyk07Rne3JeG3zYoO6/5Paj7EO50M= MIME-Version: 1.0 Received: by 10.52.92.49 with SMTP id cj17mr30705081vdb.35.1322412994421; Sun, 27 Nov 2011 08:56:34 -0800 (PST) Received: by 10.52.164.164 with HTTP; Sun, 27 Nov 2011 08:56:34 -0800 (PST) In-Reply-To: <20111127162430.GA95971@stack.nl> References: <20111127154536.GA54043@freebsd.org> <20111127162430.GA95971@stack.nl> Date: Sun, 27 Nov 2011 17:56:34 +0100 Message-ID: From: Davide Italiano To: Jilles Tjoelker Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Alexander Best , freebsd-current@freebsd.org, freebsd-net@freebsd.org Subject: Re: possible array out of bounds access in sys/netinet/sctp_output.c X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 27 Nov 2011 16:56:36 -0000 On Sun, Nov 27, 2011 at 5:24 PM, Jilles Tjoelker wrote: > On Sun, Nov 27, 2011 at 03:45:36PM +0000, Alexander Best wrote: >> i've been playing with clang tot and noticed the following error: > >> /usr/local/bin/clang -c -O3 -pipe -fno-inline-functions -fno-strict-alia= sing -march=3Dcore2 -std=3Dc99 -g -fdiagnostics-show-option -fformat-extens= ions -Wall =A0-Wcast-qual -Winline -Wmissing-include-dirs =A0-Wmissing-prot= otypes -Wnested-externs -Wpointer-arith =A0-Wredundant-decls -Wstrict-proto= types -Wundef =A0-Wno-pointer-sign -nostdinc =A0-I. -I/usr/git-freebsd-head= /sys -I/usr/git-freebsd-head/sys/contrib/altq -D_KERNEL -DHAVE_KERNEL_OPTIO= N_HEADERS -include opt_global.h =A0-fno-omit-frame-pointer -mno-aes -mno-av= x -mcmodel=3Dkernel -mno-red-zone -mno-mmx -msoft-float =A0-fno-asynchronou= s-unwind-tables -ffreestanding -Wno-error=3Dtautological-compare -Wno-error= =3Dshift-count-negative =A0-Wno-error=3Dshift-count-overflow -Wno-error=3Ds= hift-overflow -Wno-error=3Dconversion =A0-Wno-error=3Dempty-body -Wno-error= =3Dgnu-designator -Wno-error=3Dformat =A0-Wno-error=3Dformat-invalid-specif= ier -Wno-error=3Dformat-extra-args -Werror =A0/usr/git-freebsd-head/sys/net= inet/sctp_output.c >> clang: warning: argument unused during compilation: '-fformat-extensions= ' >> /usr/git-freebsd-head/sys/netinet/sctp_output.c:4685:2: error: array ind= ex 1 is past the end of the array (which contains 1 element) [-Werror,-Warr= ay-bounds] >> =A0 =A0 =A0 =A0 sup_addr->addr_type[1] =3D htons(SCTP_IPV6_ADDRESS); >> =A0 =A0 =A0 =A0 ^ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~ >> /usr/git-freebsd-head/sys/netinet/sctp_header.h:84:2: note: array 'addr_= type' declared here >> =A0 =A0 =A0 =A0 uint16_t addr_type[SCTP_ARRAY_MIN_LEN]; /* array of supp= orted address >> =A0 =A0 =A0 =A0 ^ >> 1 error generated. >> *** Error code 1 >> >> Stop in /usr/obj/usr/git-freebsd-head/sys/GENERIC. >> *** Error code 1 >> >> Stop in /usr/git-freebsd-head. >> *** Error code 1 >> >> Stop in /usr/git-freebsd-head. > >> this is from a GENERIC kernel build (so INET + INET6) for amd64. is this= a >> false positive, or is length(sup_addr->addr_type) really =3D=3D 1, thus = making >> sup_addr->addr_type[1] an illegal access? > > This is the fairly common construct of a variable-length array at the > end of a struct. With C89, this was not allowed but defining one element > and allocating more elements worked in most implementations. C99 > recognized this need and created a way to do it, which looks like > uint16_t addr_type[];. This adds any necessary padding and allows access > to however many elements have been allocated. Also, if it is not at the > end of a struct it is an error. > > Using this new construct requires code changes because some code such as > fairly close to the error message relies on the size of the one element > already in the struct. > > -- > Jilles Tjoelker > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org= " > I looked at sctp_send_initiate() and it seems that independently from the number of types supported (IPV6/IPV4 or both) two elements are allocated in the array sup_addr->addr_type[0] . In case only a type is supported one of the elements is simply padding. (http://fxr.watson.org/fxr/source/netinet/sctp_output.c#L4670) . So, this should fix the issue, but maybe I'm wrong so feel free to correct = me. http://davit.altervista.org/sctp_header_types.diff I defined a new macro mainly because SCTP_ARRAY_MIN_LEN is used in another place, i.e. in the field name of struct sctp_host_name_param, defined in sctp_header.h). Thanks to arundel@ for testing. Regards Davide