Project

General

Profile

Feature #3261

129: Calling find_next_zero_bit() with arguments in the right order

Added by Marina Makienko over 6 years ago. Updated almost 4 years ago.

Status:
Open
Priority:
Normal
Assignee:
Start date:
07/23/2012
Due date:
% Done:

100%

Estimated time:
Published in build:

Description

The find_next_zero_bit() is called with the from and to arguments in the wrong order. This results in the function always returning 0, and all media devices being registered with minor 0. Furthermore, mdev->minor is then used before being assigned with the find_next_zero_bit() return value. This really makes sure we'll always use minor 0.

Commit: 6969405

History

#1 Updated by Marina Makienko over 6 years ago

  • Status changed from New to Open

#2 Updated by Marina Makienko over 6 years ago

  • Description updated (diff)

#3 Updated by Marina Makienko over 6 years ago

  • Status changed from Open to Resolved
  • % Done changed from 0 to 100

#4 Updated by Vadim Mutilin about 4 years ago

  • Status changed from Resolved to Open
  • Assignee changed from Marina Makienko to Vadim Mutilin

The function ldv_check_arg is defined as

        void ldv_check_arg(unsigned long size, unsigned long offset)
        {
                /* LDV_COMMENT_ASSERT size should be positive. */
                ldv_assert(size > 0);
                /* LDV_COMMENT_ASSERT offset should not be negative. */
                ldv_assert(offset >= 0);

        }

After applying CIF (with gcc optimizations which appeared in LDV Tools v0.7) it is printed as

void ldv_check_arg(long unsigned int size, long unsigned int offset)
{
  if (size == 0UL)
  {
    ldv_error ( );
  }
  else
    ( void ) 0;
  ( void ) 0;
}

The rule model was prepared having BLAST in mind which does not respect unsigned integers.
The condition

ldv_assert(offset >= 0);

ensures that the user does not pass negative values.
In case if we respect unsigned then we should check for big numbers greater than MAX_INT instead of negatives.

The effect can be seen on the drivers which changed verdict from Unsafe to Safe
  • drivers/iio/frequency/ad9523.ko ldv_main0_sequence_infinite_withcheck_stateful
  • drivers/net/ppp/pptp.ko ldv_main0_sequence_infinite_withcheck_stateful
  • drivers/uwb/uwb.ko ldv_main18_sequence_infinite_withcheck_stateful

#5 Updated by Vadim Mutilin about 4 years ago

Try the following models:
1. check that offset<=size
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)
3. replace unsigned to long in parameters of ldv_check_arg (to get the old behaviour for BLAST)

#6 Updated by Vadim Mutilin almost 4 years ago

  • Subject changed from 129: correct call to find_next_zero_bit() with arguments in the right order to 129: Calling find_next_zero_bit() with arguments in the right order

Also available in: Atom PDF