https://forge.ispras.ru/https://forge.ispras.ru/favicon.ico?16490126692012-07-24T08:11:33ZOpen-Source ProjectsLinux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=122052012-07-24T08:11:33ZMarina Makienkomakienko@ispras.ru
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Open</i></li></ul> Linux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=123152012-07-25T13:24:56ZMarina Makienkomakienko@ispras.ru
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/12315/diff?detail_id=13587">diff</a>)</li></ul> Linux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=124962012-07-31T12:17:42ZMarina Makienkomakienko@ispras.ru
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul> Linux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=217462015-01-16T18:55:33ZVadim Mutilinmutilin@ispras.ru
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Open</i></li><li><strong>Assignee</strong> changed from <i>Marina Makienko</i> to <i>Vadim Mutilin</i></li></ul><p>The function ldv_check_arg is defined as<br /><pre><code class="c syntaxhl" data-language="c"> <span class="kt">void</span> <span class="nf">ldv_check_arg</span><span class="p">(</span><span class="kt">unsigned</span> <span class="kt">long</span> <span class="n">size</span><span class="p">,</span> <span class="kt">unsigned</span> <span class="kt">long</span> <span class="n">offset</span><span class="p">)</span>
<span class="p">{</span>
<span class="cm">/* LDV_COMMENT_ASSERT size should be positive. */</span>
<span class="n">ldv_assert</span><span class="p">(</span><span class="n">size</span> <span class="o">></span> <span class="mi">0</span><span class="p">);</span>
<span class="cm">/* LDV_COMMENT_ASSERT offset should not be negative. */</span>
<span class="n">ldv_assert</span><span class="p">(</span><span class="n">offset</span> <span class="o">>=</span> <span class="mi">0</span><span class="p">);</span>
<span class="p">}</span>
</code></pre></p>
<p>After applying CIF (with gcc optimizations which appeared in LDV Tools v0.7) it is printed as</p>
<pre><code class="c syntaxhl" data-language="c"><span class="kt">void</span> <span class="nf">ldv_check_arg</span><span class="p">(</span><span class="kt">long</span> <span class="kt">unsigned</span> <span class="kt">int</span> <span class="n">size</span><span class="p">,</span> <span class="kt">long</span> <span class="kt">unsigned</span> <span class="kt">int</span> <span class="n">offset</span><span class="p">)</span>
<span class="p">{</span>
<span class="k">if</span> <span class="p">(</span><span class="n">size</span> <span class="o">==</span> <span class="mi">0UL</span><span class="p">)</span>
<span class="p">{</span>
<span class="n">ldv_error</span> <span class="p">(</span> <span class="p">);</span>
<span class="p">}</span>
<span class="k">else</span>
<span class="p">(</span> <span class="kt">void</span> <span class="p">)</span> <span class="mi">0</span><span class="p">;</span>
<span class="p">(</span> <span class="kt">void</span> <span class="p">)</span> <span class="mi">0</span><span class="p">;</span>
<span class="p">}</span>
</code></pre>
<p>The rule model was prepared having BLAST in mind which does not respect unsigned integers.<br />The condition <br /><pre><code class="c syntaxhl" data-language="c"><span class="n">ldv_assert</span><span class="p">(</span><span class="n">offset</span> <span class="o">>=</span> <span class="mi">0</span><span class="p">);</span>
</code></pre><br />ensures that the user does not pass negative values.<br />In case if we respect unsigned then we should check for big numbers greater than MAX_INT instead of negatives.</p>
The effect can be seen on the drivers which changed verdict from Unsafe to Safe
<ul>
<li>drivers/iio/frequency/ad9523.ko ldv_main0_sequence_infinite_withcheck_stateful</li>
<li>drivers/net/ppp/pptp.ko ldv_main0_sequence_infinite_withcheck_stateful</li>
<li>drivers/uwb/uwb.ko ldv_main18_sequence_infinite_withcheck_stateful</li>
</ul> Linux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=218322015-01-28T13:13:49ZVadim Mutilinmutilin@ispras.ru
<ul></ul><p>Try the following models:<br />1. check that offset<=size<br />2. check only size>0 (remove the condition offset>=0 - the results should be the same as current ones since CIF already removes the condition)<br />3. replace unsigned to long in parameters of ldv_check_arg (to get the old behaviour for BLAST)</p> Linux Kernel Safety RuleDB - Feature #3261: 129: Calling find_next_zero_bit() with arguments in the right orderhttps://forge.ispras.ru/issues/3261?journal_id=230612015-05-06T11:45:29ZVadim Mutilinmutilin@ispras.ru
<ul><li><strong>Subject</strong> changed from <i>129: correct call to find_next_zero_bit() with arguments in the right order</i> to <i>129: Calling find_next_zero_bit() with arguments in the right order</i></li></ul>