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  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]
Date:   Thu, 29 Dec 2016 10:04:01 -0600
From:   Grygorii Strashko <grygorii.strashko@...com>
To:     "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>,
        Mugunthan V N <mugunthanvnm@...com>,
        Sekhar Nori <nsekhar@...com>, <linux-kernel@...r.kernel.org>,
        <linux-omap@...r.kernel.org>
Subject: Re: [PATCH] net: ethernet: ti: davinci_cpdma: fix access to
 uninitialized variable in cpdma_chan_set_descs()

Hi Ivan,

On 12/28/2016 07:49 PM, Ivan Khoronzhuk wrote:
> On Wed, Dec 28, 2016 at 05:42:13PM -0600, Grygorii Strashko wrote:
>> Now below code sequence causes "Unable to handle kernel NULL pointer
>> dereference.." exception and system crash during CPSW CPDMA initialization:
>>
>> cpsw_probe
>> |-cpdma_chan_create (TX channel)
>>   |-cpdma_chan_split_pool
>>     |-cpdma_chan_set_descs(for TX channels)
>>     |-cpdma_chan_set_descs(for RX channels) [1]
>>
>> - and -
>> static void cpdma_chan_set_descs(struct cpdma_ctlr *ctlr,
>> 				 int rx, int desc_num,
>> 				 int per_ch_desc)
>> {
>> 	struct cpdma_chan *chan, *most_chan = NULL;
>>
>> ...
>>
>> 	for (i = min; i < max; i++) {
>> 		chan = ctlr->channels[i];
>> 		if (!chan)
>> 			continue;
>> ...
>>
>> 		if (most_dnum < chan->desc_num) {
>> 			most_dnum = chan->desc_num;
>> 			most_chan = chan;
>> 		}
>> 	}
>> 	/* use remains */
>> 	most_chan->desc_num += desc_cnt; [2]
>> }
>>
>> So, most_chan value will never be reassigned when cpdma_chan_set_descs() is
>> called second time [1], because there are no RX channels yet and system
>> will crash at [2].
> 
> How did you get this?
> I just remember as I fixed it before sending patchset.
> 
> Maybe it was some experiment with it.
> I just wonder and want to find actual reason what's happening.
> 
> Look bellow:
> 
> cpsw_probe
> |-cpdma_chan_create (TX channel)
>   |-cpdma_chan_split_pool
>     |-cpdma_chan_set_descs(for TX channels)
>     |-cpdma_chan_set_descs(for RX channels) [1]
> 
> |-cpdma_chan_set_descs(for RX channels) in case you'be described has to be
> called with rx_desc_num = 0, because all descs are assigned already for tx
> channel. And, if desc_num = 0, cpdma_chan_set_descs just exits and no issues.
> So, could you please explain how you get this, in what circumstances.

You are right. I've hit this issue while working on other feature which allows
to split pool between RX and TX path and as part of it cpdma_chan_set_descs()
is called with different set of arguments. I probably will just squash it in my changes or 
or send as part of my series.


-- 
regards,
-grygorii

Powered by blists - more mailing lists