linux-kernel - Re: Buggy variable-length array code...or compiler?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e9c3a7c21002261143w9a4cc29ha738e84496ef5c4c@mail.gmail.com>
Date:	Fri, 26 Feb 2010 12:43:10 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	steve@...idescorp.com
Cc:	Dan Carpenter <error27@...il.com>,
	David Rientjes <rientjes@...gle.com>,
	linux-kernel@...r.kernel.org, microblaze-uclinux@...e.uq.edu.au,
	Dave Hansen <dave@...ux.vnet.ibm.com>
Subject: Re: Buggy variable-length array code...or compiler?

On Fri, Feb 26, 2010 at 12:15 PM, Steven J. Magnani
<steve@...idescorp.com> wrote:
> On Thu, 25 Feb 2010, Steven J. Magnani wrote:
>> >
>> > > When I run a memcpy dmatest with a Microblaze 2.6.33 noMMU kernel, the
>> > > system crashes after about 400 iterations. After much head scratching, I
>> > > believe I've narrowed the problem to this fragment of code in
>> > > drivers/dma/dmatest.c:
>> > >
>> > > static int dmatest_func(void *data)
>> > > {
>> > > ...
>> > >     int                     src_cnt;
>> > >     int                     dst_cnt;
>> > > ...
>> > >     if (thread->type == DMA_MEMCPY)
>> > >             src_cnt = dst_cnt = 1;
>> > > ...
>> > >     while (!kthread_should_stop()
>> > >            && !(iterations && total_tests >= iterations)) {
>> > > ...
>> > >             dma_addr_t dma_srcs[src_cnt];
>> > >             dma_addr_t dma_dsts[dst_cnt];
>> > > ...
>
> On Thu, Feb 25, 2010 at 03:46:48PM -0800, David Rientjes wrote:
>> > This could probably become the first kernel user of the flexible array
>> > library (see Documentation/flexible-arrays.txt).  Dan?
>> > --
>
> On Fri, 2010-02-26 at 13:27 +0300, Dan Carpenter wrote:
>> I think the max that src_cnt can be is 3 and the most dst_cnt can be is 2.
>> We could just put that there.
>
> src_cnt is dependent on module parameters.
>
> The bug goes away if dma_srcs and dma_dsts are declared outside the
> loop, but that requires knocking the loop in another tabstop. At that
> point dmatest_func() would be begging for refactoring.
>

...and even if you refactored it there is no guarantee that gcc would
not re-inline the functions and put you back in the same situation.
Especially given the finding that J.A.'s simple test, where the size
of the array should have been constant, still failed.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/