Re: possible array out of bounds access in sys/netinet/sctp_outp

On 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:
>=20
>> /usr/local/bin/clang -c -O3 -pipe -fno-inline-functions =
-fno-strict-aliasing -march=3Dcore2 -std=3Dc99 -g =
-fdiagnostics-show-option -fformat-extensions -Wall  -Wcast-qual =
-Winline -Wmissing-include-dirs  -Wmissing-prototypes -Wnested-externs =
-Wpointer-arith  -Wredundant-decls -Wstrict-prototypes -Wundef  =
-Wno-pointer-sign -nostdinc  -I. -I/usr/git-freebsd-head/sys =
-I/usr/git-freebsd-head/sys/contrib/altq -D_KERNEL =
-DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h  =
-fno-omit-frame-pointer -mno-aes -mno-avx -mcmodel=3Dkernel =
-mno-red-zone -mno-mmx -msoft-float  -fno-asynchronous-unwind-tables =
-ffreestanding -Wno-error=3Dtautological-compare =
-Wno-error=3Dshift-count-negative  -Wno-error=3Dshift-count-overflow =
-Wno-error=3Dshift-overflow -Wno-error=3Dconversion  =
-Wno-error=3Dempty-body -Wno-error=3Dgnu-designator -Wno-error=3Dformat  =
-Wno-error=3Dformat-invalid-specifier -Wno-error=3Dformat-extra-args =
-Werror  /usr/git-freebsd-head/sys/netinet/sctp_output.c
>> clang: warning: argument unused during compilation: =
'-fformat-extensions'
>> /usr/git-freebsd-head/sys/netinet/sctp_output.c:4685:2: error: array =
index 1 is past the end of the array (which contains 1 element) =
[-Werror,-Warray-bounds]
>>        sup_addr->addr_type[1] =3D htons(SCTP_IPV6_ADDRESS);
>>        ^                   ~
>> /usr/git-freebsd-head/sys/netinet/sctp_header.h:84:2: note: array =
'addr_type' declared here
>>        uint16_t addr_type[SCTP_ARRAY_MIN_LEN]; /* array of supported =
address
>>        ^
>> 1 error generated.
>> *** Error code 1
>>=20
>> Stop in /usr/obj/usr/git-freebsd-head/sys/GENERIC.
>> *** Error code 1
>>=20
>> Stop in /usr/git-freebsd-head.
>> *** Error code 1
>>=20
>> Stop in /usr/git-freebsd-head.
>=20
>> 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?
>=20
> 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.
>=20
> 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.
Hi Jilles,

you are completely right. It is a false positive.

the reason why we don't use addr_type[] is that the same code is used
on different plattforms and (at least at one point of time), using
addr_type[] didn't work there.

However, reconsidering the code right now, I guess one could change to =
code
in a way to avoid the warning. I'll put that on my ToDo list. But it is =
only
to avoid the warning, there is no real problem as said earlier.

Best regards
Michael
>=20
> --=20
> Jilles Tjoelker
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>=20

_______________________________________________
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"