linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c
@ 2003-03-04 11:12 Dawson Engler
  2003-03-04 12:24 ` Hugh Dickins
                   ` (2 more replies)
  0 siblings, 3 replies; 43+ messages in thread
From: Dawson Engler @ 2003-03-04 11:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc

Hi All,

here are a few more potential races from checking the directories:
	kernel/*.c
	mm/*.c
	net/*ipv4*.c
	arch/i386/kernel/*.c

which seem relatively more important that other directories.  [If there
are other subdirs that a worth checking let me know --- I am trying to
ensure that if we release race bugs someone will care enough to look
at them.]

The message format is a bit confusing.  It gives the places where the
checker thinks there are errors, and then a short list of examples
that made it think that an omitted lock protects the variable.

I do apologize if there are false positives --- I've gone over these,
but sometimes it's very hard to tell if state needs to be protected.

Dawson
-------------------------------------------------------
BUG? pair: lock=<struct sighand_struct.siglock>, var=<recalc_sigpending>
  z score=0.90
  singles = 1
  last = 8
  global-var
  1 error out of 11 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/kernel/ptrace.c:339:ptrace_notify: ERROR: var <recalc_sigpending> not protected by <struct sighand_struct.siglock>(pop=11, s=10) [locked=0] [seen_lock=1]


	looks like a bug:

	void ptrace_notify(int exit_code)
	{
        	/* Let the debugger run.  */
        	current->exit_code = exit_code;
        	set_current_state(TASK_STOPPED);
        	notify_parent(current, SIGCHLD);
        	schedule();
	
         	// Signals sent while we were stopped might set TIF_SIGPENDING.
        	recalc_sigpending();

	other places are carefully guard recalc_sigpending with 
	siglock:
        	spin_lock_irqsave(&current->sighand->siglock, flags);
        	current->notifier = NULL;
        	current->notifier_data = NULL;
        	recalc_sigpending();
        	spin_unlock_irqrestore(&current->sighand->siglock, flags);

  ====================================
  10 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:403:unblock_all_signals: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:unblock_all_signals:400] (pop=11, s=10)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:2183:sys_ssetmask: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_ssetmask:2178] (pop=11, s=10)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:2115:sys_sigprocmask: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_sigprocmask:2096] (pop=11, s=10)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1848:sys_rt_sigtimedwait: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_rt_sigtimedwait:1844] (pop=11, s=10)
    /u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:43:sys_sigsuspend: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:sys_sigsuspend:40] (pop=11, s=10)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1656:sigprocmask: NOTE: var <recalc_sigpending> protected by <struct sighand_struct.siglock> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sigprocmask:1640] (pop=11, s=10)
-------------------------------------------------------
BUG?  pair: lock=<socket_lock_t.slock>, var=<struct sock.backlog.tail>
  z score=2.81
  first = 38
  1 error out of 41 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/tcp.c:1580:tcp_recvmsg: ERROR: var <struct sock.backlog.tail> not protected by <socket_lock_t.slock>(pop=41, s=40) [locked=0] [seen_lock=1]

	unprotected read of backlog.tail but it's unclear if this value
	is depended on later.

  ====================================
  40 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/tcp.c:1382:tcp_data_wait: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/tcp.c:tcp_data_wait:1382] (pop=41, s=40)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/tcp.c:1668:tcp_recvmsg: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/tcp.c:tcp_recvmsg:1668] (pop=41, s=40)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/udp.c:691:udp_sendpage: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/udp.c:udp_sendpage:691] (pop=41, s=40)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_sockglue.c:688:ip_getsockopt: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_sockglue.c:ip_getsockopt:688] (pop=41, s=40)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/udp.c:627:udp_sendmsg: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/udp.c:udp_sendmsg:627] (pop=41, s=40)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_sockglue.c:642:ip_setsockopt: NOTE: var <struct sock.backlog.tail> protected by <socket_lock_t.slock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_sockglue.c:ip_setsockopt:642] (pop=41, s=40)
-------------------------------------------------------
BUG?  pair: lock=<struct sighand_struct.siglock>, var=<struct task_struct.blocked>
  z score=-0.17
  singles = 1
  first = 3
  2 errors out of 9 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:583:do_signal: ERROR: var <struct task_struct.blocked> not protected by <struct sighand_struct.siglock>(pop=9, s=7) [locked=0] [seen_lock=1]

	seems like it: other places are careful about getting
	current->blocked.  here there is no lock at all:
	
asmlinkage int
sys_sigsuspend(int history0, int history1, old_sigset_t mask)
{
...
        regs->eax = -EINTR;
        while (1) {
                current->state = TASK_INTERRUPTIBLE;
                schedule();
                if (do_signal(regs, &saveset))
                        return -EINTR;

	...

// do_signal:

        if (!oldset)
                oldset = &current->blocked;

  ====================================
  7 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1686:sys_rt_sigprocmask: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_rt_sigprocmask:1685] (pop=9, s=7)

                spin_lock_irq(&current->sighand->siglock);
                old_set = current->blocked;
                spin_unlock_irq(&current->sighand->siglock)

    /u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:70:sys_rt_sigsuspend: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:sys_rt_sigsuspend:69] (pop=9, s=7)
    /u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:41:sys_sigsuspend: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/arch/i386/kernel/signal.c:sys_sigsuspend:40] (pop=9, s=7)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1641:sigprocmask: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sigprocmask:1640] (pop=9, s=7)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1846:sys_rt_sigtimedwait: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_rt_sigtimedwait:1844] (pop=9, s=7)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1836:sys_rt_sigtimedwait: NOTE: var <struct task_struct.blocked> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:sys_rt_sigtimedwait:1824] (pop=9, s=7)
-------------------------------------------------------
BUG? pair: lock=<struct sighand_struct.siglock>, var=<struct task_struct.signal.group_stop_count>
  z score=-0.57
  singles = 1
  2 errors out of 7 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1483:get_signal_to_deliver: ERROR: var <struct task_struct.signal.group_stop_count> not protected by <struct sighand_struct.siglock>(pop=7, s=5) [locked=0] [seen_lock=1]

   seems like a race, since someone else could have slipped in and
   decremented group_stop_count after the check and before the
   acquisition:

                        if (current->signal->group_stop_count > 0) {
                                spin_lock_irq(&current->sighand->siglock);
                                --current->signal->group_stop_count;
                                spin_unlock_irq(&current->sighand->siglock
  ====================================
  5 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1485:get_signal_to_deliver: NOTE: var <struct task_struct.signal.group_stop_count> protected by <struct sighand_struct.siglock> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:get_signal_to_deliver:1484] (pop=7, s=5)

                        if (current->signal->group_stop_count > 0) {
                                spin_lock_irq(&current->sighand->siglock);
                                --current->signal->group_stop_count;
                                spin_unlock_irq(&current->sighand->siglock

    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:207:recalc_sigpending_tsk: NOTE: var <struct task_struct.signal.group_stop_count> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exit.c:exit_notify:624
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exit.c:exit_notify:631
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exit.c:exit_notify:627
	   ->end=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:recalc_sigpending_tsk:207] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/fork.c:958:copy_process: NOTE: var <struct task_struct.signal.group_stop_count> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/fork.c:copy_process:944] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:1461:get_signal_to_deliver: NOTE: var <struct task_struct.signal.group_stop_count> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/signal.c:get_signal_to_deliver:1447] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/fork.c:964:copy_process: NOTE: var <struct task_struct.signal.group_stop_count> protected by <struct sighand_struct.siglock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/fork.c:copy_process:944] (pop=7, s=5)
-------------------------------------------------------
BUG? pair: lock=<&cpufreq_driver_sem:struct semaphore:1>, var=<cpufreq_driver,struct cpufreq_driver,1>
  z score=1.11
  first = 11
  global-lock
  global-var
  1 error out of 13 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:1221:cpufreq_register_driver: ERROR: var <cpufreq_driver,struct cpufreq_driver,1> not protected by <&cpufreq_driver_sem:struct semaphore:1>(pop=13, s=12) [locked=0] [seen_lock=1]

   seems like a bug --- the exmaples strongly suggest someone can make the
   driver null, in which case i believe the following is open to a race.

        if (cpufreq_driver)
                return -EBUSY;

        if (!driver_data || !driver_data->verify ||
            ((!driver_data->setpolicy) && (!driver_data->target)))
                return -EINVAL;

        down(&cpufreq_driver_sem);


  ====================================
  12 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:331:cpufreq_add_dev: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:cpufreq_add_dev:330] (pop=13, s=12)

        down(&cpufreq_driver_sem);
        if (!cpufreq_driver) {
                up(&cpufreq_driver_sem);
                return -EINVAL;
        }


    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:309:show_cpuinfo_min_freq: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:show_cpuinfo_min_freq:309] (pop=13, s=12)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:193:show_scaling_governor: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:show_scaling_governor:192] (pop=13, s=12)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:1271:cpufreq_unregister_driver: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:cpufreq_unregister_driver:1269] (pop=13, s=12)

        down(&cpufreq_driver_sem);

        if (!cpufreq_driver ||
            ((driver != cpufreq_driver) 

    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:1062:cpufreq_set_policy: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:cpufreq_set_policy:1061] (pop=13, s=12)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:85:cpufreq_parse_governor: NOTE: var <cpufreq_driver,struct cpufreq_driver,1> protected by <&cpufreq_driver_sem:struct semaphore:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/cpufreq.c:cpufreq_parse_governor:84] (pop=13, s=12)
-------------------------------------------------------
BUG? pair: lock=<struct in_device.lock>, var=<struct in_device.mc_list>
  z score=-1.00
  singles = 1
  first = 4
  last = 1
  3 errors out of 9 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:497:ip_mc_inc_group: ERROR: var <struct in_device.mc_list> not protected by <struct in_device.lock>(pop=9, s=6) [locked=0] [seen_lock=1]

	seems like they should have a read lock.


     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:544:ip_mc_dec_group: ERROR: var <struct in_device.mc_list> not protected by <struct in_device.lock>(pop=9, s=6) [locked=0] [seen_lock=1]

	read_lock?

     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:571:ip_mc_down: ERROR: var <struct in_device.mc_list> not protected by <struct in_device.lock>(pop=9, s=6) [locked=0] [seen_lock=1]
  ====================================
  6 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:525:ip_mc_inc_group: NOTE: var <struct in_device.mc_list> protected by <struct in_device.lock> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:ip_mc_inc_group:523] (pop=9, s=6)

        write_lock_bh(&in_dev->lock);
        im->next=in_dev->mc_list;
        in_dev->mc_list=im;
        write_unlock_bh(&in_dev->lock);


    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:602:ip_mc_destroy_dev: NOTE: var <struct in_device.mc_list> protected by <struct in_device.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:ip_mc_destroy_dev:601] (pop=9, s=6)

        write_lock_bh(&in_dev->lock);
        while ((i = in_dev->mc_list) != NULL) {


    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:773:ip_check_mc: NOTE: var <struct in_device.mc_list> protected by <struct in_device.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/igmp.c:ip_check_mc:772] (pop=9, s=6)


        read_lock(&in_dev->lock);
        for (im=in_dev->mc_list; im; im=im->next) {
                if (im->multiaddr == mc_addr) {
                        read_unlock(&in_dev->lock);

-------------------------------------------------------
BUG? pair: lock=<&fib_rules_lock:rwlock_t:1>, var=<struct fib_rule.r_ifindex>
  z score=-1.12
  singles = 1
  last = 1
  global-lock
  2 errors out of 5 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:289:fib_rules_detach: ERROR: var <struct fib_rule.r_ifindex> not protected by <&fib_rules_lock:rwlock_t:1>(pop=5, s=3) [locked=0] [seen_lock=1]

   this seems pretty busted: they check r->r_ifindex and then 
   write to it without rechecking that it's the same value:

        for (r=fib_rules; r; r=r->r_next) {
                if (r->r_ifindex == dev->ifindex) {
                        write_lock_bh(&fib_rules_lock);
                        r->r_ifindex = -1;
                        write_unlock_bh(&fib_rules_lock);
                }
        }
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:302:fib_rules_attach: ERROR: var <struct fib_rule.r_ifindex> not protected by <&fib_rules_lock:rwlock_t:1>(pop=5, s=3) [locked=0] [seen_lock=1]

     same: check, and a write without rechecking:

        for (r=fib_rules; r; r=r->r_next) {
                if (r->r_ifindex == -1 && strcmp(dev->name, r->r_ifname) == 0) {
                        write_lock_bh(&fib_rules_lock);
                        r->r_ifindex = dev->ifindex;
                        write_unlock_bh(&fib_rules_lock);

  ====================================
  3 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:291:fib_rules_detach: NOTE: var <struct fib_rule.r_ifindex> protected by <&fib_rules_lock:rwlock_t:1> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:fib_rules_detach:290] (pop=5, s=3)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:304:fib_rules_attach: NOTE: var <struct fib_rule.r_ifindex> protected by <&fib_rules_lock:rwlock_t:1> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:fib_rules_attach:303] (pop=5, s=3)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:323:fib_lookup: NOTE: var <struct fib_rule.r_ifindex> protected by <&fib_rules_lock:rwlock_t:1> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:fib_lookup:321] (pop=5, s=3)
-------------------------------------------------------
BUG? pair: lock=<&fib_rules_lock:rwlock_t:1>, var=<*rp,struct fib_rule,0>
  z score=-1.50
  singles = 1
  global-lock
  2 errors out of 4 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:111:inet_rtm_delrule: ERROR: var <*rp,struct fib_rule,0> not protected by <&fib_rules_lock:rwlock_t:1>(pop=4, s=2) [locked=0] [seen_lock=1]
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:234:inet_rtm_newrule: ERROR: var <*rp,struct fib_rule,0> not protected by <&fib_rules_lock:rwlock_t:1>(pop=4, s=2) [locked=0] [seen_lock=1]

   sure seems like the code is depending on state from outside the
   critical section:

        while ( (r = *rp) != NULL ) {
                if (r->r_preference > new_r->r_preference)
                        break;
                rp = &r->r_next;
        }

        new_r->r_next = r;
        atomic_inc(&new_r->r_clntref);
        write_lock_bh(&fib_rules_lock);
        *rp = new_r;
        write_unlock_bh(&fib_rules_lock);

  ====================================
  2 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:243:inet_rtm_newrule: NOTE: var <*rp,struct fib_rule,0> protected by <&fib_rules_lock:rwlock_t:1> [annot=single] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:inet_rtm_newrule:242] (pop=4, s=2)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:129:inet_rtm_delrule: NOTE: var <*rp,struct fib_rule,0> protected by <&fib_rules_lock:rwlock_t:1> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/fib_rules.c:inet_rtm_delrule:128] (pop=4, s=2)

                        write_lock_bh(&fib_rules_lock);
                        *rp = r->r_next;
                        r->r_dead = 1;
                        write_unlock_bh(&fib_rules_lock);

-------------------------------------------------------
MINOR: not false, just dumb pair: lock=<&exec_domains_lock:rwlock_t:1>, var=<exec_domains,struct exec_domain,1>
  z score=0.20
  first = 5
  global-lock
  global-var
  1 error out of 6 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:143:unregister_exec_domain: ERROR: var <exec_domains,struct exec_domain,1> not protected by <&exec_domains_lock:rwlock_t:1>(pop=6, s=5) [locked=0] [seen_lock=1]

        epp = &exec_domains;
        write_lock(&exec_domains_lock);
        for (epp = &exec_domains; *epp; epp = &(*epp)->next) {

  ====================================
  5 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:98:lookup_exec_domain: NOTE: var <exec_domains,struct exec_domain,1> protected by <&exec_domains_lock:rwlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:lookup_exec_domain:96] (pop=6, s=5)

        read_lock(&exec_domains_lock);

        for (ep = exec_domains; ep; ep = ep->next) {
                if (pers >= ep->pers_low && pers <= ep->pers_high)
                        if (try_module_get(ep->module))
                                goto out;

    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:208:get_exec_domain_list: NOTE: var <exec_domains,struct exec_domain,1> protected by <&exec_domains_lock:rwlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:get_exec_domain_list:207] (pop=6, s=5)

        read_lock(&exec_domains_lock);
        for (ep = exec_domains; ep && len < PAGE_SIZE - 80; ep = ep->next)
                len += sprintf(page + len, "%d-%d\t%-16s\t[%s]\n",
                               ep->pers_low, ep->pers_high, ep->name,

    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:145:unregister_exec_domain: NOTE: var <exec_domains,struct exec_domain,1> protected by <&exec_domains_lock:rwlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:unregister_exec_domain:144] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:83:lookup_exec_domain: NOTE: var <exec_domains,struct exec_domain,1> protected by <&exec_domains_lock:rwlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:lookup_exec_domain:82] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:124:register_exec_domain: NOTE: var <exec_domains,struct exec_domain,1> protected by <&exec_domains_lock:rwlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/exec_domain.c:register_exec_domain:123] (pop=6, s=5)
-------------------------------------------------------
BUG? pair: lock=<&logbuf_lock:spinlock_t:1>, var=<log_start,unsigned int,1>
  z score=-0.57
  first = 3
  last = 1
  global-lock
  global-var
  2 errors out of 7 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:179:do_syslog: ERROR: var <log_start,unsigned int,1> not protected by <&logbuf_lock:spinlock_t:1>(pop=7, s=5) [locked=0] [seen_lock=1]

	supposed to protect log start?

  ====================================
  5 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:625:register_console: NOTE: var <log_start,unsigned int,1> protected by <&logbuf_lock:spinlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:register_console:624] (pop=7, s=5)

                spin_lock_irqsave(&logbuf_lock, flags);
                con_start = log_start;
                spin_unlock_irqrestore(&logbuf_lock, flags);

    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:184:do_syslog: NOTE: var <log_start,unsigned int,1> protected by <&logbuf_lock:spinlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:do_syslog:183] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:498:release_console_sem: NOTE: var <log_start,unsigned int,1> protected by <&logbuf_lock:spinlock_t:1> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:release_console_sem:497] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:186:do_syslog: NOTE: var <log_start,unsigned int,1> protected by <&logbuf_lock:spinlock_t:1> [annot=last] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:do_syslog:183] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:365:emit_log_char: NOTE: var <log_start,unsigned int,1> protected by <&logbuf_lock:spinlock_t:1> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:printk:403
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:printk:423
	   ->end=/u2/engler/mc/oses/linux/linux-2.5.62/kernel/printk.c:emit_log_char:365] (pop=7, s=5)
-------------------------------------------------------
BUG? pair: lock=<struct shmem_inode_info.lock>, var=<struct shmem_inode_info.next_index>
  z score=0.20
  first = 2
  1 error out of 6 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:385:shmem_truncate: ERROR: var <struct shmem_inode_info.next_index> not protected by <struct shmem_inode_info.lock>(pop=6, s=5) [locked=0] [seen_lock=1]

   seems like a potential race, since info_next_index seems like it could
   get decreased in the meantime violating the if-stmt:

        if (idx >= info->next_index)
                return;

        spin_lock(&info->lock);
        limit = info->next_index;


  ====================================
  5 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:389:shmem_truncate: NOTE: var <struct shmem_inode_info.next_index> protected by <struct shmem_inode_info.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_truncate:388] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:582:shmem_unuse_inode: NOTE: var <struct shmem_inode_info.next_index> protected by <struct shmem_inode_info.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_unuse_inode:581] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:482:shmem_truncate: NOTE: var <struct shmem_inode_info.next_index> protected by <struct shmem_inode_info.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_truncate:388] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:335:shmem_swp_alloc: NOTE: var <struct shmem_inode_info.next_index> protected by <struct shmem_inode_info.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_swp_alloc:324] (pop=6, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:343:shmem_swp_alloc: NOTE: var <struct shmem_inode_info.next_index> protected by <struct shmem_inode_info.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_getpage:755
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_getpage:757
	   ->end=/u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:shmem_swp_alloc:343] (pop=6, s=5)
-------------------------------------------------------
BUG: pair: lock=<struct ipq.lock>, var=<struct ipq.last_in>
  z score=-0.57
  first = 2
  2 errors out of 7 uses:
     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:163:ip_frag_destroy: ERROR: var <struct ipq.last_in> not protected by <struct ipq.lock>(pop=7, s=5) [locked=0] [seen_lock=1]

	BUG_ON call that looks at last_in

     /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:289:ip_frag_intern: ERROR: var <struct ipq.last_in> not protected by <struct ipq.lock>(pop=7, s=5) [locked=0] [seen_lock=1]

	seems like a definite bug they read-modify-write it right after
	releasing the lock:

                        write_unlock(&ipfrag_lock);
                        qp_in->last_in |= COMPLETE;
                        ipq_put(qp_in);

  ====================================
  5 examples:
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:226:ip_evictor: NOTE: var <struct ipq.last_in> protected by <struct ipq.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_evictor:225] (pop=7, s=5)

                                spin_lock(&qp->lock);
                                if (!(qp->last_in&COMPLETE))
                                        ipq_kill(qp);
                                spin_unlock(&qp->lock);

    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:249:ip_expire: NOTE: var <struct ipq.last_in> protected by <struct ipq.lock> [annot=first] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_expire:247] (pop=7, s=5)

        spin_lock(&qp->lock);

        if (qp->last_in & COMPLETE)
                goto out;

    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:619:ip_defrag: NOTE: var <struct ipq.last_in> protected by <struct ipq.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:615] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:377:ip_frag_queue: NOTE: var <struct ipq.last_in> protected by <struct ipq.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:615
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:617
	   ->end=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_frag_queue:377] (pop=7, s=5)
    /u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:193:ipq_kill: NOTE: var <struct ipq.last_in> protected by <struct ipq.lock> [annot=none] [level=0] [path=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:615
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:623
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:623
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_defrag:621
	   ->/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ip_frag_reasm:515
	   ->end=/u2/engler/mc/oses/linux/linux-2.5.62/net/ipv4/ip_fragment.c:ipq_kill:193] (pop=7, s=5)
-------------------------------------------------------

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c
  2003-03-04 11:12 [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c Dawson Engler
@ 2003-03-04 12:24 ` Hugh Dickins
  2003-03-04 13:23 ` Martin Josefsson
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
  2 siblings, 0 replies; 43+ messages in thread
From: Hugh Dickins @ 2003-03-04 12:24 UTC (permalink / raw)
  To: Dawson Engler; +Cc: linux-kernel, mc

On Tue, 4 Mar 2003, Dawson Engler wrote:
> 
> BUG? pair: lock=<struct shmem_inode_info.lock>, var=<struct shmem_inode_info.next_index>
>   z score=0.20
>   first = 2
>   1 error out of 6 uses:
>      /u2/engler/mc/oses/linux/linux-2.5.62/mm/shmem.c:385:shmem_truncate: ERROR: var <struct shmem_inode_info.next_index> not protected by <struct shmem_inode_info.lock>(pop=6, s=5) [locked=0] [seen_lock=1]
> 
>    seems like a potential race, since info_next_index seems like it could
>    get decreased in the meantime violating the if-stmt:
> 
>         if (idx >= info->next_index)
>                 return;
> 
>         spin_lock(&info->lock);
>         limit = info->next_index;

Thanks for the report, but this one is okay as is.

info->next_index is only ever _decreased_ here in this shmem_truncate
function, which is always entered under the protection of inode->i_sem
(which is guarding corresponding changes to inode->i_size).  As you've
noticed, it's okay if next_index increases before spin lock is taken.

Hugh


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c
  2003-03-04 11:12 [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c Dawson Engler
  2003-03-04 12:24 ` Hugh Dickins
@ 2003-03-04 13:23 ` Martin Josefsson
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
  2 siblings, 0 replies; 43+ messages in thread
From: Martin Josefsson @ 2003-03-04 13:23 UTC (permalink / raw)
  To: Dawson Engler; +Cc: linux-kernel, mc

On Tue, 2003-03-04 at 12:12, Dawson Engler wrote:
> Hi All,
> 
> here are a few more potential races from checking the directories:
> 	kernel/*.c
> 	mm/*.c
> 	net/*ipv4*.c
> 	arch/i386/kernel/*.c
> 
> which seem relatively more important that other directories.  [If there
> are other subdirs that a worth checking let me know --- I am trying to
> ensure that if we release race bugs someone will care enough to look
> at them.]

It would be great if you could check net/{ipv4,ipv6}/netfilter/*.c and
send the results to lkml and netfilter-devel@lists.netfilter.org

Thanks in advance.

-- 
/Martin

Never argue with an idiot. They drag you down to their level, then beat you with experience.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] potential dereference of user pointer errors
  2003-03-04 11:12 [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c Dawson Engler
  2003-03-04 12:24 ` Hugh Dickins
  2003-03-04 13:23 ` Martin Josefsson
@ 2003-03-21  6:33 ` Junfeng Yang
  2003-03-21 21:44   ` Chris Wright
                     ` (6 more replies)
  2 siblings, 7 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-03-21  6:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc

Hi,

We are the the Stanford Checker team that constantly send error reports to
the linux kernel mailing list. Enclosed are 10 dereference of user pointer
warnings catched by our checker. We started by marking the second
parameter of copy_from_user (to, from, len), the first parameter of
copy_to_user (to, from, len), and all parameters of the sys_* functions as
tainted, then propogated the tainted annotations along call chains. If two
functions get assigned to the same structure field, we propogate the
tainted annotations between them, too. An example of such propogation is
the warning about drivers/media/video/cpia.c::cpia_write_proc and
drivers/media/usb/media/vicam.c. The error message uses "thru
struct_name:field_name" to represent such propogations.

We are not sure if these warnings are real bugs or false positives, so any
confirmation or clarification will be greatly appreciated.

Question: can the write function to /proc file system takes tainted
inputs? Originally we think so but we've found some number of write_proc
functions in different driver modules that dereference the inputs
directly, so it is likey that we are missing something.

If any of the error message is not clear, please email me.

Thanks.

[BUG] not sure. the dereference occurs at a tainted place. call
copy_from_user (, data, ), then copy_from_user (, *data, )

/home/junfeng/linux-2.5.63/sound/core/seq/instr/ainstr_iw.c:92:snd_seq_iwffff_copy_env_from_stream:
ERROR:TAINTED deferencing "data" tainted by [dist=0][copy_from_user:parm1]

[BUG] cmd is tainted. then they do copy_to_user (cmd->resbuf, ...) resbuf
is declared as a pointer, not an array, so cmd->resbuf can point to
anywhere.

/home/junfeng/linux-2.5.63/drivers/message/i2o/i2o_config.c:440:ioctl_parms:
ERROR:TAINTED deferencing "cmd" tainted by [dist=0][(null)]

[BUG] cosa_getidstr calls copy_to_user on arg, which implies arg is
tainted.

/home/junfeng/linux-2.5.63/drivers/net/wan/cosa.c:1109:cosa_readmem:
ERROR:TAINTED deferencing "d" tainted by [dist=2][called by
cosa_ioctl_common:parm3 calling cosa_getidstr:parm1 calling
copy_to_user:parm0]

[MINOR] in debug data

/home/junfeng/linux-2.5.63/drivers/usb/serial/kobil_sct.c:429:kobil_write:
ERROR:TAINTED deferencing "buf" tainted by [dist=0][copy_from_user:parm1]

[UNKNOWN] sys_quotaoctl is a real system call. The entry point is at
entry.S. But this file is under subdir fs. So there must be something we
are missing

/home/junfeng/linux-2.5.63/fs/block_dev.c:817:lookup_bdev: ERROR:TAINTED
deferencing "path" tainted by [dist=1][called by
/home/junfeng/linux-2.5.63/fs/quota.c:sys_quotactl:parm1]

[UNKNOWN] the warning itself is not a bug. cpia.c::cpia_write_proc calls
copy_from_user, which implies it can take tainted inputs.
vicam_write_proc_gain and cpia_write_proc are both assigned to
proc_dir_entry.write_proc. So either cpia_write_proc doesn't need to do
copy_from_user, or vicam_write_proc misses the copy_from_user
Should I assume that write to /proc file system takes tainted inputs or
not? or it depends?

/home/junfeng/linux-2.5.63/lib/vsprintf.c:64:simple_strtol: ERROR:TAINTED
deferencing "cp" tainted by [dist=3][ calling simple_strtoul  called by
/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:vicam_write_proc_gain:parm1
thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1]

similar things

/home/junfeng/linux-2.5.63/drivers/isdn/hardware/eicon/divasproc.c:info_write
thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc

[BUG] the write function can take a tainted pointer as input. in the same
sub direction, scanner.c::write_scanner does copy_from_user (should fix
the function pointer propogation thing, try to get the 'nearest' one.)

/home/junfeng/linux-2.5.63/drivers/usb/image/mdc800.c:794:mdc800_device_write:
ERROR:TAINTED deferencing "buf" tainted by [dist=1][thru
file_operations:write
/home/junfeng/linux-2.5.63/sound/oss/es1370.c:es1370_write parm1  calling
copy_from_user:parm1]

[BUG] under the same subdir, sisfb_ioctl and matroxfb_dh_ioctl assigned to
the same struct field fb_ops::fb_ioctl. matroxfb_dh_ioctl does a
copy_to_user on the param @arg, which implies it is tainted. sisfb_ioctl
calls sis_malloc and sis_malloc then deref's arg. actually, sisfb_ioctl
assumes arg is untainited

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:1817:sis_malloc:
ERROR:TAINTED deferencing "req" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:259:sis_get_glyph:
ERROR:TAINTED deferencing "gly" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:276:sis_dispinfo:
ERROR:TAINTED deferencing "rec" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]

[BUG] guswave_ioctl does a copy_to_user on the same case branch
SNDCTL_SYNTH_INFO
/home/junfeng/linux-2.5.63/sound/oss/awe_wave.c:2049:awe_ioctl:
ERROR:TAINTED passing "arg" into deref cal __constant_memcpy [dist=1][thru
synth_operations:ioctl
/home/junfeng/linux-2.5.63/sound/oss/gus_wave.c:guswave_ioctl:parm2
copy_to_user:parm0]

[BUG] maybe. snd_cmipci_ac3_copy does copy_from_user implies src is
tainted. The only suspicious point is that in snd_cmipci_ac3_copy the
tainted parm is called src, while in snd_ex1938_capture_copy it is called
dst.

/home/junfeng/linux-2.5.63/sound/pci/es1938.c:833:snd_es1938_capture_copy:
ERROR:TAINTED passing "dst" into deref cal __constant_memcpy [dist=1][thru
snd_pcm_ops_t:copy
/home/junfeng/linux-2.5.63/sound/pci/cmipci.c:snd_cmipci_ac3_copy:parm3
copy_from_user:parm1]



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
@ 2003-03-21 21:44   ` Chris Wright
  2003-03-21 21:58     ` Junfeng Yang
  2003-03-21 22:08     ` Junfeng Yang
  2003-03-21 22:15   ` Chris Wright
                     ` (5 subsequent siblings)
  6 siblings, 2 replies; 43+ messages in thread
From: Chris Wright @ 2003-03-21 21:44 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc, perex

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> [BUG] not sure. the dereference occurs at a tainted place. call
> copy_from_user (, data, ), then copy_from_user (, *data, )
> 
> /home/junfeng/linux-2.5.63/sound/core/seq/instr/ainstr_iw.c:92:snd_seq_iwffff_copy_env_from_stream:
> ERROR:TAINTED deferencing "data" tainted by [dist=0][copy_from_user:parm1]

seems like a bug, although i think it's the first copy_from_user that
would be broken.

Looks like call flow could be something like:
Store user buf ptr in event.data.ext.ptr.
Call snd_seq_iwffff_put with the event.data.ext.ptr (offset by header bits)
Call snd_seq_iwffff_copy_env_from_stream with &event.data.ext.ptr (offset a
little further).

Finally, call copy_from_user on &event.data.ext.ptr, which is copying
into an stype.  This looks like it's the bug.   I don't think the 32 bit
addr should be copied into the stype, rather the 1st 32bits of the data
(IOW copy_from_user(stype, *data,...)).  The second copy_from_user (with
*data) copies in the whole iwffff_env_record_t struct which begins with
an stype, so it really looks as if the first copy is broken. Patch
below.   Jaroslav?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== instr/ainstr_iw.c 1.3 vs edited =====
--- 1.3/sound/core/seq/instr/ainstr_iw.c	Mon Feb 10 02:39:27 2003
+++ edited/instr/ainstr_iw.c	Fri Mar 21 13:01:32 2003
@@ -78,7 +78,7 @@
 	while (1) {
 		if (*len < (long)sizeof(__u32))
 			return -EINVAL;
-		if (copy_from_user(&stype, data, sizeof(stype)))
+		if (copy_from_user(&stype, *data, sizeof(stype)))
 			return -EFAULT;
 		if (stype == IWFFFF_STRU_WAVE)
 			return 0;

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21 21:44   ` Chris Wright
@ 2003-03-21 21:58     ` Junfeng Yang
  2003-03-21 22:06       ` Chris Wright
  2003-03-21 22:08     ` Junfeng Yang
  1 sibling, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-03-21 21:58 UTC (permalink / raw)
  To: Chris Wright; +Cc: linux-kernel, mc, perex


Thanks a lot for your confirmation! So actually the first copy_from_user
is meant to copy in what "*data" points to, not "data".

-Junfeng

On Fri, 21 Mar 2003, Chris Wright wrote:

> * Junfeng Yang (yjf@stanford.edu) wrote:
> >
> > [BUG] not sure. the dereference occurs at a tainted place. call
> > copy_from_user (, data, ), then copy_from_user (, *data, )
> >
> > /home/junfeng/linux-2.5.63/sound/core/seq/instr/ainstr_iw.c:92:snd_seq_iwffff_copy_env_from_stream:
> > ERROR:TAINTED deferencing "data" tainted by [dist=0][copy_from_user:parm1]
>
> seems like a bug, although i think it's the first copy_from_user that
> would be broken.
>
> Looks like call flow could be something like:
> Store user buf ptr in event.data.ext.ptr.
> Call snd_seq_iwffff_put with the event.data.ext.ptr (offset by header bits)
> Call snd_seq_iwffff_copy_env_from_stream with &event.data.ext.ptr (offset a
> little further).
>
> Finally, call copy_from_user on &event.data.ext.ptr, which is copying
> into an stype.  This looks like it's the bug.   I don't think the 32 bit
> addr should be copied into the stype, rather the 1st 32bits of the data
> (IOW copy_from_user(stype, *data,...)).  The second copy_from_user (with
> *data) copies in the whole iwffff_env_record_t struct which begins with
> an stype, so it really looks as if the first copy is broken. Patch
> below.   Jaroslav?
>
> thanks,
> -chris
> --
> Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net
>
> ===== instr/ainstr_iw.c 1.3 vs edited =====
> --- 1.3/sound/core/seq/instr/ainstr_iw.c	Mon Feb 10 02:39:27 2003
> +++ edited/instr/ainstr_iw.c	Fri Mar 21 13:01:32 2003
> @@ -78,7 +78,7 @@
>  	while (1) {
>  		if (*len < (long)sizeof(__u32))
>  			return -EINVAL;
> -		if (copy_from_user(&stype, data, sizeof(stype)))
> +		if (copy_from_user(&stype, *data, sizeof(stype)))
>  			return -EFAULT;
>  		if (stype == IWFFFF_STRU_WAVE)
>  			return 0;
>


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21 21:58     ` Junfeng Yang
@ 2003-03-21 22:06       ` Chris Wright
  0 siblings, 0 replies; 43+ messages in thread
From: Chris Wright @ 2003-03-21 22:06 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc, perex

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> Thanks a lot for your confirmation! So actually the first copy_from_user
> is meant to copy in what "*data" points to, not "data".

That's how I read it, but I can't claim to know this code.  Sorry, I mistyped
Jaroslav's address on the original email (fixed now), I expect he's the one
to ask.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] potential dereference of user pointer errors
  2003-03-21 21:44   ` Chris Wright
  2003-03-21 21:58     ` Junfeng Yang
@ 2003-03-21 22:08     ` Junfeng Yang
  1 sibling, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-03-21 22:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc


Hi,

This is a resend. I've made the error logs prettier.

Here is a summary of where the bugs occur.

	fs/block_dev.c
	drivers/usb/image/mdc800.c
	drivers/message/i2o/i2o_config.c
	drivers/net/wan/cosa.c
	drivers/sound/pci/es1938.c
	drivers/sound/oss/awe_wave.c
	drivers/usb/media/vicam.c
	drivers/usb/serial/kobil_sct.c
	drivers/video/sis/sis_main.c

As usual, any response will be greatly appreciated.

Question: 1. can the proc_dir_entry.write_proc take tainted inputs?
	  2. do all the _ioctl functions take tainted inputs?

------------------------------------------------------------------------
[UNKNOWN] sys_quotaoctl is a system call. It called lookup_bdev on the
param @special. But this file is under subdir fs. So there must be
something we are missing

/home/junfeng/linux-2.5.63/fs/block_dev.c:817:lookup_bdev:
ERROR:TAINTED:817:817:deferencing "path" tainted by [dist=1][called by
/home/junfeng/linux-2.5.63/fs/quota.c:sys_quotactl:parm1]

	struct block_device *bdev;
	struct inode *inode;
	struct nameidata nd;
	int error;


Error --->
	if (!path || !*path)
		return ERR_PTR(-EINVAL);

	error = path_lookup(path, LOOKUP_FOLLOW, &nd);
---------------------------------------------------------
[BUG] the write function can take tainted inputs. In the same sub dir,
scanner.c::write_scanner does copy_from_user

/home/junfeng/linux-2.5.63/drivers/usb/image/mdc800.c:794:mdc800_device_write:
ERROR:TAINTED:794:794:deferencing "buf" tainted by [dist=1][thru
file_operations:write
/home/junfeng/linux-2.5.63/sound/oss/es1370.c:es1370_write parm1  calling
copy_from_user:parm1]

			up (&mdc800->io_lock);
			return -EINTR;
		}

		/* check for command start */

Error --->
		if (buf [i] == (char) 0x55)
		{
			mdc800->in_count=0;
			mdc800->out_count=0;
---------------------------------------------------------
[BUG] copy_from_user (_, cmd, _) then copy_to_user (cmd->resbuf, _, _)

/home/junfeng/linux-2.5.63/drivers/message/i2o/i2o_config.c:440:ioctl_parms:
ERROR:TAINTED:408:440:deferencing "cmd" tainted by [dist=0][(null)]


	u32 i2o_cmd = (type == I2OPARMGET ?
				I2O_CMD_UTIL_PARAMS_GET :
				I2O_CMD_UTIL_PARAMS_SET);

Start --->
	if(copy_from_user(&kcmd, cmd, sizeof(struct i2o_cmd_psetget)))

	... DELETED 43 lines ...

	}

	put_user(len, kcmd.reslen);
	if(len > reslen)
		ret = -ENOBUFS;
Error --->
	else if(copy_to_user(cmd->resbuf, res, len))
		ret = -EFAULT;

	kfree(res);
---------------------------------------------------------
[BUG] cosa_readmem and cosa_getidstr are both called by cosa_ioctl_common

/home/junfeng/linux-2.5.63/drivers/net/wan/cosa.c:1109:cosa_readmem:
ERROR:TAINTED:1152:1109:deferencing "d" tainted by [dist=2][called by
cosa_ioctl_common:parm3 calling cosa_getidstr:parm1 calling
copy_to_user:parm0]

		return -EFAULT;

	/* If something fails, force the user to reset the card */
	cosa->firmware_status &= ~COSA_FW_RESET;

Error --->
	if ((i=readmem(cosa, d->code, len, addr)) < 0) {

	... DELETED 37 lines ...


/* Buffer of size at least COSA_MAX_ID_STRING is expected */
static inline int cosa_getidstr(struct cosa_data *cosa, char *string)
{
	int l = strlen(cosa->id_string)+1;
Start --->
	if (copy_to_user(string, cosa->id_string, l))
		return -EFAULT;
	return l;
}
---------------------------------------------------------
[BUG] snd_cmipci_ac3_copy does copy_from_user, but snd_es1938_capture_copy
doesn't

/home/junfeng/linux-2.5.63/sound/pci/es1938.c:833:snd_es1938_capture_copy:
ERROR:TAINTED:993:833:passing "dst" into deref cal __constant_memcpy
[dist=1][thru snd_pcm_ops_t:copy
/home/junfeng/linux-2.5.63/sound/pci/cmipci.c:snd_cmipci_ac3_copy:parm3
copy_from_user:parm1]

	es1938_t *chip = snd_pcm_substream_chip(substream);
	pos <<= chip->dma1_shift;
	count <<= chip->dma1_shift;
	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
	if (pos + count < chip->dma1_size)
Error --->
		memcpy(dst, runtime->dma_area + pos + 1, count);

	... DELETED 154 lines ...

	.hw_params =	snd_es1938_pcm_hw_params,
	.hw_free =	snd_es1938_pcm_hw_free,
	.prepare =	snd_es1938_capture_prepare,
	.trigger =	snd_es1938_capture_trigger,
	.pointer =	snd_es1938_capture_pointer,
Start --->
	.copy =		snd_es1938_capture_copy,
};

static void snd_es1938_free_pcm(snd_pcm_t *pcm)
---------------------------------------------------------
[BUG] matroxfb_dh_ioctl does copy_from_user but sisfb_ioctl doesn't

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:1817:sis_malloc:
ERROR:TAINTED:2572:1817:deferencing "req" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]


void sis_malloc(struct sis_memreq *req)
{
	SIS_OH *poh;

Error --->
	poh = sisfb_poh_allocate(req->size);

	... DELETED 749 lines ...

	.fb_set_cmap	= sisfb_set_cmap,
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,5,23)
        .fb_setcolreg	= sisfb_setcolreg,
        .fb_blank	= sisfb_blank,
#endif
Start --->
	.fb_ioctl	= sisfb_ioctl,
	.fb_mmap	= sisfb_mmap,
};

similar things

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:1817:sis_malloc:
ERROR:TAINTED deferencing "req" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:259:sis_get_glyph:
ERROR:TAINTED deferencing "gly" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]

/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:276:sis_dispinfo:
ERROR:TAINTED deferencing "rec" tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0]


---------------------------------------------------------
[BUG] guswave_ioctl does copy_to_user but awe_ioctl doesn't, for the same
case branch "case SNDCTL_SYNTH_INFO"

/home/junfeng/linux-2.5.63/sound/oss/awe_wave.c:2049:awe_ioctl:
ERROR:TAINTED:504:2049: passing "arg" into deref cal __constant_memcpy
[dist=1][thru synth_operations:ioctl
/home/junfeng/linux-2.5.63/sound/oss/gus_wave.c:guswave_ioctl:parm2
copy_to_user:parm0]

	midi_dev:	0,
	synth_type:	SYNTH_TYPE_SAMPLE,
	synth_subtype:	SAMPLE_TYPE_AWE32,
	open:		awe_open,
	close:		awe_close,
Start --->
	ioctl:		awe_ioctl,

	... DELETED 1539 lines ...

	case SNDCTL_SYNTH_INFO:
		if (playing_mode == AWE_PLAY_DIRECT)
			awe_info.nr_voices = awe_max_voices;
		else
			awe_info.nr_voices = AWE_MAX_CHANNELS;
Error --->
		memcpy((char*)arg, &awe_info, sizeof(awe_info));
		return 0;
		break;

---------------------------------------------------------
[BUG] Bug is in vicam_write_proc_gain. cpia_write_proc does copy_from_user
while vicam_write_proc_gain doesn't.

/home/junfeng/linux-2.5.63/lib/vsprintf.c:64:simple_strtol:
ERROR:TAINTED:64:64:deferencing "cp" tainted by [dist=3][ calling
simple_strtoul  called by
/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:vicam_write_proc_gain:parm1
thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1]

 * @endp: A pointer to the end of the parsed string will be placed here
 * @base: The number base to use
 */
long simple_strtol(const char *cp,char **endp,unsigned int base)
{

Error --->
	if(*cp=='-')
		return -simple_strtoul(cp+1,endp,base);
	return simple_strtoul(cp,endp,base);
}
---------------------------------------------------------
[MINOR] in debugging code

/home/junfeng/linux-2.5.63/drivers/usb/serial/kobil_sct.c:429:kobil_write:
ERROR:TAINTED:437:429:deferencing "buf" tainted by
[dist=0][copy_from_user:parm1]

	if (! data) {
		return (-1);
	}
	memset(data, 0, (3 * count + 10));
	for (i = 0; i < count; i++) {
Error --->
		sprintf(data +3*i, "%02X ", buf[i]);
	}
	dbg(" %d --> %s", port->number, data );
	kfree(data);
	// END DEBUG

	// Copy data to buffer
	if (from_user) {
Start --->
		if (copy_from_user(priv->buf + priv->filled, buf, count))
{
			return -EFAULT;
		}
	} else {



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
  2003-03-21 21:44   ` Chris Wright
@ 2003-03-21 22:15   ` Chris Wright
  2003-03-22 20:49     ` Alan Cox
  2003-03-21 23:55   ` Chris Wright
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-03-21 22:15 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc, alan

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> [BUG] cmd is tainted. then they do copy_to_user (cmd->resbuf, ...) resbuf
> is declared as a pointer, not an array, so cmd->resbuf can point to
> anywhere.
> 
> /home/junfeng/linux-2.5.63/drivers/message/i2o/i2o_config.c:440:ioctl_parms:
> ERROR:TAINTED deferencing "cmd" tainted by [dist=0][(null)]

I don't think this could be used to corrupt kernel data because the
copy_to_user will, of course, still verify the resbuf pointer, and the
original copy_from_user validated the cmd pointer.  The possibility of
trashing the processes own memory space is there, but can be done anyway
on first pass of the cmd.  However, this is inconsistent with the rest
of the file, so here is a patch to use kcmd.resbuf.  I also added a NULL
check, as done in similar funcitons in this file.  Alan, this look ok?

thanks,
-chris
--
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/message/i2o/i2o_config.c 1.14 vs edited =====
--- 1.14/drivers/message/i2o/i2o_config.c	Wed Feb 19 12:18:31 2003
+++ edited/drivers/message/i2o/i2o_config.c	Fri Mar 21 14:17:20 2003
@@ -394,6 +394,9 @@
 	if(get_user(reslen, kcmd.reslen))
 		return -EFAULT;
 
+	if(!kcmd.resbuf)
+		return -EFAULT;
+
 	c = i2o_find_controller(kcmd.iop);
 	if(!c)
 		return -ENXIO;
@@ -437,7 +440,7 @@
 	put_user(len, kcmd.reslen);
 	if(len > reslen)
 		ret = -ENOBUFS;
-	else if(copy_to_user(cmd->resbuf, res, len))
+	else if(copy_to_user(kcmd->resbuf, res, len))
 		ret = -EFAULT;
 
 	kfree(res);

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
  2003-03-21 21:44   ` Chris Wright
  2003-03-21 22:15   ` Chris Wright
@ 2003-03-21 23:55   ` Chris Wright
  2003-03-27  8:07     ` Jan Kasprzak
  2003-03-22  0:15   ` [CHECKER] potential dereference of user pointer errors Chris Wright
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-03-21 23:55 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc, kas

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> [BUG] cosa_getidstr calls copy_to_user on arg, which implies arg is
> tainted.

True, arg is a pointer from userspace call to ioctl().

> /home/junfeng/linux-2.5.63/drivers/net/wan/cosa.c:1109:cosa_readmem:
> ERROR:TAINTED deferencing "d" tainted by [dist=2][called by
> cosa_ioctl_common:parm3 calling cosa_getidstr:parm1 calling
> copy_to_user:parm0]

Both cosa_readmem and cosa_download don't seem to do any validation of
the user supplied ptr at all before dereferncing it in get_user.  And
it'd make sense to use 'code' in cosa_reamdme (as in cosa_download)
instead of 'd->code'.  Jan, does this look OK?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/net/wan/cosa.c 1.17 vs edited =====
--- 1.17/drivers/net/wan/cosa.c	Mon Jan 13 17:11:59 2003
+++ edited/drivers/net/wan/cosa.c	Fri Mar 21 15:53:38 2003
@@ -1057,7 +1057,8 @@
 		return -EPERM;
 	}
 
-	if (get_user(addr, &(d->addr)) ||
+	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
+	    __get_user(addr, &(d->addr)) ||
 	    __get_user(len, &(d->len)) ||
 	    __get_user(code, &(d->code)))
 		return -EFAULT;
@@ -1098,7 +1099,8 @@
 		return -EPERM;
 	}
 
-	if (get_user(addr, &(d->addr)) ||
+	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
+	    __get_user(addr, &(d->addr)) ||
 	    __get_user(len, &(d->len)) ||
 	    __get_user(code, &(d->code)))
 		return -EFAULT;
@@ -1106,7 +1108,7 @@
 	/* If something fails, force the user to reset the card */
 	cosa->firmware_status &= ~COSA_FW_RESET;
 
-	if ((i=readmem(cosa, d->code, len, addr)) < 0) {
+	if ((i=readmem(cosa, code, len, addr)) < 0) {
 		printk(KERN_NOTICE "cosa%d: reading memory failed: %d\n",
 			cosa->num, i);
 		return -EIO;

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
                     ` (2 preceding siblings ...)
  2003-03-21 23:55   ` Chris Wright
@ 2003-03-22  0:15   ` Chris Wright
  2003-03-22  0:32     ` Greg KH
  2003-03-22  0:32   ` Chris Wright
                     ` (2 subsequent siblings)
  6 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-03-22  0:15 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc, greg

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> [MINOR] in debug data
> 
> /home/junfeng/linux-2.5.63/drivers/usb/serial/kobil_sct.c:429:kobil_write:
> ERROR:TAINTED deferencing "buf" tainted by [dist=0][copy_from_user:parm1]

This is a bug, which could print kernel data if debugging was enabled.
Greg, any reason the debug info can't just use the priv->buf?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-22  0:15   ` [CHECKER] potential dereference of user pointer errors Chris Wright
@ 2003-03-22  0:32     ` Greg KH
  2003-03-22  0:47       ` Chris Wright
  0 siblings, 1 reply; 43+ messages in thread
From: Greg KH @ 2003-03-22  0:32 UTC (permalink / raw)
  To: Junfeng Yang, linux-kernel, mc

On Fri, Mar 21, 2003 at 04:15:50PM -0800, Chris Wright wrote:
> * Junfeng Yang (yjf@stanford.edu) wrote:
> > 
> > [MINOR] in debug data
> > 
> > /home/junfeng/linux-2.5.63/drivers/usb/serial/kobil_sct.c:429:kobil_write:
> > ERROR:TAINTED deferencing "buf" tainted by [dist=0][copy_from_user:parm1]
> 
> This is a bug, which could print kernel data if debugging was enabled.
> Greg, any reason the debug info can't just use the priv->buf?

Ugh, that's pretty bad.  That whole chunk of debug code should just be
replaced with a call to usb_serial_debug_data() like the other
usb-serial drivers do.

Patches welcomed :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
                     ` (3 preceding siblings ...)
  2003-03-22  0:15   ` [CHECKER] potential dereference of user pointer errors Chris Wright
@ 2003-03-22  0:32   ` Chris Wright
  2003-03-23 23:10   ` Junfeng Yang
  2003-03-24 22:28   ` Raja R Harinath
  6 siblings, 0 replies; 43+ messages in thread
From: Chris Wright @ 2003-03-22  0:32 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> [UNKNOWN] sys_quotaoctl is a real system call. The entry point is at
> entry.S. But this file is under subdir fs. So there must be something we
> are missing
> 
> /home/junfeng/linux-2.5.63/fs/block_dev.c:817:lookup_bdev: ERROR:TAINTED
> deferencing "path" tainted by [dist=1][called by
> /home/junfeng/linux-2.5.63/fs/quota.c:sys_quotactl:parm1]

Yup, this is a bug, should have a getname() in there.  Patch below.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== fs/quota.c 1.12 vs edited =====
--- 1.12/fs/quota.c	Sun Feb  2 13:40:31 2003
+++ edited/fs/quota.c	Fri Mar 21 16:25:46 2003
@@ -221,12 +221,17 @@
 	uint cmds, type;
 	struct super_block *sb = NULL;
 	struct block_device *bdev;
+	char *tmp;
 	int ret = -ENODEV;
 
 	cmds = cmd >> SUBCMDSHIFT;
 	type = cmd & SUBCMDMASK;
 
-	bdev = lookup_bdev(special);
+	tmp = getname(special);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+	bdev = lookup_bdev(tmp);
+	putname(tmp);
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 	sb = get_super(bdev);

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-22  0:32     ` Greg KH
@ 2003-03-22  0:47       ` Chris Wright
  2003-03-22  1:00         ` Greg KH
  0 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-03-22  0:47 UTC (permalink / raw)
  To: Greg KH; +Cc: Junfeng Yang, linux-kernel, mc

* Greg KH (greg@kroah.com) wrote:
> 
> Ugh, that's pretty bad.  That whole chunk of debug code should just be
> replaced with a call to usb_serial_debug_data() like the other
> usb-serial drivers do.
> 
> Patches welcomed :)

Something like this?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/usb/serial/kobil_sct.c 1.5 vs edited =====
--- 1.5/drivers/usb/serial/kobil_sct.c	Wed Mar 12 14:57:33 2003
+++ edited/drivers/usb/serial/kobil_sct.c	Fri Mar 21 16:50:56 2003
@@ -406,8 +406,6 @@
 	int result = 0;
 	int todo = 0;
 	struct kobil_private * priv;
-	int i;
-	char *data;
 
 	if (count == 0) {
 		dbg("%s - port %d write request of 0 bytes", __FUNCTION__, port->number);
@@ -421,19 +419,6 @@
 		return -ENOMEM;
 	}
 
-	// BEGIN DEBUG
-	data = (unsigned char *) kmalloc((3 * count + 10) * sizeof(char), GFP_KERNEL);  
-	if (! data) {
-		return (-1);
-	}
-	memset(data, 0, (3 * count + 10));
-	for (i = 0; i < count; i++) { 
-		sprintf(data +3*i, "%02X ", buf[i]); 
-	} 
-	dbg(" %d --> %s", port->number, data );
-	kfree(data);
-	// END DEBUG
-
 	// Copy data to buffer
 	if (from_user) {
 		if (copy_from_user(priv->buf + priv->filled, buf, count)) {
@@ -442,6 +427,8 @@
 	} else {
 		memcpy (priv->buf + priv->filled, buf, count);
 	}
+
+	usb_serial_debug_data (__FILE__, __FUNCTION__, count, priv->buf + priv->filled);
 
 	priv->filled = priv->filled + count;
 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-22  0:47       ` Chris Wright
@ 2003-03-22  1:00         ` Greg KH
  0 siblings, 0 replies; 43+ messages in thread
From: Greg KH @ 2003-03-22  1:00 UTC (permalink / raw)
  To: Junfeng Yang, linux-kernel, mc

On Fri, Mar 21, 2003 at 04:47:08PM -0800, Chris Wright wrote:
> * Greg KH (greg@kroah.com) wrote:
> > 
> > Ugh, that's pretty bad.  That whole chunk of debug code should just be
> > replaced with a call to usb_serial_debug_data() like the other
> > usb-serial drivers do.
> > 
> > Patches welcomed :)
> 
> Something like this?

Looks good, I've added it to my trees.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-22 20:49     ` Alan Cox
@ 2003-03-22 20:19       ` Chris Wright
  0 siblings, 0 replies; 43+ messages in thread
From: Chris Wright @ 2003-03-22 20:19 UTC (permalink / raw)
  To: Alan Cox; +Cc: Chris Wright, Junfeng Yang, Linux Kernel Mailing List, mc

* Alan Cox (alan@lxorguk.ukuu.org.uk) wrote:
> On Fri, 2003-03-21 at 22:15, Chris Wright wrote:
> > on first pass of the cmd.  However, this is inconsistent with the rest
> > of the file, so here is a patch to use kcmd.resbuf.  I also added a NULL
> > check, as done in similar funcitons in this file.  Alan, this look ok?
> 
> Looks slightly wrong to me
> 
> #1 ->resbuf = NULL is a completely acceptable if odd user choice. If invalid
> its covered

OK, I wasn't sure if it was valid.  I noticed the other routines in that
file making similar checks.

> #2 - We copy to the users nominated cmd->resbuf. You are correct there, 
> that we should be using the kernel side copy. Fixed in my tree.

Great, thanks.
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21 22:15   ` Chris Wright
@ 2003-03-22 20:49     ` Alan Cox
  2003-03-22 20:19       ` Chris Wright
  0 siblings, 1 reply; 43+ messages in thread
From: Alan Cox @ 2003-03-22 20:49 UTC (permalink / raw)
  To: Chris Wright; +Cc: Junfeng Yang, Linux Kernel Mailing List, mc

On Fri, 2003-03-21 at 22:15, Chris Wright wrote:
> on first pass of the cmd.  However, this is inconsistent with the rest
> of the file, so here is a patch to use kcmd.resbuf.  I also added a NULL
> check, as done in similar funcitons in this file.  Alan, this look ok?

Looks slightly wrong to me

#1 ->resbuf = NULL is a completely acceptable if odd user choice. If invalid
its covered

#2 - We copy to the users nominated cmd->resbuf. You are correct there, 
that we should be using the kernel side copy. Fixed in my tree.


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
                     ` (4 preceding siblings ...)
  2003-03-22  0:32   ` Chris Wright
@ 2003-03-23 23:10   ` Junfeng Yang
  2003-03-24  0:24     ` [CHECKER] 63 potential calling blocking functions with locks held errors Junfeng Yang
                       ` (2 more replies)
  2003-03-24 22:28   ` Raja R Harinath
  6 siblings, 3 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-03-23 23:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc

Hi,

Here are more bugs on derefences of user pointers.

Where the bugs occur:

      fs/intermezzo/vfs.c
      drivers/media/video/zoran_procfs.c
      drivers/media/video/zr36120.c
      drivers/video/sis/sis_main.c
      drivers/char/specialix.c
      drivers/pnp/pnpbios/proc.c
      drivers/media/dvb/av7110/av7110_ir.c
      drivers/usb/serial/ipaq.c
      drivers/char/vt.c
      drivers/isdn/hardware/eicon/divasproc.c
      drivers/media/radio/radio-cadet.c
      drivers/media/video/bw-qcam.c
      sound/pci/es1938.c
      sound/oss/awe_wave.c
      sound/isa/sb/sb16_csp.c
      sound/oss/cmpci.c

Main sources of bugs: proc_write functions and ioctl functions

Thanks a lot for Chris and other people's replies on my previous error
reports.

Junfeng

---------------------------------------------------------
[BUG] write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/zoran_procfs.c:122:zoran_write_proc:
ERROR:tainted:122:122: passing buffer into deref cal __constant_memcpy
[dist=1][thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1][START: buffer, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/media/video/zoran_procfs.c,
zoran_write_proc, 122]

	string = sp = vmalloc(count + 1);
	if (!string) {
		printk(KERN_ERR "%s: write_proc: can not allocate
memory\n", zr->name);
		return -ENOMEM;
	}

Error --->
	memcpy(string, buffer, count);
	string[count] = 0;
	DEBUG2(printk(KERN_INFO "%s: write_proc: name=%s count=%lu
data=%x\n", zr->name, file->f_dentry->d_name.name, count, (int) data));
	ldelim = " \t\n";
---------------------------------------------------------
[BUG] buf is passed into access_ok.
/home/junfeng/linux-2.5.63/drivers/char/specialix.c:1654:sx_write:
ERROR:tainted:1654:1654: passing buf into deref cal __constant_memcpy
[dist=1][thru tty_driver:write
/home/junfeng/linux-2.5.63/drivers/char/epca.c:pc_write:parm2
copy_from_user:parm1][START: buf, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/char/specialix.c, sx_write, 1654]

					   SERIAL_XMIT_SIZE -
port->xmit_head));
			if (c <= 0) {
				restore_flags(flags);
				break;
			}

Error --->
			memcpy(port->xmit_buf + port->xmit_head, buf, c);
			port->xmit_head = (port->xmit_head + c) &
(SERIAL_XMIT_SIZE-1);
			port->xmit_cnt += c;
			restore_flags(flags);
---------------------------------------------------------
[BUG] buf is passed into access_ok, which implies it is tainted.
/home/junfeng/linux-2.5.63/drivers/media/video/zr36120.c:1698:vbi_read:
ERROR:tainted:1698:1698:deferencing optr++ tainted by [dist=-1][(null)]

		{
			/* copy to doubled data to userland */
			for (x=0; optr+1<eptr && x<-done->w; x++)
			{
				unsigned char a = iptr[x*2];

Error --->
				*optr++ = a;
				*optr++ = a;
			}
			/* and clear the rest of the line */
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:1817:sis_malloc:
ERROR:tainted:1817:1817:deferencing req tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0][START: req, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c, sis_malloc, 1817]


void sis_malloc(struct sis_memreq *req)
{
	SIS_OH *poh;


Error --->
	poh = sisfb_poh_allocate(req->size);

	if (poh == NULL) {
		req->offset = 0;
---------------------------------------------------------
[BUG] proc_write
/home/junfeng/linux-2.5.63/drivers/pnp/pnpbios/proc.c:190:proc_write_node:
ERROR:tainted:190:190: passing buf into deref cal __constant_memcpy
[dist=1][thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1][START: buf, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/pnp/pnpbios/proc.c, proc_write_node,
190]

	if (!node) return -ENOMEM;
	if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
		return -EIO;
	if (count != node->size - sizeof(struct pnp_bios_node))
		return -EINVAL;

Error --->
	memcpy(node->data, buf, count);
	if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
	    return -EINVAL;
	kfree(node);
---------------------------------------------------------
[BUG] bug is in lento_symlink. oldname is passed into getname, which
implies it is tainted. Then it is passed to presto_do_symlink. should pass
in "from" not "oldname".
/home/junfeng/linux-2.5.63/fs/namei.c:2190:page_symlink:
ERROR:tainted:2190:2190: passing symname into deref cal __constant_memcpy
[dist=6][ called by
/home/junfeng/linux-2.5.63/fs/ramfs/inode.c:ramfs_symlink:parm2 thru
inode_operations:symlink
/home/junfeng/linux-2.5.63/fs/intermezzo/dir.c:presto_symlink:parm2
calling
/home/junfeng/linux-2.5.63/fs/intermezzo/vfs.c:presto_do_symlink:parm3
called by lento_symlink  calling
/home/junfeng/linux-2.5.63/fs/namei.c:getname  called by
/home/junfeng/linux-2.5.63/fs/open.c:sys_open sys_open:parm0][START:
symname, unknown->tainted, /home/junfeng/linux-2.5.63/fs/namei.c,
page_symlink, 2190]

		goto fail;
	err = mapping->a_ops->prepare_write(NULL, page, 0, len-1);
	if (err)
		goto fail_map;
	kaddr = kmap_atomic(page, KM_USER0);

Error --->
	memcpy(kaddr, symname, len-1);
	kunmap_atomic(kaddr, KM_USER0);
	mapping->a_ops->commit_write(NULL, page, 0, len-1);
	/*
---------------------------------------------------------
[BUG] buf is passed to access_ok
/home/junfeng/linux-2.5.63/drivers/media/video/zr36120.c:1703:vbi_read:
ERROR:tainted:1703:1703:deferencing optr++ tainted by [dist=-1][(null)]

				*optr++ = a;
				*optr++ = a;
			}
			/* and clear the rest of the line */
			for (x*=2; optr<eptr && x<done->bpl; x++)

Error --->
				*optr++ = 0;
			/* next line */
			iptr += done->bpl;
		}
---------------------------------------------------------
[BUG] in the caller cm_write, buff is passed into access_ok
/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:593:trans_ac3:
ERROR:tainted:593:593:deferencing src++ tainted by [dist=-1][(null)]

	unsigned long data;
	unsigned long *dst = (unsigned long *) dest;
	unsigned short *src = (unsigned short *)source;

	do {

Error --->
		data = (unsigned long) *src++;
		data <<= 12;			// ok for 16-bit data
		if (s->spdif_counter == 2 || s->spdif_counter == 3)
			data |= 0x40000000;	// indicate AC-3 raw data
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:1673:cm_write:
ERROR:tainted:1673:1673:deferencing src++ tainted by [dist=-1][(null)]

			src = (unsigned long *) buffer;
			dst0 = (unsigned long *) (s->dma_dac.rawbuf +
swptr);
			dst1 = (unsigned long *) (s->dma_adc.rawbuf +
swptr);
			// copy left/right sample at one time
			for (i = 0; i <= cnt / 4; i++) {

Error --->
				*dst0++ = *src++;
				*dst1++ = *src++;
			}
			swptr = (swptr + cnt) % s->dma_dac.dmasize;
---------------------------------------------------------
[BUG] Bug is in ipaq_write. if debug is on, can print out kernel data
/home/junfeng/linux-2.5.63/drivers/usb/serial/usb-serial.h:363:usb_serial_debug_data:
ERROR:tainted:363:363:deferencing data tainted by [dist=2][ called by
/home/junfeng/linux-2.5.63/drivers/usb/serial/ipaq.c:ipaq_write:parm2 thru
usb_serial_device_type:write
/home/junfeng/linux-2.5.63/drivers/usb/serial/keyspan_pda.c:keyspan_pda_write:parm2
copy_from_user:parm1][START: data, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/usb/serial/usb-serial.h,
usb_serial_debug_data, 363]

	if (!debug)
		return;

	printk (KERN_DEBUG "%s: %s - length = %d, data = ", file,
function, size);
	for (i = 0; i < size; ++i) {

Error --->
		printk ("%.2x ", data[i]);
	}
	printk ("\n");
}
---------------------------------------------------------
[BUG] write_proc
/home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc:
ERROR:tainted:116:116: passing buffer into deref cal __constant_memcpy
[dist=1][thru proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1][START: buffer, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c,
av7110_ir_write_proc, 116]

	u32 ir_config;

	if (count < 4 + 256 * sizeof(u16))
		return -EINVAL;


Error --->
	memcpy (&ir_config, buffer, 4);
	memcpy (&key_map, buffer + 4, 256 * sizeof(u16));

	av7110_setup_irc_config (NULL, ir_config);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c:276:sis_dispinfo:
ERROR:tainted:276:276:deferencing rec tainted by [dist=2][ called by
sisfb_ioctl:parm3 thru fb_ops:fb_ioctl
/home/junfeng/linux-2.5.63/drivers/video/matrox/matroxfb_crtc2.c:matroxfb_dh_ioctl:parm3
copy_to_user:parm0][START: rec, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/video/sis/sis_main.c, sis_dispinfo,
276]

	gly->ngmask = size;
}

void sis_dispinfo(struct ap_data *rec)
{

Error --->
	rec->minfo.bpp    = ivideo.video_bpp;
	rec->minfo.xres   = ivideo.video_width;
	rec->minfo.yres   = ivideo.video_height;
	rec->minfo.v_xres = ivideo.video_vwidth;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/awe_wave.c:2049:awe_ioctl:
ERROR:tainted:2049:2049: passing arg into deref cal __constant_memcpy
[dist=1][thru synth_operations:ioctl
/home/junfeng/linux-2.5.63/sound/oss/gus_wave.c:guswave_ioctl:parm2
copy_to_user:parm0][START: arg, unknown->tainted,
/home/junfeng/linux-2.5.63/sound/oss/awe_wave.c, awe_ioctl, 2049]

	case SNDCTL_SYNTH_INFO:
		if (playing_mode == AWE_PLAY_DIRECT)
			awe_info.nr_voices = awe_max_voices;
		else
			awe_info.nr_voices = AWE_MAX_CHANNELS;

Error --->
		memcpy((char*)arg, &awe_info, sizeof(awe_info));
		return 0;
		break;

---------------------------------------------------------
[BUG] snd_sb_csp_ioctl -> snd_sb_csp_riff_load ->snd_sb_csp_load.
/home/junfeng/linux-2.5.63/sound/isa/sb/sb16_csp.c:647:snd_sb_csp_load:
ERROR:tainted:647:647:deferencing buf++ tainted by
[dist=-1][(null)][TRANS: buf++, $stop$->tainted,
/home/junfeng/linux-2.5.63/sound/isa/sb/sb16_csp.c, snd_sb_csp_load, 647]

		}
		kfree (_kbuf);
	} else {
		/* load from kernel space */
		while (size--) {

Error --->
			if (!snd_sbdsp_command(p->chip, *buf++))
				goto __fail;
		}
	}
---------------------------------------------------------
[BUG] vt_console_print is assigned to struct console.write
/home/junfeng/linux-2.5.63/drivers/char/vt.c:2126:vt_console_print:
ERROR:tainted:2126:2126:deferencing b++ tainted by
[dist=-1][(null)][TRANS: b++, $stop$->tainted,
/home/junfeng/linux-2.5.63/drivers/char/vt.c, vt_console_print, 2126]

	start = (ushort *)pos;

	/* Contrived structure to try to emulate original need_wrap
behaviour
	 * Problems caused when we have need_wrap set on '\n' character */
	while (count--) {

Error --->
		c = *b++;
		if (c == 10 || c == 13 || c == 8 || need_wrap) {
			if (cnt > 0) {
				if (IS_VISIBLE)
---------------------------------------------------------
[BUG] write_proc
/home/junfeng/linux-2.5.63/drivers/isdn/hardware/eicon/divasproc.c:191:write_d_l1_down:
ERROR:tainted:191:191:deferencing buffer tainted by [dist=1][thru
proc_dir_entry:write_proc
/home/junfeng/linux-2.5.63/drivers/media/video/cpia.c:cpia_write_proc:parm1
copy_from_user:parm1][START: buffer, unknown->tainted,
/home/junfeng/linux-2.5.63/drivers/isdn/hardware/eicon/divasproc.c,
write_d_l1_down, 191]

{
	diva_os_xdi_adapter_t *a = (diva_os_xdi_adapter_t *) data;
	PISDN_ADAPTER IoAdapter = IoAdapters[a->controller - 1];

	if ((count == 1) || (count == 2)) {

Error --->
		switch (buffer[0]) {
		case '0':
			IoAdapter->capi_cfg.cfg_1 &=
			    ~DIVA_XDI_CAPI_CFG_1_DYNAMIC_L1_ON;
---------------------------------------------------------
[BUG] in ioctl
/home/junfeng/linux-2.5.63/drivers/media/radio/radio-cadet.c:396:cadet_do_ioctl:
ERROR:tainted:395:396: passing v into deref cal
__constant_c_and_count_memset [dist=-1][(null)][TRANS: v,
tainted->tainted,
/home/junfeng/linux-2.5.63/drivers/media/radio/radio-cadet.c,
cadet_do_ioctl, 395]

{
	switch(cmd)
	{
		case VIDIOCGCAP:
		{
Start --->
			struct video_capability *v = arg;
Error --->
			memset(v,0,sizeof(*v));
			v->type=VID_TYPE_TUNER;
			v->channels=2;
			v->audios=1;
---------------------------------------------------------
[BUG] in ioctl
/home/junfeng/linux-2.5.63/drivers/media/video/bw-qcam.c:836:qcam_do_ioctl:
ERROR:tainted:835:836: passing vw into deref cal
__constant_c_and_count_memset [dist=-1][(null)][TRANS: vw,
tainted->tainted,
/home/junfeng/linux-2.5.63/drivers/media/video/bw-qcam.c, qcam_do_ioctl,
835]

			/* Ok we figured out what to use from our wide
choice */
			return 0;
		}
		case VIDIOCGWIN:
		{
Start --->
			struct video_window *vw = arg;
Error --->
			memset(vw, 0, sizeof(*vw));
			vw->width=qcam->width/qcam->transfer_scale;
			vw->height=qcam->height/qcam->transfer_scale;
			return 0;
---------------------------------------------------------
[BUG] copy function can take tainted inputs
/home/junfeng/linux-2.5.63/sound/pci/es1938.c:833:snd_es1938_capture_copy:
ERROR:tainted:835:833: passing dst into deref cal __constant_memcpy
[dist=1][thru snd_pcm_ops_t:copy
/home/junfeng/linux-2.5.63/sound/pci/cmipci.c:snd_cmipci_ac3_copy:parm3
copy_from_user:parm1][START: dst, unknown->tainted,
/home/junfeng/linux-2.5.63/sound/pci/es1938.c, snd_es1938_capture_copy,
833]

	es1938_t *chip = snd_pcm_substream_chip(substream);
	pos <<= chip->dma1_shift;
	count <<= chip->dma1_shift;
	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
	if (pos + count < chip->dma1_size)
Error --->
		memcpy(dst, runtime->dma_area + pos + 1, count);
	else {
Start --->
		memcpy(dst, runtime->dma_area + pos + 1, count - 1);
		((unsigned char *)dst)[count - 1] = runtime->dma_area[0];
	}
	return 0;




^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 63 potential calling blocking functions with locks held errors
  2003-03-23 23:10   ` Junfeng Yang
@ 2003-03-24  0:24     ` Junfeng Yang
  2003-03-24 12:35       ` [CHECKER] 8 potential calling blocking kmalloc(GFP_KERNEL) " Junfeng Yang
  2003-03-24  0:29     ` [CHECKER] 1 potential double unlock error Junfeng Yang
  2003-03-24  9:07     ` [CHECKER] potential dereference of user pointer errors Jaroslav Kysela
  2 siblings, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-03-24  0:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc


Enclosed are 63 potential calling blocking functions with lock held
errors. 37 of them appeared in sound/oss/maestro3.c. Any confirmation or
clarification will be greatly appreciated.

Junfeng

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1772:m3_ioctl:
ERROR:BLOCK:1771:1772:calling blocking fn m3_update_ptr with lock
&(*card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1771]

    case SNDCTL_DSP_GETOSPACE:
        if (!(file->f_mode & FMODE_WRITE))
            return -EINVAL;
        if (!(s->enable & DAC_RUNNING) && (val = prog_dmabuf(s, 0)) !=
0)
            return val;
Start --->
        spin_lock_irqsave(&card->lock, flags);
Error --->
        m3_update_ptr(s);
        abinfo.fragsize = s->dma_dac.fragsize;
        abinfo.bytes = s->dma_dac.dmasize - s->dma_dac.count;
        abinfo.fragstotal = s->dma_dac.numfrag;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2773:ess_ioctl:
ERROR:BLOCK:2772:2773:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_ioctl, 2772]

        case SNDCTL_DSP_GETOPTR:
		if (!(file->f_mode & FMODE_WRITE))
			return -EINVAL;
		if (!s->dma_dac.ready && (ret = prog_dmabuf(s, 0)))
			return ret;
Start --->
		spin_lock_irqsave(&s->lock, flags);
Error --->
		ess_update_ptr(s);
                cinfo.bytes = s->dma_dac.total_bytes;
                cinfo.blocks = s->dma_dac.count >> s->dma_dac.fragshift;
                cinfo.ptr = s->dma_dac.hwptr;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1826:m3_ioctl:
ERROR:BLOCK:1825:1826:calling blocking fn m3_update_ptr with lock
&(*card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1825]

	return 0;

    case SNDCTL_DSP_GETOPTR:
        if (!(file->f_mode & FMODE_WRITE))
            return -EINVAL;
Start --->
        spin_lock_irqsave(&card->lock, flags);
Error --->
        m3_update_ptr(s);
        cinfo.bytes = s->dma_dac.total_bytes;
        cinfo.blocks = s->dma_dac.count >> s->dma_dac.fragshift;
        cinfo.ptr = s->dma_dac.hwptr;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1511:m3_poll:
ERROR:BLOCK:1510:1511:calling blocking fn m3_update_ptr with lock
&(*(*s).card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*(*s).card).lock,
unknown->locked, /home/junfeng/linux-2.5.63/sound/oss/maestro3.c,
m3_poll, 1510]

    if (file->f_mode & FMODE_WRITE)
        poll_wait(file, &s->dma_dac.wait, wait);
    if (file->f_mode & FMODE_READ)
        poll_wait(file, &s->dma_adc.wait, wait);

Start --->
    spin_lock_irqsave(&s->card->lock, flags);
Error --->
    m3_update_ptr(s);

    if (file->f_mode & FMODE_READ) {
        if (s->dma_adc.count >= (signed)s->dma_adc.fragsize)
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2756:ess_ioctl:
ERROR:BLOCK:2755:2756:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_ioctl, 2755]

        case SNDCTL_DSP_GETIPTR:
		if (!(file->f_mode & FMODE_READ))
			return -EINVAL;
		if (!s->dma_adc.ready && (ret =  prog_dmabuf(s, 1)))
			return ret;
Start --->
		spin_lock_irqsave(&s->lock, flags);
Error --->
		ess_update_ptr(s);
                cinfo.bytes = s->dma_adc.total_bytes;
                cinfo.blocks = s->dma_adc.count >> s->dma_adc.fragshift;
                cinfo.ptr = s->dma_adc.hwptr;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2745:ess_ioctl:
ERROR:BLOCK:2744:2745:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_ioctl, 2744]

        case SNDCTL_DSP_GETODELAY:
		if (!(file->f_mode & FMODE_WRITE))
			return -EINVAL;
		if (!s->dma_dac.ready && (ret = prog_dmabuf(s, 0)))
			return ret;
Start --->
		spin_lock_irqsave(&s->lock, flags);
Error --->
		ess_update_ptr(s);
                val = s->dma_dac.count;
		spin_unlock_irqrestore(&s->lock, flags);
		return put_user(val, (int *)arg);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:430:m3_assp_read:
ERROR:BLOCK:429:430:calling blocking fn __m3_assp_read with lock
&(*card).lock held [__m3_assp_read calling m3_outw calling check_suspend
calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_assp_read, 429]

static u16 m3_assp_read(struct m3_card *card, u16 region, u16 index)
{
    unsigned long flags;
    u16 ret;

Start --->
    spin_lock_irqsave(&(card->lock), flags);
Error --->
    ret = __m3_assp_read(card, region, index);
    spin_unlock_irqrestore(&(card->lock), flags);

    return ret;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/atm/iphase.c:3221:ia_init_one:
ERROR:BLOCK:3220:3221:calling blocking fn ia_start with lock
&(*iadev).misc_lock held [ia_start calling rx_init calling
kmalloc][START: &(*iadev).misc_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/atm/iphase.c, ia_init_one, 3220]

	ia_dev[iadev_count] = iadev;
	_ia_dev[iadev_count] = dev;
	iadev_count++;
	spin_lock_init(&iadev->misc_lock);
	/* First fixes first. I don't want to think about this now. */
Start --->
	spin_lock_irqsave(&iadev->misc_lock, flags);
Error --->
	if (ia_init(dev) || ia_start(dev)) {
		IF_INIT(printk("IA register failed!\n");)
		iadev_count--;
		ia_dev[iadev_count] = NULL;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:449:m3_assp_write:
ERROR:BLOCK:448:449:calling blocking fn __m3_assp_write with lock
&(*card).lock held [__m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_assp_write, 448]

static void m3_assp_write(struct m3_card *card,
        u16 region, u16 index, u16 data)
{
    unsigned long flags;

Start --->
    spin_lock_irqsave(&(card->lock), flags);
Error --->
    __m3_assp_write(card, region, index, data);
    spin_unlock_irqrestore(&(card->lock), flags);
}

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2727:ess_ioctl:
ERROR:BLOCK:2726:2727:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_ioctl, 2726]

	case SNDCTL_DSP_GETISPACE:
		if (!(file->f_mode & FMODE_READ))
			return -EINVAL;
		if (!s->dma_adc.ready && (ret =  prog_dmabuf(s, 1)))
			return ret;
Start --->
		spin_lock_irqsave(&s->lock, flags);
Error --->
		ess_update_ptr(s);
		abinfo.fragsize = s->dma_adc.fragsize;
                abinfo.bytes = s->dma_adc.count;
                abinfo.fragstotal = s->dma_adc.numfrag;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2468:ess_poll:
ERROR:BLOCK:2467:2468:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_poll, 2467]


	if (file->f_mode & FMODE_WRITE)
		poll_wait(file, &s->dma_dac.wait, wait);
	if (file->f_mode & FMODE_READ)
		poll_wait(file, &s->dma_adc.wait, wait);
Start --->
	spin_lock_irqsave(&s->lock, flags);
Error --->
	ess_update_ptr(s);
	if (file->f_mode & FMODE_READ) {
		if (s->dma_adc.count >= (signed)s->dma_adc.fragsize)
			mask |= POLLIN | POLLRDNORM;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1786:m3_ioctl:
ERROR:BLOCK:1785:1786:calling blocking fn m3_update_ptr with lock
&(*card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1785]

    case SNDCTL_DSP_GETISPACE:
        if (!(file->f_mode & FMODE_READ))
            return -EINVAL;
        if (!(s->enable & ADC_RUNNING) && (val = prog_dmabuf(s, 1)) !=
0)
            return val;
Start --->
        spin_lock_irqsave(&card->lock, flags);
Error --->
        m3_update_ptr(s);
        abinfo.fragsize = s->dma_adc.fragsize;
        abinfo.bytes = s->dma_adc.count;
        abinfo.fragstotal = s->dma_adc.numfrag;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1802:m3_ioctl:
ERROR:BLOCK:1801:1802:calling blocking fn m3_update_ptr with lock
&(*card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1801]

        return 0;

    case SNDCTL_DSP_GETODELAY:
        if (!(file->f_mode & FMODE_WRITE))
            return -EINVAL;
Start --->
        spin_lock_irqsave(&card->lock, flags);
Error --->
        m3_update_ptr(s);
        val = s->dma_dac.count;
        spin_unlock_irqrestore(&card->lock, flags);
        return put_user(val, (int *)arg);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:2713:ess_ioctl:
ERROR:BLOCK:2712:2713:calling blocking fn ess_update_ptr with lock
&(*s).lock held [ess_update_ptr calling get_dmaa calling
apu_get_register calling check_suspend calling schedule][START:
&(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, ess_ioctl, 2712]

	case SNDCTL_DSP_GETOSPACE:
		if (!(file->f_mode & FMODE_WRITE))
			return -EINVAL;
		if (!s->dma_dac.ready && (ret = prog_dmabuf(s, 0)))
			return ret;
Start --->
		spin_lock_irqsave(&s->lock, flags);
Error --->
		ess_update_ptr(s);
		abinfo.fragsize = s->dma_dac.fragsize;
                abinfo.bytes = s->dma_dac.dmasize - s->dma_dac.count;
                abinfo.fragstotal = s->dma_dac.numfrag;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1811:m3_ioctl:
ERROR:BLOCK:1810:1811:calling blocking fn m3_update_ptr with lock
&(*card).lock held [m3_update_ptr calling stop_dac calling
dec_timer_users calling __m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1810]

        return put_user(val, (int *)arg);

    case SNDCTL_DSP_GETIPTR:
        if (!(file->f_mode & FMODE_READ))
            return -EINVAL;
Start --->
        spin_lock_irqsave(&card->lock, flags);
Error --->
        m3_update_ptr(s);
        cinfo.bytes = s->dma_adc.total_bytes;
        cinfo.blocks = s->dma_adc.count >> s->dma_adc.fragshift;
        cinfo.ptr = s->dma_adc.hwptr;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1617:m3_ioctl:
ERROR:BLOCK:1615:1617:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1615]


    case SNDCTL_DSP_GETCAPS:
        return put_user(DSP_CAP_DUPLEX | DSP_CAP_REALTIME |
DSP_CAP_TRIGGER | DSP_CAP_MMAP, (int *)arg);

    case SNDCTL_DSP_RESET:
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (file->f_mode & FMODE_WRITE) {
Error --->
            stop_dac(s);
            synchronize_irq(s->card->pcidev->irq);
            s->dma_dac.swptr = s->dma_dac.hwptr = s->dma_dac.count =
s->dma_dac.total_bytes = 0;
        }
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/ieee1394/sbp2.c:684:sbp2util_create_command_orb_pool:
ERROR:BLOCK:682:684:calling blocking fn kmalloc with lock
&(*scsi_id).sbp2_command_orb_lock held [kmalloc][START:
&(*scsi_id).sbp2_command_orb_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/ieee1394/sbp2.c,
sbp2util_create_command_orb_pool, 682]

	unsigned long flags, orbs;
	struct sbp2_command_info *command;

	orbs = sbp2_serialize_io ? 2 : SBP2_MAX_COMMAND_ORBS;

Start --->
	spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
	for (i = 0; i < orbs; i++) {
Error --->
		command = (struct sbp2_command_info *)
		    kmalloc(sizeof(struct sbp2_command_info),
GFP_KERNEL);
		if (!command) {

spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1274:snd_korg1212_capture_open:
ERROR:BLOCK:1272:1274:calling blocking fn snd_korg1212_OpenCard with
lock &(*korg1212).lock held [snd_korg1212_OpenCard calling
snd_korg1212_setCardState calling snd_korg1212_TurnOffIdleMonitor
calling snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_capture_open, 1272]


#if K1212_DEBUG_LEVEL > 0
		K1212_DEBUG_PRINTK("K1212_DEBUG:
snd_korg1212_capture_open [%s]\n", stateName[korg1212->cardState]);
#endif

Start --->
        spin_lock_irqsave(&korg1212->lock, flags);

Error --->
        snd_korg1212_OpenCard(korg1212);

        snd_pcm_set_sync(substream);    // ???

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1244:snd_korg1212_playback_open:
ERROR:BLOCK:1242:1244:calling blocking fn snd_korg1212_OpenCard with
lock &(*korg1212).lock held [snd_korg1212_OpenCard calling
snd_korg1212_setCardState calling snd_korg1212_TurnOffIdleMonitor
calling snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_playback_open, 1242]


#if K1212_DEBUG_LEVEL > 0
		K1212_DEBUG_PRINTK("K1212_DEBUG:
snd_korg1212_playback_open [%s]\n", stateName[korg1212->cardState]);
#endif

Start --->
        spin_lock_irqsave(&korg1212->lock, flags);

Error --->
        snd_korg1212_OpenCard(korg1212);

        snd_pcm_set_sync(substream);    // ???

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/fs/jffs2/nodemgmt.c:111:jffs2_reserve_space_gc:
ERROR:BLOCK:109:111:calling blocking fn jffs2_do_reserve_space with lock
&(*c).erase_completion_lock held [jffs2_do_reserve_space calling
schedule][START: &(*c).erase_completion_lock, unknown->locked,
/home/junfeng/linux-2.5.63/fs/jffs2/nodemgmt.c, jffs2_reserve_space_gc,
109]

	int ret = -EAGAIN;
	minsize = PAD(minsize);

	D1(printk(KERN_DEBUG "jffs2_reserve_space_gc(): Requested 0x%x
bytes\n", minsize));

Start --->
	spin_lock_bh(&c->erase_completion_lock);
	while(ret == -EAGAIN) {
Error --->
		ret = jffs2_do_reserve_space(c, minsize, ofs, len);
		if (ret) {
		        D1(printk(KERN_DEBUG "jffs2_reserve_space_gc:
looping, ret is %d\n", ret));
		}
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1408:snd_korg1212_prepare:
ERROR:BLOCK:1406:1408:calling blocking fn snd_korg1212_SetupForPlay with
lock &(*korg1212).lock held [snd_korg1212_SetupForPlay calling
snd_korg1212_setCardState calling snd_korg1212_TurnOffIdleMonitor
calling snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_prepare, 1406]


#if K1212_DEBUG_LEVEL > 0
		K1212_DEBUG_PRINTK("K1212_DEBUG: snd_korg1212_prepare
[%s]\n", stateName[korg1212->cardState]);
#endif

Start --->
        spin_lock_irqsave(&korg1212->lock, flags);

Error --->
        snd_korg1212_SetupForPlay(korg1212);

        korg1212->currentBuffer = -1;

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:1751:prog_dmabuf:
ERROR:BLOCK:1748:1751:calling blocking fn stop_adc with lock &(*s).lock
held [stop_adc calling apu_set_register calling check_suspend calling
schedule][START: &(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, prog_dmabuf, 1748]

	unsigned bytepersec;
	unsigned bufs;
	unsigned char fmt;
	unsigned long flags;

Start --->
	spin_lock_irqsave(&s->lock, flags);
	fmt = s->fmt;
	if (rec) {
Error --->
		stop_adc(s);
		fmt >>= ESS_ADC_SHIFT;
	} else {
		stop_dac(s);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2072:m3_release:
ERROR:BLOCK:2069:2072:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_release, 2069]

    VALIDATE_STATE(s);
    if (file->f_mode & FMODE_WRITE)
        drain_dac(s, file->f_flags & O_NONBLOCK);

    down(&s->open_sem);
Start --->
    spin_lock_irqsave(&card->lock, flags);

    if (file->f_mode & FMODE_WRITE) {
Error --->
        stop_dac(s);
        if(s->dma_dac.in_lists) {
            m3_remove_list(s->card, &(s->card->mixer_list),
s->dma_dac.mixer_index);
            nuke_lists(s->card, &(s->dma_dac));
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1634:m3_ioctl:
ERROR:BLOCK:1631:1634:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1631]

        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

    case SNDCTL_DSP_SPEED:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val >= 0) {
            if (file->f_mode & FMODE_READ) {
Error --->
                stop_adc(s);
                s->dma_adc.ready = 0;
                set_adc_rate(s, val);
            }
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1653:m3_ioctl:
ERROR:BLOCK:1649:1653:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1649]

        spin_unlock_irqrestore(&card->lock, flags);
        return put_user((file->f_mode & FMODE_READ) ? s->rateadc :
s->ratedac, (int *)arg);

    case SNDCTL_DSP_STEREO:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        fmtd = 0;
        fmtm = ~0;
        if (file->f_mode & FMODE_READ) {
Error --->
            stop_adc(s);
            s->dma_adc.ready = 0;
            if (val)
                fmtd |= ESS_FMT_STEREO << ESS_ADC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1094:prog_dmabuf:
ERROR:BLOCK:1090:1094:calling blocking fn stop_adc with lock
&(*(*s).card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*(*s).card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, prog_dmabuf, 1090]

    unsigned bytepersec;
    unsigned bufs;
    unsigned char fmt;
    unsigned long flags;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

    fmt = s->fmt;
    if (rec) {
Error --->
        stop_adc(s);
        fmt >>= ESS_ADC_SHIFT;
    } else {
        stop_dac(s);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1679:m3_ioctl:
ERROR:BLOCK:1674:1679:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1674]

        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

    case SNDCTL_DSP_CHANNELS:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val != 0) {
            fmtd = 0;
            fmtm = ~0;
            if (file->f_mode & FMODE_READ) {
Error --->
                stop_adc(s);
                s->dma_adc.ready = 0;
                if (val >= 2)
                    fmtd |= ESS_FMT_STEREO << ESS_ADC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1306:snd_korg1212_playback_close:
ERROR:BLOCK:1301:1306:calling blocking fn snd_korg1212_CloseCard with
lock &(*korg1212).lock held [snd_korg1212_CloseCard calling
snd_korg1212_setCardState calling snd_korg1212_TurnOffIdleMonitor
calling snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_playback_close, 1301]


#if K1212_DEBUG_LEVEL > 0
		K1212_DEBUG_PRINTK("K1212_DEBUG:
snd_korg1212_playback_close [%s]\n", stateName[korg1212->cardState]);
#endif

Start --->
        spin_lock_irqsave(&korg1212->lock, flags);

        korg1212->playback_substream = NULL;
        korg1212->periodsize = 0;

Error --->
        snd_korg1212_CloseCard(korg1212);

        spin_unlock_irqrestore(&korg1212->lock, flags);
        return 0;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1326:snd_korg1212_capture_close:
ERROR:BLOCK:1321:1326:calling blocking fn snd_korg1212_CloseCard with
lock &(*korg1212).lock held [snd_korg1212_CloseCard calling
snd_korg1212_setCardState calling snd_korg1212_TurnOffIdleMonitor
calling snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_capture_close, 1321]


#if K1212_DEBUG_LEVEL > 0
		K1212_DEBUG_PRINTK("K1212_DEBUG:
snd_korg1212_capture_close [%s]\n", stateName[korg1212->cardState]);
#endif

Start --->
        spin_lock_irqsave(&korg1212->lock, flags);

        korg1212->capture_substream = NULL;
        korg1212->periodsize = 0;

Error --->
        snd_korg1212_CloseCard(korg1212);

        spin_unlock_irqrestore(&korg1212->lock, flags);
        return 0;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1710:m3_ioctl:
ERROR:BLOCK:1705:1710:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1705]

    case SNDCTL_DSP_GETFMTS: /* Returns a mask */
        return put_user(AFMT_U8|AFMT_S16_LE, (int *)arg);

    case SNDCTL_DSP_SETFMT: /* Selects ONE fmt*/
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val != AFMT_QUERY) {
            fmtd = 0;
            fmtm = ~0;
            if (file->f_mode & FMODE_READ) {
Error --->
                stop_adc(s);
                s->dma_adc.ready = 0;
                if (val == AFMT_S16_LE)
                    fmtd |= ESS_FMT_16BIT << ESS_ADC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/scsi/dpt_i2o.c:1705:adpt_i2o_passthru:
ERROR:BLOCK:1699:1705:calling blocking fn adpt_i2o_post_wait with lock
(*(*pHba).host).host_lock held [adpt_i2o_post_wait calling
kmalloc][START: (*(*pHba).host).host_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/scsi/dpt_i2o.c, adpt_i2o_passthru,
1699]

			sg[i].addr_bus = (u32)virt_to_bus((void*)p);
		}
	}

	do {
Start --->
		spin_lock_irqsave(pHba->host->host_lock, flags);
		// This state stops any new commands from enterring the
		// controller while processing the ioctl
//		pHba->state |= DPTI_STATE_IOCTL;
//		We can't set this now - The scsi subsystem sets
host_blocked and
//		the queue empties and stops.  We need a way to restart
the queue
Error --->
		rcode = adpt_i2o_post_wait(pHba, msg, size, FOREVER);
//		pHba->state &= ~DPTI_STATE_IOCTL;
		spin_unlock_irqrestore(pHba->host->host_lock, flags);
	} while(rcode == -ETIMEDOUT);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:1754:prog_dmabuf:
ERROR:BLOCK:1748:1754:calling blocking fn stop_dac with lock &(*s).lock
held [stop_dac calling apu_set_register calling check_suspend calling
schedule][START: &(*s).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, prog_dmabuf, 1748]

	unsigned bytepersec;
	unsigned bufs;
	unsigned char fmt;
	unsigned long flags;

Start --->
	spin_lock_irqsave(&s->lock, flags);
	fmt = s->fmt;
	if (rec) {
		stop_adc(s);
		fmt >>= ESS_ADC_SHIFT;
	} else {
Error --->
		stop_dac(s);
		fmt >>= ESS_DAC_SHIFT;
	}
	spin_unlock_irqrestore(&s->lock, flags);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1097:prog_dmabuf:
ERROR:BLOCK:1090:1097:calling blocking fn stop_dac with lock
&(*(*s).card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*(*s).card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, prog_dmabuf, 1090]

    unsigned bytepersec;
    unsigned bufs;
    unsigned char fmt;
    unsigned long flags;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

    fmt = s->fmt;
    if (rec) {
        stop_adc(s);
        fmt >>= ESS_ADC_SHIFT;
    } else {
Error --->
        stop_dac(s);
        fmt >>= ESS_DAC_SHIFT;
    }
    fmt &= ESS_FMT_MASK;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1622:m3_ioctl:
ERROR:BLOCK:1615:1622:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1615]


    case SNDCTL_DSP_GETCAPS:
        return put_user(DSP_CAP_DUPLEX | DSP_CAP_REALTIME |
DSP_CAP_TRIGGER | DSP_CAP_MMAP, (int *)arg);

    case SNDCTL_DSP_RESET:
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (file->f_mode & FMODE_WRITE) {
            stop_dac(s);
            synchronize_irq(s->card->pcidev->irq);
            s->dma_dac.swptr = s->dma_dac.hwptr = s->dma_dac.count =
s->dma_dac.total_bytes = 0;
        }
        if (file->f_mode & FMODE_READ) {
Error --->
            stop_adc(s);
            synchronize_irq(s->card->pcidev->irq);
            s->dma_adc.swptr = s->dma_adc.hwptr = s->dma_adc.count =
s->dma_adc.total_bytes = 0;
        }
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:3738:maestro_resume:
ERROR:BLOCK:3731:3738:calling blocking fn maestro_config with lock
&(*card).lock held [maestro_config calling maestro_read calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, maestro_resume, 3731]

maestro_resume(struct ess_card *card)
{
	unsigned long flags;
	int i;

Start --->
	spin_lock_irqsave(&card->lock,flags); /* over-kill */

	card->in_suspend = 0;

	M_printk("maestro: resuming card at %p\n",card);

	/* restore all our config */
Error --->
	maestro_config(card);
	/* need to restore the base pointers.. */
	if(card->dmapages)
		set_base_registers(&card->channels[0],card->dmapages);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:619:dec_timer_users:
ERROR:BLOCK:611:619:calling blocking fn __m3_assp_write with lock
&(*card).lock held [__m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, dec_timer_users, 611]


static void dec_timer_users(struct m3_card *card)
{
    unsigned long flags;

Start --->
    spin_lock_irqsave(&card->lock, flags);

    card->timer_users--;
    DPRINTK(DPSYS, "dec timer users now %d\n",
            card->timer_users);
    if(card->timer_users > 0 )
        goto out;

Error --->
    __m3_assp_write(card, MEMTYPE_INTERNAL_DATA,
        KDATA_TIMER_COUNT_RELOAD,
         0 ) ;

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2040:m3_open:
ERROR:BLOCK:2032:2040:calling blocking fn set_adc_rate with lock
&(*c).lock held [set_adc_rate calling m3_assp_write calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*c).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_open, 2032]

        if (signal_pending(current))
            return -ERESTARTSYS;
        down(&s->open_sem);
    }

Start --->
    spin_lock_irqsave(&c->lock, flags);

    if (file->f_mode & FMODE_READ) {
        fmtm &= ~((ESS_FMT_STEREO | ESS_FMT_16BIT) << ESS_ADC_SHIFT);
        if ((minor & 0xf) == SND_DEV_DSP16)
            fmts |= ESS_FMT_16BIT << ESS_ADC_SHIFT;

        s->dma_adc.ossfragshift = s->dma_adc.ossmaxfrags =
s->dma_adc.subdivision = 0;
Error --->
        set_adc_rate(s, 8000);
    }
    if (file->f_mode & FMODE_WRITE) {
        fmtm &= ~((ESS_FMT_STEREO | ESS_FMT_16BIT) << ESS_DAC_SHIFT);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2854:m3_resume:
ERROR:BLOCK:2846:2854:calling blocking fn m3_outw with lock
&(*card).lock held [m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_resume, 2846]

    unsigned long flags;
    int index;
    int i;
    struct m3_card *card = pci_get_drvdata(pci_dev);

Start --->
	spin_lock_irqsave(&card->lock, flags);
    card->in_suspend = 0;

    DPRINTK(DPMOD, "resuming\n");

    /* first lets just bring everything back. .*/

    DPRINTK(DPMOD, "bringing power back on card 0x%p\n",card);
Error --->
    m3_outw(card, 0, 0x54);
    m3_outw(card, 0, 0x56);

    DPRINTK(DPMOD, "restoring pci configs and reseting codec\n");
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1639:m3_ioctl:
ERROR:BLOCK:1631:1639:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1631]

        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

    case SNDCTL_DSP_SPEED:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val >= 0) {
            if (file->f_mode & FMODE_READ) {
                stop_adc(s);
                s->dma_adc.ready = 0;
                set_adc_rate(s, val);
            }
            if (file->f_mode & FMODE_WRITE) {
Error --->
                stop_dac(s);
                s->dma_dac.ready = 0;
                set_dac_rate(s, val);
            }
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:592:inc_timer_users:
ERROR:BLOCK:584:592:calling blocking fn __m3_assp_write with lock
&(*card).lock held [__m3_assp_write calling m3_outw calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, inc_timer_users, 584]


static void inc_timer_users(struct m3_card *card)
{
    unsigned long flags;

Start --->
    spin_lock_irqsave(&card->lock, flags);

    card->timer_users++;
    DPRINTK(DPSYS, "inc timer users now %d\n",
            card->timer_users);
    if(card->timer_users != 1)
        goto out;

Error --->
    __m3_assp_write(card, MEMTYPE_INTERNAL_DATA,
        KDATA_TIMER_COUNT_RELOAD,
         240 ) ;

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2079:m3_release:
ERROR:BLOCK:2069:2079:calling blocking fn stop_adc with lock
&(*card).lock held [stop_adc calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_release, 2069]

    VALIDATE_STATE(s);
    if (file->f_mode & FMODE_WRITE)
        drain_dac(s, file->f_flags & O_NONBLOCK);

    down(&s->open_sem);
Start --->
    spin_lock_irqsave(&card->lock, flags);

    if (file->f_mode & FMODE_WRITE) {
        stop_dac(s);
        if(s->dma_dac.in_lists) {
            m3_remove_list(s->card, &(s->card->mixer_list),
s->dma_dac.mixer_index);
            nuke_lists(s->card, &(s->dma_dac));
        }
    }
    if (file->f_mode & FMODE_READ) {
Error --->
        stop_adc(s);
        if(s->dma_adc.in_lists) {
            m3_remove_list(s->card, &(s->card->adc1_list),
s->dma_adc.adc1_index);
            nuke_lists(s->card, &(s->dma_adc));
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2807:m3_suspend:
ERROR:BLOCK:2796:2807:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_suspend, 2796]

    unsigned long flags;
    int i;
    struct m3_card *card = pci_get_drvdata(pci_dev);

    /* must be a better way.. */
Start --->
	spin_lock_irqsave(&card->lock, flags);

    DPRINTK(DPMOD, "pm in dev %p\n",card);

    for(i=0;i<NR_DSPS;i++) {
        struct m3_state *s = &card->channels[i];

        if(s->dev_audio == -1)
            continue;

        DPRINTK(DPMOD, "stop_adc/dac() device %d\n",i);
Error --->
        stop_dac(s);
        stop_adc(s);
    }

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c:1765:snd_korg1212_control_analog_put:
ERROR:BLOCK:1753:1765:calling blocking fn
snd_korg1212_WriteADCSensitivity with lock &(*korg1212).lock held
[snd_korg1212_WriteADCSensitivity calling
snd_korg1212_WaitForCardStopAck calling schedule][START:
&(*korg1212).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/pci/korg1212/korg1212.c,
snd_korg1212_control_analog_put, 1753]

{
	korg1212_t *korg1212 = _snd_kcontrol_chip(kcontrol);
	unsigned long flags;
        int change = 0;

Start --->
	spin_lock_irqsave(&korg1212->lock, flags);

        if (u->value.integer.value[0] != korg1212->leftADCInSens) {
                korg1212->leftADCInSens = u->value.integer.value[0];
                change = 1;
        }
        if (u->value.integer.value[1] != korg1212->rightADCInSens) {
                korg1212->rightADCInSens = u->value.integer.value[1];
                change = 1;
        }

        if (change)
Error --->
                snd_korg1212_WriteADCSensitivity(korg1212);

	spin_unlock_irqrestore(&korg1212->lock, flags);

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/ieee1394/ohci1394.c:994:ohci_devctl:
ERROR:BLOCK:982:994:calling blocking fn alloc_dma_rcv_ctx with lock
&(*ohci).IR_channel_lock held [alloc_dma_rcv_ctx calling kmalloc][START:
&(*ohci).IR_channel_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/ieee1394/ohci1394.c, ohci_devctl,
982]

			return -EFAULT;
		}

		mask = (u64)0x1<<arg;

Start --->
                spin_lock_irqsave(&ohci->IR_channel_lock, flags);

		if (ohci->ISO_channel_usage & mask) {
			PRINT(KERN_ERR, ohci->id,
			      "%s: IS0 listen channel %d is already
used",
			      __FUNCTION__, arg);
			spin_unlock_irqrestore(&ohci->IR_channel_lock,
flags);
			return -EFAULT;
		}

		/* activate the legacy IR context */
		if(ohci->ir_legacy_context.ohci == NULL) {
Error --->
			if(alloc_dma_rcv_ctx(ohci,
&ohci->ir_legacy_context,
					     DMA_CTX_ISO, 0,
IR_NUM_DESC,
					     IR_BUF_SIZE,
IR_SPLIT_BUF_SIZE,
					     OHCI1394_IsoRcvContextBase)
< 0) {
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1661:m3_ioctl:
ERROR:BLOCK:1649:1661:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1649]

        spin_unlock_irqrestore(&card->lock, flags);
        return put_user((file->f_mode & FMODE_READ) ? s->rateadc :
s->ratedac, (int *)arg);

    case SNDCTL_DSP_STEREO:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        fmtd = 0;
        fmtm = ~0;
        if (file->f_mode & FMODE_READ) {
            stop_adc(s);
            s->dma_adc.ready = 0;
            if (val)
                fmtd |= ESS_FMT_STEREO << ESS_ADC_SHIFT;
            else
                fmtm &= ~(ESS_FMT_STEREO << ESS_ADC_SHIFT);
        }
        if (file->f_mode & FMODE_WRITE) {
Error --->
            stop_dac(s);
            s->dma_dac.ready = 0;
            if (val)
                fmtd |= ESS_FMT_STEREO << ESS_DAC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1718:m3_ioctl:
ERROR:BLOCK:1705:1718:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1705]

    case SNDCTL_DSP_GETFMTS: /* Returns a mask */
        return put_user(AFMT_U8|AFMT_S16_LE, (int *)arg);

    case SNDCTL_DSP_SETFMT: /* Selects ONE fmt*/
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val != AFMT_QUERY) {
            fmtd = 0;
            fmtm = ~0;
            if (file->f_mode & FMODE_READ) {
                stop_adc(s);
                s->dma_adc.ready = 0;
                if (val == AFMT_S16_LE)
                    fmtd |= ESS_FMT_16BIT << ESS_ADC_SHIFT;
                else
                    fmtm &= ~(ESS_FMT_16BIT << ESS_ADC_SHIFT);
            }
            if (file->f_mode & FMODE_WRITE) {
Error --->
                stop_dac(s);
                s->dma_dac.ready = 0;
                if (val == AFMT_S16_LE)
                    fmtd |= ESS_FMT_16BIT << ESS_DAC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1687:m3_ioctl:
ERROR:BLOCK:1674:1687:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling dec_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1674]

        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

    case SNDCTL_DSP_CHANNELS:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);
        if (val != 0) {
            fmtd = 0;
            fmtm = ~0;
            if (file->f_mode & FMODE_READ) {
                stop_adc(s);
                s->dma_adc.ready = 0;
                if (val >= 2)
                    fmtd |= ESS_FMT_STEREO << ESS_ADC_SHIFT;
                else
                    fmtm &= ~(ESS_FMT_STEREO << ESS_ADC_SHIFT);
            }
            if (file->f_mode & FMODE_WRITE) {
Error --->
                stop_dac(s);
                s->dma_dac.ready = 0;
                if (val >= 2)
                    fmtd |= ESS_FMT_STEREO << ESS_DAC_SHIFT;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1363:m3_read:
ERROR:BLOCK:1349:1363:calling blocking fn start_adc with lock
&(*(*s).card).lock held [start_adc calling inc_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*(*s).card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_read, 1349]

        return ret;
    if (!access_ok(VERIFY_WRITE, buffer, count))
        return -EFAULT;
    ret = 0;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

    while (count > 0) {
        int timed_out;

        swptr = s->dma_adc.swptr;
        cnt = s->dma_adc.dmasize-swptr;
        if (s->dma_adc.count < cnt)
            cnt = s->dma_adc.count;

        if (cnt > count)
            cnt = count;

        if (cnt <= 0) {
Error --->
            start_adc(s);
            if (file->f_flags & O_NONBLOCK)
            {
                ret = ret ? ret : -EAGAIN;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:3704:maestro_suspend:
ERROR:BLOCK:3689:3704:calling blocking fn stop_dac with lock
&(*card).lock held [stop_dac calling apu_set_register calling
check_suspend calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, maestro_suspend, 3689]

maestro_suspend(struct ess_card *card)
{
	unsigned long flags;
	int i,j;

Start --->
	spin_lock_irqsave(&card->lock,flags); /* over-kill */

	... DELETED 9 lines ...


		if(s->dev_audio == -1)
			continue;

		M_printk("maestro: stopping apus for device %d\n",i);
Error --->
		stop_dac(s);
		stop_adc(s);
		for(j=0;j<6;j++)

card->apu_map[s->apu[j]][5]=apu_get_register(s,j,5);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/isdn/i4l/isdn_common.c:933:register_isdn:
ERROR:BLOCK:918:933:calling blocking fn isdn_add_channels with lock
&drivers_lock held [isdn_add_channels calling kmalloc][START:
&drivers_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/isdn/i4l/isdn_common.c,
register_isdn, 918]

	drv->fi.state = ST_DRV_NULL;
	drv->fi.debug = 1;
	drv->fi.userdata = drv;
	drv->fi.printdebug = drv_debug;

Start --->
	spin_lock_irqsave(&drivers_lock, flags);

	... DELETED 9 lines ...


	if (__isdn_drv_lookup(iif->id) >= 0)
		goto fail_unlock;

	strcpy(drv->id, iif->id);
Error --->
	if (isdn_add_channels(drv, iif->channels))
		goto fail_unlock;

	drv->di = drvidx;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2048:m3_open:
ERROR:BLOCK:2032:2048:calling blocking fn set_dac_rate with lock
&(*c).lock held [set_dac_rate calling m3_assp_write calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*c).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_open, 2032]

        if (signal_pending(current))
            return -ERESTARTSYS;
        down(&s->open_sem);
    }

Start --->
    spin_lock_irqsave(&c->lock, flags);

	... DELETED 10 lines ...

        fmtm &= ~((ESS_FMT_STEREO | ESS_FMT_16BIT) << ESS_DAC_SHIFT);
        if ((minor & 0xf) == SND_DEV_DSP16)
            fmts |= ESS_FMT_16BIT << ESS_DAC_SHIFT;

        s->dma_dac.ossfragshift = s->dma_dac.ossmaxfrags =
s->dma_dac.subdivision = 0;
Error --->
        set_dac_rate(s, 8000);
    }
    set_fmt(s, fmtm, fmts);
    s->open_mode |= file->f_mode & (FMODE_READ | FMODE_WRITE);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2813:m3_suspend:
ERROR:BLOCK:2796:2813:calling blocking fn m3_assp_halt with lock
&(*card).lock held [m3_assp_halt calling m3_inb calling check_suspend
calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_suspend, 2796]

    unsigned long flags;
    int i;
    struct m3_card *card = pci_get_drvdata(pci_dev);

    /* must be a better way.. */
Start --->
	spin_lock_irqsave(&card->lock, flags);

	... DELETED 11 lines ...

        stop_adc(s);
    }

    mdelay(10); /* give the assp a chance to idle.. */

Error --->
    m3_assp_halt(card);

    if(card->suspend_mem) {
        int index = 0;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:2050:m3_open:
ERROR:BLOCK:2032:2050:calling blocking fn set_fmt with lock &(*c).lock
held [set_fmt calling m3_assp_write calling __m3_assp_write calling
m3_outw calling check_suspend calling schedule][START: &(*c).lock,
unknown->locked, /home/junfeng/linux-2.5.63/sound/oss/maestro3.c,
m3_open, 2032]

        if (signal_pending(current))
            return -ERESTARTSYS;
        down(&s->open_sem);
    }

Start --->
    spin_lock_irqsave(&c->lock, flags);

	... DELETED 12 lines ...

            fmts |= ESS_FMT_16BIT << ESS_DAC_SHIFT;

        s->dma_dac.ossfragshift = s->dma_dac.ossmaxfrags =
s->dma_dac.subdivision = 0;
        set_dac_rate(s, 8000);
    }
Error --->
    set_fmt(s, fmtm, fmts);
    s->open_mode |= file->f_mode & (FMODE_READ | FMODE_WRITE);

    up(&s->open_sem);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1668:m3_ioctl:
ERROR:BLOCK:1649:1668:calling blocking fn set_fmt with lock
&(*card).lock held [set_fmt calling m3_assp_write calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1649]

        spin_unlock_irqrestore(&card->lock, flags);
        return put_user((file->f_mode & FMODE_READ) ? s->rateadc :
s->ratedac, (int *)arg);

    case SNDCTL_DSP_STEREO:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);

	... DELETED 13 lines ...

            if (val)
                fmtd |= ESS_FMT_STEREO << ESS_DAC_SHIFT;
            else
                fmtm &= ~(ESS_FMT_STEREO << ESS_DAC_SHIFT);
        }
Error --->
        set_fmt(s, fmtm, fmtd);
        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1694:m3_ioctl:
ERROR:BLOCK:1674:1694:calling blocking fn set_fmt with lock
&(*card).lock held [set_fmt calling m3_assp_write calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1674]

        spin_unlock_irqrestore(&card->lock, flags);
        return 0;

    case SNDCTL_DSP_CHANNELS:
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);

	... DELETED 14 lines ...

                if (val >= 2)
                    fmtd |= ESS_FMT_STEREO << ESS_DAC_SHIFT;
                else
                    fmtm &= ~(ESS_FMT_STEREO << ESS_DAC_SHIFT);
            }
Error --->
            set_fmt(s, fmtm, fmtd);
        }
        spin_unlock_irqrestore(&card->lock, flags);
        return put_user((s->fmt & ((file->f_mode & FMODE_READ) ?
(ESS_FMT_STEREO << ESS_ADC_SHIFT)
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/net/wan/comx-hw-munich.c:1458:MUNICH_open:
ERROR:BLOCK:1438:1458:calling blocking fn kmalloc with lock &mister_lock
held [kmalloc][START: &mister_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/net/wan/comx-hw-munich.c,
MUNICH_open, 1438]

	printk("MUNICH_open: no %s board with boardnum = %d\n",
	       ch->hardware->name, hw->boardnum);
	return -ENODEV;
    }

Start --->
    spin_lock_irqsave(&mister_lock, flags);

	... DELETED 14 lines ...

	    board->histogram[i] = 0;

	board->lineup = 0;

	/* Allocate CCB: */
Error --->
        board->ccb = kmalloc(sizeof(munich_ccb_t), GFP_KERNEL);
	if (board->ccb == NULL)
	{
	    spin_unlock_irqrestore(&mister_lock, flags);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1725:m3_ioctl:
ERROR:BLOCK:1705:1725:calling blocking fn set_fmt with lock
&(*card).lock held [set_fmt calling m3_assp_write calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_ioctl, 1705]

    case SNDCTL_DSP_GETFMTS: /* Returns a mask */
        return put_user(AFMT_U8|AFMT_S16_LE, (int *)arg);

    case SNDCTL_DSP_SETFMT: /* Selects ONE fmt*/
        get_user_ret(val, (int *)arg, -EFAULT);
Start --->
        spin_lock_irqsave(&card->lock, flags);

	... DELETED 14 lines ...

                if (val == AFMT_S16_LE)
                    fmtd |= ESS_FMT_16BIT << ESS_DAC_SHIFT;
                else
                    fmtm &= ~(ESS_FMT_16BIT << ESS_DAC_SHIFT);
            }
Error --->
            set_fmt(s, fmtm, fmtd);
        }
        spin_unlock_irqrestore(&card->lock, flags);
        return put_user((s->fmt & ((file->f_mode & FMODE_READ) ?
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1451:m3_write:
ERROR:BLOCK:1430:1451:calling blocking fn start_dac with lock
&(*(*s).card).lock held [start_dac calling inc_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][START: &(*(*s).card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_write, 1430]

        return ret;
    if (!access_ok(VERIFY_READ, buffer, count))
        return -EFAULT;
    ret = 0;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

	... DELETED 15 lines ...


        if (cnt > count)
            cnt = count;

        if (cnt <= 0) {
Error --->
            start_dac(s);
            if (file->f_flags & O_NONBLOCK) {
                if(!ret) ret = -EAGAIN;
                goto out;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro.c:3713:maestro_suspend:
ERROR:BLOCK:3689:3713:calling blocking fn stop_bob with lock
&(*card).lock held [stop_bob calling maestro_read calling check_suspend
calling schedule][START: &(*card).lock, unknown->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro.c, maestro_suspend, 3689]

maestro_suspend(struct ess_card *card)
{
	unsigned long flags;
	int i,j;

Start --->
	spin_lock_irqsave(&card->lock,flags); /* over-kill */

	... DELETED 18 lines ...


	}

	/* get rid of interrupts? */
	if( card->dsps_open > 0)
Error --->
		stop_bob(&card->channels[0]);

	card->in_suspend++;

---------------------------------------------------------
[BUG] interesting things to check: calling kmalloc with GFP_KERNEL and
then with GFP_ATOMIC.
/home/junfeng/linux-2.5.63/net/rose/rose_route.c:112:rose_add_node:
ERROR:BLOCK:64:112:calling blocking fn kmalloc with lock
&rose_neigh_list_lock held [kmalloc][START: &rose_neigh_list_lock,
unknown->locked, /home/junfeng/linux-2.5.63/net/rose/rose_route.c,
rose_add_node, 64]

	struct rose_node  *rose_node, *rose_tmpn, *rose_tmpp;
	struct rose_neigh *rose_neigh;
	int i, res = 0;

	spin_lock_bh(&rose_node_list_lock);
Start --->
	spin_lock_bh(&rose_neigh_list_lock);

	... DELETED 42 lines ...


		init_timer(&rose_neigh->ftimer);
		init_timer(&rose_neigh->t0timer);

		if (rose_route->ndigis != 0) {
Error --->
			if ((rose_neigh->digipeat =
kmalloc(sizeof(ax25_digi), GFP_KERNEL)) == NULL) {
				kfree(rose_neigh);
				res = -ENOMEM;
				goto out;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1403:m3_read:
ERROR:BLOCK:1349:1403:calling blocking fn start_adc with lock
&(*(*s).card).lock held [start_adc calling inc_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][TRANS: &(*(*s).card).lock, unlocked->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_read, 1395]

        return ret;
    if (!access_ok(VERIFY_WRITE, buffer, count))
        return -EFAULT;
    ret = 0;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

	... DELETED 48 lines ...

        s->dma_adc.swptr = swptr;
        s->dma_adc.count -= cnt;
        count -= cnt;
        buffer += cnt;
        ret += cnt;
Error --->
        start_adc(s);
    }

out:
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c:1491:m3_write:
ERROR:BLOCK:1430:1491:calling blocking fn start_dac with lock
&(*(*s).card).lock held [start_dac calling inc_timer_users calling
__m3_assp_write calling m3_outw calling check_suspend calling
schedule][TRANS: &(*(*s).card).lock, unlocked->locked,
/home/junfeng/linux-2.5.63/sound/oss/maestro3.c, m3_write, 1478]

        return ret;
    if (!access_ok(VERIFY_READ, buffer, count))
        return -EFAULT;
    ret = 0;

Start --->
    spin_lock_irqsave(&s->card->lock, flags);

	... DELETED 55 lines ...

        s->dma_dac.count += cnt;
        s->dma_dac.endcleared = 0;
        count -= cnt;
        buffer += cnt;
        ret += cnt;
Error --->
        start_dac(s);
    }
out:
    spin_unlock_irqrestore(&s->card->lock, flags);
---------------------------------------------------------
[BUG] such a huge ioctl function ...
/home/junfeng/linux-2.5.63/drivers/net/wan/lmc/lmc_main.c:500:lmc_ioctl:
ERROR:BLOCK:160:500:calling blocking fn kmalloc with lock
&(*sc).lmc_lock held [kmalloc][START: &(*sc).lmc_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/net/wan/lmc/lmc_main.c, lmc_ioctl,
160]


    /*
     * Most functions mess with the structure
     * Disable interrupts while we do the polling
     */
Start --->
    spin_lock_irqsave(&sc->lmc_lock, flags);

	... DELETED 334 lines ...

                    if(xc.data == 0x0){
                            ret = -EINVAL;
                            break;
                    }

Error --->
                    data = kmalloc(xc.len, GFP_KERNEL);
                    if(data == 0x0){
                            printk(KERN_WARNING "%s: Failed to allocate
memory for copy\n", dev->name);
                            ret = -ENOMEM;



^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 1 potential double unlock error
  2003-03-23 23:10   ` Junfeng Yang
  2003-03-24  0:24     ` [CHECKER] 63 potential calling blocking functions with locks held errors Junfeng Yang
@ 2003-03-24  0:29     ` Junfeng Yang
  2003-03-24  9:07     ` [CHECKER] potential dereference of user pointer errors Jaroslav Kysela
  2 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-03-24  0:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc


It's in net/3c505.c. Please help us to confirm or clarify. Thanks.

Junfeng

[BUG] if timeouts, will double unlock

/home/junfeng/linux-2.5.63/drivers/net/3c505.c:467:send_pcb:
ERROR:LOCK:432:467:double unlock &(*adapter).lock[TRANS: &(*adapter).lock,
locked->unlocked, /home/junfeng/linux-2.5.63/drivers/net/3c505.c,
send_pcb, 446]

	set_hsf(dev, 0);

	if (send_pcb_slow(dev->base_addr, pcb->command))
		goto abort;

Start --->
	spin_lock_irqsave(&adapter->lock, flags);

	... DELETED 29 lines ...


	if (elp_debug >= 1)
		printk("%s: timeout waiting for PCB acknowledge (status
%02x)\n", dev->name, inb_status(dev->base_addr));

      sti_abort:
Error --->
	spin_unlock_irqrestore(&adapter->lock, flags);
      abort:
	adapter->send_pcb_semaphore = 0;
	return FALSE;



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-23 23:10   ` Junfeng Yang
  2003-03-24  0:24     ` [CHECKER] 63 potential calling blocking functions with locks held errors Junfeng Yang
  2003-03-24  0:29     ` [CHECKER] 1 potential double unlock error Junfeng Yang
@ 2003-03-24  9:07     ` Jaroslav Kysela
  2 siblings, 0 replies; 43+ messages in thread
From: Jaroslav Kysela @ 2003-03-24  9:07 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc

On Sun, 23 Mar 2003, Junfeng Yang wrote:

> ---------------------------------------------------------
> [BUG] snd_sb_csp_ioctl -> snd_sb_csp_riff_load ->snd_sb_csp_load.
> /home/junfeng/linux-2.5.63/sound/isa/sb/sb16_csp.c:647:snd_sb_csp_load:
> ERROR:tainted:647:647:deferencing buf++ tainted by
> [dist=-1][(null)][TRANS: buf++, $stop$->tainted,
> /home/junfeng/linux-2.5.63/sound/isa/sb/sb16_csp.c, snd_sb_csp_load, 647]
> 
> 		}
> 		kfree (_kbuf);
> 	} else {
> 		/* load from kernel space */
> 		while (size--) {
> 
> Error --->
> 			if (!snd_sbdsp_command(p->chip, *buf++))
> 				goto __fail;
> 		}
> 	}

This code is ok. The source (user or kernel space) is checked a few lines 
before.

> ---------------------------------------------------------
> [BUG] copy function can take tainted inputs
> /home/junfeng/linux-2.5.63/sound/pci/es1938.c:833:snd_es1938_capture_copy:
> ERROR:tainted:835:833: passing dst into deref cal __constant_memcpy
> [dist=1][thru snd_pcm_ops_t:copy
> /home/junfeng/linux-2.5.63/sound/pci/cmipci.c:snd_cmipci_ac3_copy:parm3
> copy_from_user:parm1][START: dst, unknown->tainted,
> /home/junfeng/linux-2.5.63/sound/pci/es1938.c, snd_es1938_capture_copy,
> 833]
> 
> 	es1938_t *chip = snd_pcm_substream_chip(substream);
> 	pos <<= chip->dma1_shift;
> 	count <<= chip->dma1_shift;
> 	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
> 	if (pos + count < chip->dma1_size)
> Error --->
> 		memcpy(dst, runtime->dma_area + pos + 1, count);
> 	else {
> Start --->
> 		memcpy(dst, runtime->dma_area + pos + 1, count - 1);
> 		((unsigned char *)dst)[count - 1] = runtime->dma_area[0];
> 	}
> 	return 0;

Fixed. Replaced memcpy with copy_to_user(). Thanks.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs


^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 8 potential calling blocking kmalloc(GFP_KERNEL) with locks held errors
  2003-03-24  0:24     ` [CHECKER] 63 potential calling blocking functions with locks held errors Junfeng Yang
@ 2003-03-24 12:35       ` Junfeng Yang
  0 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-03-24 12:35 UTC (permalink / raw)
  To: linux-kernel; +Cc: mc


Hi,

Enclosed are 8 potential deadlock errors. The checker warns when it sees a
call to kmalloc(*, GFP_KERNEL) with locks held. Any comments will be
greatly appereciated.

-Junfeng

[BUG]
/home/junfeng/linux-2.5.63/drivers/atm/iphase.c:3221:ia_init_one:
ERROR:BLOCK:3220:3221:calling blocking fn ia_start with lock
&(*iadev).misc_lock held [ia_start calling rx_init calling kmalloc][START:
&(*iadev).misc_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/atm/iphase.c, ia_init_one, 3220]

	ia_dev[iadev_count] = iadev;
	_ia_dev[iadev_count] = dev;
	iadev_count++;
	spin_lock_init(&iadev->misc_lock);
	/* First fixes first. I don't want to think about this now. */
Start --->
	spin_lock_irqsave(&iadev->misc_lock, flags);
Error --->
	if (ia_init(dev) || ia_start(dev)) {
		IF_INIT(printk("IA register failed!\n");)
		iadev_count--;
		ia_dev[iadev_count] = NULL;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/ieee1394/sbp2.c:684:sbp2util_create_command_orb_pool:
ERROR:BLOCK:682:684:calling blocking fn kmalloc with lock
&(*scsi_id).sbp2_command_orb_lock held [kmalloc][START:
&(*scsi_id).sbp2_command_orb_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/ieee1394/sbp2.c,
sbp2util_create_command_orb_pool, 682]

	unsigned long flags, orbs;
	struct sbp2_command_info *command;

	orbs = sbp2_serialize_io ? 2 : SBP2_MAX_COMMAND_ORBS;

Start --->
	spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
	for (i = 0; i < orbs; i++) {
Error --->
		command = (struct sbp2_command_info *)
		    kmalloc(sizeof(struct sbp2_command_info), GFP_KERNEL);
		if (!command) {

spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/scsi/dpt_i2o.c:1705:adpt_i2o_passthru:
ERROR:BLOCK:1699:1705:calling blocking fn adpt_i2o_post_wait with lock
(*(*pHba).host).host_lock held [adpt_i2o_post_wait calling kmalloc][START:
(*(*pHba).host).host_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/scsi/dpt_i2o.c, adpt_i2o_passthru,
1699]

			sg[i].addr_bus = (u32)virt_to_bus((void*)p);
		}
	}

	do {
Start --->
		spin_lock_irqsave(pHba->host->host_lock, flags);
		// This state stops any new commands from enterring the
		// controller while processing the ioctl
//		pHba->state |= DPTI_STATE_IOCTL;
//		We can't set this now - The scsi subsystem sets
host_blocked and
//		the queue empties and stops.  We need a way to restart the
queue
Error --->
		rcode = adpt_i2o_post_wait(pHba, msg, size, FOREVER);
//		pHba->state &= ~DPTI_STATE_IOCTL;
		spin_unlock_irqrestore(pHba->host->host_lock, flags);
	} while(rcode == -ETIMEDOUT);
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/ieee1394/ohci1394.c:994:ohci_devctl:
ERROR:BLOCK:982:994:calling blocking fn alloc_dma_rcv_ctx with lock
&(*ohci).IR_channel_lock held [alloc_dma_rcv_ctx calling kmalloc][START:
&(*ohci).IR_channel_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/ieee1394/ohci1394.c, ohci_devctl, 982]

			return -EFAULT;
		}

		mask = (u64)0x1<<arg;

Start --->
                spin_lock_irqsave(&ohci->IR_channel_lock, flags);

		if (ohci->ISO_channel_usage & mask) {
			PRINT(KERN_ERR, ohci->id,
			      "%s: IS0 listen channel %d is already used",
			      __FUNCTION__, arg);
			spin_unlock_irqrestore(&ohci->IR_channel_lock,
flags);
			return -EFAULT;
		}

		/* activate the legacy IR context */
		if(ohci->ir_legacy_context.ohci == NULL) {
Error --->
			if(alloc_dma_rcv_ctx(ohci,
&ohci->ir_legacy_context,
					     DMA_CTX_ISO, 0, IR_NUM_DESC,
					     IR_BUF_SIZE,
IR_SPLIT_BUF_SIZE,
					     OHCI1394_IsoRcvContextBase) <
0) {
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/isdn/i4l/isdn_common.c:933:register_isdn:
ERROR:BLOCK:918:933:calling blocking fn isdn_add_channels with lock
&drivers_lock held [isdn_add_channels calling kmalloc][START:
&drivers_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/isdn/i4l/isdn_common.c, register_isdn,
918]

	drv->fi.state = ST_DRV_NULL;
	drv->fi.debug = 1;
	drv->fi.userdata = drv;
	drv->fi.printdebug = drv_debug;

Start --->
	spin_lock_irqsave(&drivers_lock, flags);

	... DELETED 9 lines ...


	if (__isdn_drv_lookup(iif->id) >= 0)
		goto fail_unlock;

	strcpy(drv->id, iif->id);
Error --->
	if (isdn_add_channels(drv, iif->channels))
		goto fail_unlock;

	drv->di = drvidx;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-2.5.63/drivers/net/wan/comx-hw-munich.c:1458:MUNICH_open:
ERROR:BLOCK:1438:1458:calling blocking fn kmalloc with lock &mister_lock
held [kmalloc][START: &mister_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/net/wan/comx-hw-munich.c, MUNICH_open,
1438]

	printk("MUNICH_open: no %s board with boardnum = %d\n",
	       ch->hardware->name, hw->boardnum);
	return -ENODEV;
    }

Start --->
    spin_lock_irqsave(&mister_lock, flags);

	... DELETED 14 lines ...

	    board->histogram[i] = 0;

	board->lineup = 0;

	/* Allocate CCB: */
Error --->
        board->ccb = kmalloc(sizeof(munich_ccb_t), GFP_KERNEL);
	if (board->ccb == NULL)
	{
	    spin_unlock_irqrestore(&mister_lock, flags);
---------------------------------------------------------
[BUG] interesting things to check: calling kmalloc with GFP_KERNEL and
then with GFP_ATOMIC.
/home/junfeng/linux-2.5.63/net/rose/rose_route.c:112:rose_add_node:
ERROR:BLOCK:64:112:calling blocking fn kmalloc with lock
&rose_neigh_list_lock held [kmalloc][START: &rose_neigh_list_lock,
unknown->locked, /home/junfeng/linux-2.5.63/net/rose/rose_route.c,
rose_add_node, 64]

	struct rose_node  *rose_node, *rose_tmpn, *rose_tmpp;
	struct rose_neigh *rose_neigh;
	int i, res = 0;

	spin_lock_bh(&rose_node_list_lock);
Start --->
	spin_lock_bh(&rose_neigh_list_lock);

	... DELETED 42 lines ...


		init_timer(&rose_neigh->ftimer);
		init_timer(&rose_neigh->t0timer);

		if (rose_route->ndigis != 0) {
Error --->
			if ((rose_neigh->digipeat =
kmalloc(sizeof(ax25_digi), GFP_KERNEL)) == NULL) {
				kfree(rose_neigh);
				res = -ENOMEM;
				goto out;
---------------------------------------------------------
[BUG] such a huge ioctl function ...
/home/junfeng/linux-2.5.63/drivers/net/wan/lmc/lmc_main.c:500:lmc_ioctl:
ERROR:BLOCK:160:500:calling blocking fn kmalloc with lock &(*sc).lmc_lock
held [kmalloc][START: &(*sc).lmc_lock, unknown->locked,
/home/junfeng/linux-2.5.63/drivers/net/wan/lmc/lmc_main.c, lmc_ioctl, 160]


    /*
     * Most functions mess with the structure
     * Disable interrupts while we do the polling
     */
Start --->
    spin_lock_irqsave(&sc->lmc_lock, flags);

	... DELETED 334 lines ...

                    if(xc.data == 0x0){
                            ret = -EINVAL;
                            break;
                    }

Error --->
                    data = kmalloc(xc.len, GFP_KERNEL);
                    if(data == 0x0){
                            printk(KERN_WARNING "%s: Failed to allocate
memory for copy\n", dev->name);
                            ret = -ENOMEM;





^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
                     ` (5 preceding siblings ...)
  2003-03-23 23:10   ` Junfeng Yang
@ 2003-03-24 22:28   ` Raja R Harinath
  2003-03-25  0:44     ` David S. Miller
  6 siblings, 1 reply; 43+ messages in thread
From: Raja R Harinath @ 2003-03-24 22:28 UTC (permalink / raw)
  To: linux-kernel

Hi,

Junfeng Yang <yjf@stanford.edu> writes:

> We are the the Stanford Checker team that constantly send error
> reports to the linux kernel mailing list. Enclosed are 10
> dereference of user pointer warnings catched by our checker. We
> started by marking the second parameter of copy_from_user (to, from,
> len), the first parameter of copy_to_user (to, from, len), and all
> parameters of the sys_* functions as tainted, then propogated the
> tainted annotations along call chains. If two functions get assigned
> to the same structure field, we propogate the tainted annotations
> between them, too. An example of such propogation is the warning
> about drivers/media/video/cpia.c::cpia_write_proc and
> drivers/media/usb/media/vicam.c. The error message uses "thru
> struct_name:field_name" to represent such propogations.

Can't pointer-dereference errors be handled directly by any C
compiler.  Is CHECKER necessary for this.  Use an incomplete
struct pointer and the compiler will complain on all dereferences.

Something like

  /* struct user_space should never be defined.  */
  typedef struct user_space user_space;

  int copy_to_user (user_space *to, char *from, size_t len);
  int copy_from_user (char *to, user_space *from, size_t len);
  /* ... */

  #define TREAT_AS_USER_SPACE_POINTER(p) \
            ({                                  \
              BUG_ON(get_fs() != get_gs());     \
              (user_space *)p;                  \
            })

- Hari
-- 
Raja R Harinath ------------------------------ harinath@cs.umn.edu


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-24 22:28   ` Raja R Harinath
@ 2003-03-25  0:44     ` David S. Miller
  2003-03-25 18:52       ` Raja R Harinath
  0 siblings, 1 reply; 43+ messages in thread
From: David S. Miller @ 2003-03-25  0:44 UTC (permalink / raw)
  To: Raja R Harinath; +Cc: linux-kernel

On Mon, 2003-03-24 at 14:28, Raja R Harinath wrote:
> Something like
> 
>   /* struct user_space should never be defined.  */
>   typedef struct user_space user_space;
> 
>   int copy_to_user (user_space *to, char *from, size_t len);
>   int copy_from_user (char *to, user_space *from, size_t len);
>   /* ... */
> 
>   #define TREAT_AS_USER_SPACE_POINTER(p) \
>             ({                                  \
>               BUG_ON(get_fs() != get_gs());     \
>               (user_space *)p;                  \
>             })

A great idea, we'd need to use this struct user_space at every
system call, and it doesn't work to well when pointers are
embedded inside of a structure.

This is why a GCC attribute of some sort would be more useful.
But I don't see the GCC folks offering a user-definable attribute
+ checking system any time soon.

-- 
David S. Miller <davem@redhat.com>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-25  0:44     ` David S. Miller
@ 2003-03-25 18:52       ` Raja R Harinath
  0 siblings, 0 replies; 43+ messages in thread
From: Raja R Harinath @ 2003-03-25 18:52 UTC (permalink / raw)
  To: linux-kernel

Hi,

"David S. Miller" <davem@redhat.com> writes:

> On Mon, 2003-03-24 at 14:28, Raja R Harinath wrote:
>> Something like
>> 
>>   /* struct user_space should never be defined.  */
>>   typedef struct user_space user_space;
>> 
>>   int copy_to_user (user_space *to, char *from, size_t len);
>>   int copy_from_user (char *to, user_space *from, size_t len);
>>   /* ... */
>> 
>>   #define TREAT_AS_USER_SPACE_POINTER(p) \
>>             ({                                  \
>>               BUG_ON(get_fs() != get_gs());     \
>>               (user_space *)p;                  \
>>             })
>
> A great idea, we'd need to use this struct user_space at every
> system call, 

Which is good, I think.  It annotates the source.

> and it doesn't work to well when pointers are embedded inside of a
> structure.

I don't understand this point.  'struct user_space' will never be
defined.  The C compiler ensure that 'struct user_space *p' cannot be
dereferenced then, since it will be a pointer to an incomplete type.

I don't see any difference between

  struct foo { char *p; };

and 

  struct foo { user_space *p; };

in terms of the code generated.  (On second reading, I don't think
this was your point though).

- Hari
-- 
Raja R Harinath ------------------------------ harinath@cs.umn.edu


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-21 23:55   ` Chris Wright
@ 2003-03-27  8:07     ` Jan Kasprzak
  2003-03-27 17:10       ` Chris Wright
  0 siblings, 1 reply; 43+ messages in thread
From: Jan Kasprzak @ 2003-03-27  8:07 UTC (permalink / raw)
  To: Junfeng Yang, linux-kernel, mc

Chris Wright wrote:
: Both cosa_readmem and cosa_download don't seem to do any validation of
: the user supplied ptr at all before dereferncing it in get_user.  And
: it'd make sense to use 'code' in cosa_reamdme (as in cosa_download)
: instead of 'd->code'.  Jan, does this look OK?

	Yes, you are right. I've missed this. However, it is not
as bad as it looks like, because you need the CAP_SYS_RAWIO to
exploit this. I agree this patch should be applied.

: ===== drivers/net/wan/cosa.c 1.17 vs edited =====
: --- 1.17/drivers/net/wan/cosa.c	Mon Jan 13 17:11:59 2003
: +++ edited/drivers/net/wan/cosa.c	Fri Mar 21 15:53:38 2003
: @@ -1057,7 +1057,8 @@
:  		return -EPERM;
:  	}
:  
: -	if (get_user(addr, &(d->addr)) ||
: +	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
: +	    __get_user(addr, &(d->addr)) ||
:  	    __get_user(len, &(d->len)) ||
:  	    __get_user(code, &(d->code)))
:  		return -EFAULT;
: @@ -1098,7 +1099,8 @@
:  		return -EPERM;
:  	}
:  
: -	if (get_user(addr, &(d->addr)) ||
: +	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
: +	    __get_user(addr, &(d->addr)) ||
:  	    __get_user(len, &(d->len)) ||
:  	    __get_user(code, &(d->code)))
:  		return -EFAULT;
: @@ -1106,7 +1108,7 @@
:  	/* If something fails, force the user to reset the card */
:  	cosa->firmware_status &= ~COSA_FW_RESET;
:  
: -	if ((i=readmem(cosa, d->code, len, addr)) < 0) {
: +	if ((i=readmem(cosa, code, len, addr)) < 0) {
:  		printk(KERN_NOTICE "cosa%d: reading memory failed: %d\n",
:  			cosa->num, i);
:  		return -EIO;

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
|-- If you start doing things because you hate others and want to screw  --|
|-- them over the end result is bad.   --Linus Torvalds to the BBC News  --|

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] potential dereference of user pointer errors
  2003-03-27  8:07     ` Jan Kasprzak
@ 2003-03-27 17:10       ` Chris Wright
  2003-04-21  7:49         ` [CHECKER] Help Needed! Junfeng Yang
  0 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-03-27 17:10 UTC (permalink / raw)
  To: Jan Kasprzak; +Cc: Junfeng Yang, linux-kernel, mc

* Jan Kasprzak (kas@informatics.muni.cz) wrote:
> Chris Wright wrote:
> : Both cosa_readmem and cosa_download don't seem to do any validation of
> : the user supplied ptr at all before dereferncing it in get_user.  And
> : it'd make sense to use 'code' in cosa_reamdme (as in cosa_download)
> : instead of 'd->code'.  Jan, does this look OK?
> 
> 	Yes, you are right. I've missed this. However, it is not
> as bad as it looks like, because you need the CAP_SYS_RAWIO to
> exploit this. I agree this patch should be applied.

Thanks for the confirmation.
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER]  Help Needed!
  2003-03-27 17:10       ` Chris Wright
@ 2003-04-21  7:49         ` Junfeng Yang
  2003-04-21 21:26           ` Chris Wright
  0 siblings, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-04-21  7:49 UTC (permalink / raw)
  To: linux-kernel, Chris Wright

Hi,

We run a checker that warns about derefence of user-pointer errors on
linux-2.5.63 and got the following warning message in
init/do_mounts.c:create_dev, which needs clarifications.

---------------------------------------------------------
/home/junfeng/linux-2.5.63/init/do_mounts.c:437:create_dev: ERROR:TAINTED:442:437:dereferencing tainted ptr 'devfs_name'

	sys_unlink(name);
	if (!do_devfs)
		return sys_mknod(name, S_IFBLK|0600, dev);

Error--->
	if (devfs_name && devfs_name[0]) {
		if (strncmp(devfs_name, "/dev/", 5) == 0)
			devfs_name += 5;
		sprintf(path, "/dev/%s", devfs_name);
		if (sys_access(path, 0) == 0)
Start --->
			return sys_symlink(devfs_name, name);
	}
	if (!dev)
		return -1;


It seems to us that create_dev can only be called at boot time (the
"__init" attribute), so devfs_name must be an untainted kernel pointer.
The warning on line 437 isn't a real error.

However, this pointer is finally passed into strncpy_from_user through the
call chain [ sys_symlink (devfs_name, name) --> getname (oldname) -->
do_getname(filename, _) --> strncpy_from_user (_, filename, _)].  Is it
okay to call *_from_user functions with the second arguements untainted?
What will access_ok(VERIFY_READ, src, 1) return?

As usual, any response will be greatly apppreciated.  Thanks a lot!

-Junfeng


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER]  Help Needed!
  2003-04-21  7:49         ` [CHECKER] Help Needed! Junfeng Yang
@ 2003-04-21 21:26           ` Chris Wright
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
  0 siblings, 1 reply; 43+ messages in thread
From: Chris Wright @ 2003-04-21 21:26 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel

* Junfeng Yang (yjf@stanford.edu) wrote:
> 
> It seems to us that create_dev can only be called at boot time (the
> "__init" attribute), so devfs_name must be an untainted kernel pointer.
> The warning on line 437 isn't a real error.

Yes.

> However, this pointer is finally passed into strncpy_from_user through the
> call chain [ sys_symlink (devfs_name, name) --> getname (oldname) -->
> do_getname(filename, _) --> strncpy_from_user (_, filename, _)].  Is it
> okay to call *_from_user functions with the second arguements untainted?
> What will access_ok(VERIFY_READ, src, 1) return?

This checks against the current addr_limit which depends on the context.
KERNEL_DS lets this check succeed.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-21 21:26           ` Chris Wright
@ 2003-04-26  2:18             ` Junfeng Yang
  2003-04-27  9:26               ` James Morris
                                 ` (5 more replies)
  0 siblings, 6 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-26  2:18 UTC (permalink / raw)
  To: linux-kernel, Chris Wright; +Cc: mc

---------------------------------------------------------
Hi,

I've submitted 8 such bugs several weeks before and thanks to Chris and
other guys to fix most of them. Here are another 30 similar bugs. I got a
warning in net/ipv4, which must be that I'm missing something, so please
clarify. Also it seems I'm missing some assumptions for those video
drivers.

As usual, any responses will be appreciated. Thanks!

Where bugs occur:

net/ipv4/ip_sockglue.c
drivers/i2c/i2c-dev.c
drivers/ieee1394/video1394.c
drivers/isdn/eicon/linchr.c
drivers/media/dvb/av7110/av7110_ir.c
drivers/media/radio/radio-cadet.c
drivers/media/video/bw-qcam.c
drivers/media/video/cpia.c
drivers/media/video/zoran_procfs.c
drivers/media/video/zr36120.c
drivers/pnp/pnpbios/proc.c
drivers/scsi/aacraid/commctrl.c
drivers/usb/image/mdc800.c
drivers/usb/media/vicam.c
drivers/usb/serial/empeg.c
drivers/usb/serial/io_edgeport.c
drivers/usb/serial/ipaq.c
drivers/usb/serial/keyspan.c
drivers/video/sis/sis_main.c
sound/oss/awe_wave.c
sound/oss/cmpci.c
sound/oss/mpu401.c
sound/pci/es1938.c


[UNKNOWN] maybe? it is in ipv4 though. ip_cmsg_send -> ip_options_get
implies CMSG_DATA(cmsg) is tainted. It could be that the taintedness of CMSG_DATA(cmsg) depends on the case branch

/home/junfeng/linux-tainted/net/ipv4/ip_sockglue.c:168:ip_cmsg_send: ERROR:TAINTED:168:168: dereferencing tainted ptr 'info' [Callstack: /home/junfeng/linux-tainted/net/ipv4/ip_options.c:518:ip_cmsg_send()]

		{
			struct in_pktinfo *info;
			if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct in_pktinfo)))
				return -EINVAL;
			info = (struct in_pktinfo *)CMSG_DATA(cmsg);

Error --->
			ipc->oif = info->ipi_ifindex;
			ipc->addr = info->ipi_spec_dst.s_addr;
			break;
		}
---------------------------------------------------------
[BUG] buf is tainted. can print out arbitrary kernel data
/home/junfeng/linux-tainted/drivers/usb/serial/empeg.c:225:empeg_write: ERROR:TAINTED:225:225: passing tainted ptr 'buf' to usb_serial_debug_data [Callstack: /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:empeg_write((tainted 2))]

	int bytes_sent = 0;
	int transfer_size;

	dbg("%s - port %d", __FUNCTION__, port->number);


Error --->
	usb_serial_debug_data (__FILE__, __FUNCTION__, count, buf);

	while (count > 0) {

---------------------------------------------------------
[BUG] can print out arbitrary kernel data
/home/junfeng/linux-tainted/drivers/usb/serial/ipaq.c:371:ipaq_write: ERROR:TAINTED:371:371: passing tainted ptr 'buf' to usb_serial_debug_data [Callstack: /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:ipaq_write((tainted 2))]

	int			bytes_sent = 0;
	int			transfer_size;

	dbg("%s - port %d", __FUNCTION__, port->number);


Error --->
	usb_serial_debug_data(__FILE__, __FUNCTION__, count, buf);

	while (count > 0) {
		transfer_size = min(count, PACKET_SIZE);
---------------------------------------------------------
[BUG] can print out arbitrary kernel data
/home/junfeng/linux-tainted/drivers/usb/serial/keyspan.c:328:keyspan_write: ERROR:TAINTED:328:328: dereferencing tainted ptr 'buf' [Callstack: ]


	p_priv = usb_get_serial_port_data(port);
	d_details = p_priv->device_details;

	dbg("%s - for port %d (%d chars [%x]), flip=%d",

Error --->
	    __FUNCTION__, port->number, count, buf[0], p_priv->out_flip);

	for (left = count; left > 0; left -= todo) {
		todo = left;
---------------------------------------------------------
[BUG] pointer (rdwr_arg.msgs[i]) points to user space
/home/junfeng/linux-tainted/drivers/i2c/i2c-dev.c:230:i2cdev_ioctl: ERROR:TAINTED:230:230: dereferencing tainted ptr 'rdwr_arg.msgs + i * 12' [Callstack: ]

			if(rdwr_pa[i].buf == NULL)
			{
				res = -ENOMEM;
				break;
			}

Error --->
			if(copy_from_user(rdwr_pa[i].buf,
				rdwr_arg.msgs[i].buf,
				rdwr_pa[i].len))
			{
---------------------------------------------------------
[BUG] file_operations.read. even the parm itself has a name pUserBuffer
/home/junfeng/linux-tainted/drivers/isdn/eicon/linchr.c:166:do_read: ERROR:TAINTED:166:166: passing tainted ptr 'pClientLogBuffer' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:do_read((tainted 1))]


	pHeadItem = (klog_t *) DivasLogFifoRead();

	if (pHeadItem)
	{

Error --->
		memcpy(pClientLogBuffer, pHeadItem, sizeof(klog_t));
		kfree(pHeadItem);
		return sizeof(klog_t);
	}
---------------------------------------------------------
[BUG] file_operations.write -> cm_write -> trans_ac3. write can take tainted. write can take tainted inputs.
/home/junfeng/linux-tainted/sound/oss/cmpci.c:593:trans_ac3: ERROR:TAINTED:593:593: dereferencing tainted ptr 'src' [Callstack: /home/junfeng/linux-tainted/fs/read_write.c:307:vfs_write((tainted 1)(tainted 2)) -> /home/junfeng/linux-tainted/fs/read_write.c:241:cm_write((tainted 1)(tainted 2)) -> /home/junfeng/linux-tainted/sound/oss/cmpci.c:1662:trans_ac3((tainted 2))]

	unsigned long data;
	unsigned long *dst = (unsigned long *) dest;
	unsigned short *src = (unsigned short *)source;

	do {

Error --->
		data = (unsigned long) *src++;
		data <<= 12;			// ok for 16-bit data
		if (s->spdif_counter == 2 || s->spdif_counter == 3)
			data |= 0x40000000;	// indicate AC-3 raw data
---------------------------------------------------------
[BUG] in ioctl function. v (==arg) is not checked at all. can write arbitrary data to the kernel. on the same case branch, copy_to_user is called on arg
/home/junfeng/linux-tainted/drivers/media/radio/radio-cadet.c:397:cadet_do_ioctl: ERROR:TAINTED:397:397: dereferencing tainted ptr 'v' [Callstack: ]

	{
		case VIDIOCGCAP:
		{
			struct video_capability *v = arg;
			memset(v,0,sizeof(*v));

Error --->
			v->type=VID_TYPE_TUNER;
			v->channels=2;
			v->audios=1;
			strcpy(v->name, "ADS Cadet");
---------------------------------------------------------
[BUG] no check in sis_main.c at all. can write arbitrary data to kernel
/home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2476:sisfb_ioctl: ERROR:TAINTED:2476:2476: dereferencing tainted ptr 'x' [Callstack: /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted 3))]

		}
	   case FBIOPUT_MODEINFO:
		{
			struct mode_info *x = (struct mode_info *)arg;


Error --->
			ivideo.video_bpp      = x->bpp;
			ivideo.video_width    = x->xres;
			ivideo.video_height   = x->yres;
			ivideo.video_vwidth   = x->v_xres;
---------------------------------------------------------
[BUG] on VIDIOCGCAPUTRE and VIDIOCSCAPUTRE branches copy_*_user functions are called. on other branches not
/home/junfeng/linux-tainted/drivers/media/video/cpia.c:3432:cpia_do_ioctl: ERROR:TAINTED:3432:3432: dereferencing tainted ptr 'vp' [Callstack: ]

		DBG("VIDIOCSPICT\n");

		/* check validity */
		DBG("palette: %d\n", vp->palette);
		DBG("depth: %d\n", vp->depth);

Error --->
		if (!valid_mode(vp->palette, vp->depth)) {
			retval = -EINVAL;
			break;
		}
---------------------------------------------------------
[BUG] sisfb_ioctl->sis_dispinfo. drivers/video/sstfb_ioctl assumes "arg" is tainted. both of them are assigned to fb_ops.fb_ioctl
/home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:276:sis_dispinfo: ERROR:TAINTED:276:276: dereferencing tainted ptr 'rec' [Callstack: /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted 3)) -> /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2488:sis_dispinfo((tainted 0))]

	gly->ngmask = size;
}

void sis_dispinfo(struct ap_data *rec)
{

Error --->
	rec->minfo.bpp    = ivideo.video_bpp;
	rec->minfo.xres   = ivideo.video_width;
	rec->minfo.yres   = ivideo.video_height;
	rec->minfo.v_xres = ivideo.video_vwidth;
---------------------------------------------------------
[BUG] mdc800_device_read is assigned to file_operations.read, which can take tainted inputs
/home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:750:mdc800_device_read: ERROR:TAINTED:750:750: passing tainted ptr 'ptr' to __memcpy [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:mdc800_device_read((tainted 1))]

			}
		}
		else
		{
			/* memcpy Bytes */

Error --->
			memcpy (ptr, &mdc800->out [mdc800->out_ptr], sts);
			ptr+=sts;
			left-=sts;
			mdc800->out_ptr+=sts;
---------------------------------------------------------
[BUG] call usb_serial_debug_data on data
/home/junfeng/linux-tainted/drivers/usb/serial/io_edgeport.c:1381:edge_write: ERROR:TAINTED:1381:1381: passing tainted ptr 'data' to usb_serial_debug_data [Callstack: ]

		fifo->head  += secondhalf;
		// No need to check for wrap since we can not get to end of fifo in this part
	}

	if (copySize) {

Error --->
		usb_serial_debug_data (__FILE__, __FUNCTION__, copySize, data);
	}

	send_more_port_data((struct edgeport_serial *)usb_get_serial_data(port->serial), edge_port);
---------------------------------------------------------
[BUG] proc_dir_entry.read_proc can take tainted inputs
/home/junfeng/linux-tainted/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain: ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:vicam_write_proc_gain((tainted 1))]

static int vicam_write_proc_gain(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->gain = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] midi_synth.c:midi_synth_ioctl does __copy_to_user(not copy_to_user?). it could be that there are some assumptions about the devices.
/home/junfeng/linux-tainted/sound/oss/mpu401.c:792:mpu_synth_ioctl: ERROR:TAINTED:792:792: passing tainted ptr 'arg' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:mpu_synth_ioctl((tainted 2))]


	switch (cmd)
	{

		case SNDCTL_SYNTH_INFO:

Error --->
			memcpy((&((char *) arg)[0]), (char *) &mpu_synth_info[midi_dev], sizeof(struct synth_info));
			return 0;

		case SNDCTL_SYNTH_MEMAVL:
---------------------------------------------------------
[BUG] similar errors
/home/junfeng/linux-tainted/drivers/media/radio/radio-cadet.c:399:cadet_do_ioctl: ERROR:TAINTED:399:399: dereferencing tainted ptr 'v' [Callstack: ]

		{
			struct video_capability *v = arg;
			memset(v,0,sizeof(*v));
			v->type=VID_TYPE_TUNER;
			v->channels=2;

Error --->
			v->audios=1;
			strcpy(v->name, "ADS Cadet");
			return 0;
		}
---------------------------------------------------------
[BUG] ioctl
/home/junfeng/linux-tainted/drivers/usb/media/vicam.c:617:vicam_ioctl: ERROR:TAINTED:617:617: dereferencing tainted ptr 'vp' [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:vicam_ioctl((tainted 3))]

			struct video_picture *vp = (struct video_picture *) arg;

			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
			    vp->palette);


Error --->
			cam->gain = vp->brightness >> 8;

			if (vp->depth != 24
			    || vp->palette != VIDEO_PALETTE_RGB24)
---------------------------------------------------------
[BUG] copy_to_user is called on case VIDIOCGCHAN
/home/junfeng/linux-tainted/drivers/media/video/bw-qcam.c:763:qcam_do_ioctl: ERROR:TAINTED:763:763: dereferencing tainted ptr 'p' [Callstack: ]

			return 0;
		}
		case VIDIOCGPICT:
		{
			struct video_picture *p = arg;

Error --->
			p->colour=0x8000;
			p->hue=0x8000;
			p->brightness=qcam->brightness<<8;
			p->contrast=qcam->contrast<<8;
---------------------------------------------------------
[BUG]
/home/junfeng/linux-tainted/drivers/media/video/bw-qcam.c:723:qcam_do_ioctl: ERROR:TAINTED:723:723: dereferencing tainted ptr 'v' [Callstack: ]

		case VIDIOCGCHAN:
		{
			struct video_channel *v = arg;
			if(v->channel!=0)
				return -EINVAL;

Error --->
			v->flags=0;
			v->tuners=0;
			/* Good question.. its composite or SVHS so.. */
			v->type = VIDEO_TYPE_CAMERA;
---------------------------------------------------------
[BUG] file_opetations.ioctl -> aac_cfg_ioctl -> aac_do_ioctl -> close_getadapter_fib -> aac_close_fib_context. also, check_revision calls copy_to_user on "arg"
/home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:277:aac_close_fib_context: ERROR:TAINTED:277:277: dereferencing tainted ptr 'fibctx' [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:aac_cfg_ioctl((tainted 3)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/linit.c:671:aac_do_ioctl((tainted 2)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:421:close_getadapter_fib((tainted 1)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:348:aac_close_fib_context((tainted 1))]

	while (!list_empty(&fibctx->fibs)) {
		struct list_head * entry;
		/*
		 *	Pull the next fib from the fibs
		 */

Error --->
		entry = fibctx->fibs.next;
		list_del(entry);
		fib = list_entry(entry, struct hw_fib, header.FibLinks);
		fibctx->count--;
---------------------------------------------------------
[BUG] zoran_read and vbi_read are both assigned to video_device.read, while zoran_read assumes "buf" is tainted
/home/junfeng/linux-tainted/drivers/media/video/zr36120.c:1698:vbi_read: ERROR:TAINTED:1698:1698: dereferencing tainted ptr 'optr' [Callstack: /home/junfeng/linux-tainted/drivers/media/video/zr36120.c:934:vbi_read((tainted 1))]

		{
			/* copy to doubled data to userland */
			for (x=0; optr+1<eptr && x<-done->w; x++)
			{
				unsigned char a = iptr[x*2];

Error --->
				*optr++ = a;
				*optr++ = a;
			}
			/* and clear the rest of the line */
---------------------------------------------------------
[BUG] video1394_ioctl's param arg is tainted -> initialize_dma_it_prg_var_packet_queue's param packet_sizes is also tainted
/home/junfeng/linux-tainted/drivers/ieee1394/video1394.c:668:initialize_dma_it_prg_var_packet_queue: ERROR:TAINTED:668:668: dereferencing tainted ptr 'packet_sizes + i * 4' [Callstack: /home/junfeng/linux-tainted/drivers/ieee1394/video1394.c:1047:initialize_dma_it_prg_var_packet_queue((tainted 2))]

	for (i = 0; i < d->nb_cmd; i++) {
		unsigned int size;
		if (packet_sizes[i] > d->packet_size) {
			size = d->packet_size;
		} else {

Error --->
			size = packet_sizes[i];
		}
		it_prg[i].data[1] = cpu_to_le32(size << 16);
		it_prg[i].end.control = cpu_to_le32(DMA_CTL_OUTPUT_LAST | DMA_CTL_BRANCH);
---------------------------------------------------------
[BUG] sound/pci/rme9652/rme9652.c:snd_rme9652_capture_copy treats the parm "dst" as tainted and they are both assigned to snd_pcm_ops_t.copy
/home/junfeng/linux-tainted/sound/pci/es1938.c:833:snd_es1938_capture_copy: ERROR:TAINTED:833:833: passing tainted ptr 'dst' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/pci/rme9652/rme9652.c:2010:snd_es1938_capture_copy((tainted 3))]

	es1938_t *chip = snd_pcm_substream_chip(substream);
	pos <<= chip->dma1_shift;
	count <<= chip->dma1_shift;
	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
	if (pos + count < chip->dma1_size)

Error --->
		memcpy(dst, runtime->dma_area + pos + 1, count);
	else {
		memcpy(dst, runtime->dma_area + pos + 1, count - 1);
		((unsigned char *)dst)[count - 1] = runtime->dma_area[0];
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc
/home/junfeng/linux-tainted/drivers/media/video/zoran_procfs.c:122:zoran_write_proc: ERROR:TAINTED:122:122: passing tainted ptr 'buffer' to __memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:zoran_write_proc((tainted 1))]

	string = sp = vmalloc(count + 1);
	if (!string) {
		printk(KERN_ERR "%s: write_proc: can not allocate memory\n", zr->name);
		return -ENOMEM;
	}

Error --->
	memcpy(string, buffer, count);
	string[count] = 0;
	DEBUG2(printk(KERN_INFO "%s: write_proc: name=%s count=%lu data=%x\n", zr->name, file->f_dentry->d_name.name, count, (int) data));
	ldelim = " \t\n";
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc
/home/junfeng/linux-tainted/drivers/pnp/pnpbios/proc.c:190:proc_write_node: ERROR:TAINTED:190:190: passing tainted ptr 'buf' to __memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:proc_write_node((tainted 1))]

	if (!node) return -ENOMEM;
	if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
		return -EIO;
	if (count != node->size - sizeof(struct pnp_bios_node))
		return -EINVAL;

Error --->
	memcpy(node->data, buf, count);
	if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
	    return -EINVAL;
	kfree(node);
---------------------------------------------------------
[BUG] file_operations.write
/home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:805:mdc800_device_write: ERROR:TAINTED:805:805: dereferencing tainted ptr 'buf + i' [Callstack: /home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110.c:3858:mdc800_device_write((tainted 1))]

		}

		/* save command byte */
		if (mdc800->in_count < 8)
		{

Error --->
			mdc800->in[mdc800->in_count]=buf[i];
			mdc800->in_count++;
		}
		else
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs
/home/junfeng/linux-tainted/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter: ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted 1))]

static int vicam_write_proc_shutter(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] av7110_ir_write_proc is assigned to proc_dir_entry.write_proc
/home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc: ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:av7110_ir_write_proc((tainted 1))]

	u32 ir_config;

	if (count < 4 + 256 * sizeof(u16))
		return -EINVAL;


Error --->
	memcpy (&ir_config, buffer, 4);
	memcpy (&key_map, buffer + 4, 256 * sizeof(u16));

	av7110_setup_irc_config (NULL, ir_config);
---------------------------------------------------------
[BUG] pointer (rdwr_arg.msgs[i]) points to user space
/home/junfeng/linux-tainted/drivers/i2c/i2c-dev.c:249:i2cdev_ioctl: ERROR:TAINTED:249:249: dereferencing tainted ptr 'rdwr_arg.msgs + i * 12' [Callstack: ]

		}
		while(i-- > 0)
		{
			if( res>=0 && (rdwr_pa[i].flags & I2C_M_RD))
			{

Error --->
				if(copy_to_user(
					rdwr_arg.msgs[i].buf,
					rdwr_pa[i].buf,
					rdwr_pa[i].len))
---------------------------------------------------------
[BUG] ioctl
/home/junfeng/linux-tainted/sound/oss/awe_wave.c:2049:awe_ioctl: ERROR:TAINTED:2049:2049: passing tainted ptr 'arg' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:awe_ioctl((tainted 2))]

	case SNDCTL_SYNTH_INFO:
		if (playing_mode == AWE_PLAY_DIRECT)
			awe_info.nr_voices = awe_max_voices;
		else
			awe_info.nr_voices = AWE_MAX_CHANNELS;

Error --->
		memcpy((char*)arg, &awe_info, sizeof(awe_info));
		return 0;
		break;



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
@ 2003-04-27  9:26               ` James Morris
  2003-04-28  1:55                 ` Junfeng Yang
  2003-04-27 20:18               ` Nick Holloway
                                 ` (4 subsequent siblings)
  5 siblings, 1 reply; 43+ messages in thread
From: James Morris @ 2003-04-27  9:26 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, Chris Wright, mc

On Fri, 25 Apr 2003, Junfeng Yang wrote:

> Where bugs occur:
> 
> net/ipv4/ip_sockglue.c

Which kernel version is this for?


- James
-- 
James Morris
<jmorris@intercode.com.au>


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
  2003-04-27  9:26               ` James Morris
@ 2003-04-27 20:18               ` Nick Holloway
  2003-04-27 21:14                 ` Junfeng Yang
  2003-04-27 21:29               ` Junfeng Yang
                                 ` (3 subsequent siblings)
  5 siblings, 1 reply; 43+ messages in thread
From: Nick Holloway @ 2003-04-27 20:18 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel

In list.linux-kernel yjf@stanford.edu (Junfeng Yang) wrote:
> [BUG] on VIDIOCGCAPUTRE and VIDIOCSCAPUTRE branches copy_*_user functions are called. on other branches not
> /home/junfeng/linux-tainted/drivers/media/video/cpia.c:3432:cpia_do_ioctl: ERROR:TAINTED:3432:3432: dereferencing tainted ptr 'vp' [Callstack: ]
> 
> 		DBG("VIDIOCSPICT\n");
> 
> 		/* check validity */
> 		DBG("palette: %d\n", vp->palette);
> 		DBG("depth: %d\n", vp->depth);
> 
> Error --->
> 		if (!valid_mode(vp->palette, vp->depth)) {
> 			retval = -EINVAL;
> 			break;
> 		}
> ---------------------------------------------------------

I can't see this.  This code fragment is from cpia_do_ioctl.  This is
never called directly, the entry point is cpia_ioctl, which always passes
ioctl calls to video_usercopy (which calls cpia_do_ioctl through the
supplied function pointer).

In video_usercopy, it calls copy_from_user for an _IOW ioctl (which
VIDIOCSPICT is).  There is certainly no differentiation between the
different ioctl calls made by video_usercopy.

Is there something I have missed?

-- 
 `O O'  | Nick.Holloway@pyrites.org.uk
// ^ \\ | http://www.pyrites.org.uk/

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-27 20:18               ` Nick Holloway
@ 2003-04-27 21:14                 ` Junfeng Yang
  0 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-27 21:14 UTC (permalink / raw)
  To: Nick Holloway; +Cc: linux-kernel, mc

On Sun, 27 Apr 2003, Nick Holloway wrote:

> In list.linux-kernel yjf@stanford.edu (Junfeng Yang) wrote:
> > [BUG] on VIDIOCGCAPUTRE and VIDIOCSCAPUTRE branches copy_*_user functions are called. on other branches not
> > /home/junfeng/linux-tainted/drivers/media/video/cpia.c:3432:cpia_do_ioctl: ERROR:TAINTED:3432:3432: dereferencing tainted ptr 'vp' [Callstack: ]
> >
> > 		DBG("VIDIOCSPICT\n");
> >
> > 		/* check validity */
> > 		DBG("palette: %d\n", vp->palette);
> > 		DBG("depth: %d\n", vp->depth);
> >
> > Error --->
> > 		if (!valid_mode(vp->palette, vp->depth)) {
> > 			retval = -EINVAL;
> > 			break;
> > 		}
> > ---------------------------------------------------------
>
> I can't see this.  This code fragment is from cpia_do_ioctl.  This is
> never called directly, the entry point is cpia_ioctl, which always passes
> ioctl calls to video_usercopy (which calls cpia_do_ioctl through the
> supplied function pointer).
>
> In video_usercopy, it calls copy_from_user for an _IOW ioctl (which
> VIDIOCSPICT is).  There is certainly no differentiation between the
> different ioctl calls made by video_usercopy.

Hi,

Our checker finds an inconsistencies in this function. For case branch
"VIDIOCSCAPTURE" and "VIDIOCGCAPTUER", copy_from_user is called on
argument "arg", which implies arg is a user-pointer. The checker isn't
smart enough to figure out if the dereferences in other branches are
errors or the copy_*_user calls are errors.

I checked videodev.h, where VIDIOCGCAPTURE and VIDIOCSCAPTUER are defined
as _IOR ... and _IOW. does that mean the copy_from_user calls on case
branch VIDIOCGCPAUTER and VIDIOCSCAPUTER are on kernel pointers?

-Junfeng


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
  2003-04-27  9:26               ` James Morris
  2003-04-27 20:18               ` Nick Holloway
@ 2003-04-27 21:29               ` Junfeng Yang
  2003-04-28  6:43               ` [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data Junfeng Yang
                                 ` (2 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-27 21:29 UTC (permalink / raw)
  To: linux-kernel

On Fri, 25 Apr 2003, Junfeng Yang wrote:

BTW, these reports are from 2.5.63.


> ---------------------------------------------------------
> Hi,
>
> I've submitted 8 such bugs several weeks before and thanks to Chris and
> other guys to fix most of them. Here are another 30 similar bugs. I got a
> warning in net/ipv4, which must be that I'm missing something, so please
> clarify. Also it seems I'm missing some assumptions for those video
> drivers.
>
> As usual, any responses will be appreciated. Thanks!
>
> Where bugs occur:
>
> net/ipv4/ip_sockglue.c
> drivers/i2c/i2c-dev.c
> drivers/ieee1394/video1394.c
> drivers/isdn/eicon/linchr.c
> drivers/media/dvb/av7110/av7110_ir.c
> drivers/media/radio/radio-cadet.c
> drivers/media/video/bw-qcam.c
> drivers/media/video/cpia.c
> drivers/media/video/zoran_procfs.c
> drivers/media/video/zr36120.c
> drivers/pnp/pnpbios/proc.c
> drivers/scsi/aacraid/commctrl.c
> drivers/usb/image/mdc800.c
> drivers/usb/media/vicam.c
> drivers/usb/serial/empeg.c
> drivers/usb/serial/io_edgeport.c
> drivers/usb/serial/ipaq.c
> drivers/usb/serial/keyspan.c
> drivers/video/sis/sis_main.c
> sound/oss/awe_wave.c
> sound/oss/cmpci.c
> sound/oss/mpu401.c
> sound/pci/es1938.c
>
>
> [UNKNOWN] maybe? it is in ipv4 though. ip_cmsg_send -> ip_options_get
> implies CMSG_DATA(cmsg) is tainted. It could be that the taintedness of CMSG_DATA(cmsg) depends on the case branch
>
> /home/junfeng/linux-tainted/net/ipv4/ip_sockglue.c:168:ip_cmsg_send: ERROR:TAINTED:168:168: dereferencing tainted ptr 'info' [Callstack: /home/junfeng/linux-tainted/net/ipv4/ip_options.c:518:ip_cmsg_send()]
>
> 		{
> 			struct in_pktinfo *info;
> 			if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct in_pktinfo)))
> 				return -EINVAL;
> 			info = (struct in_pktinfo *)CMSG_DATA(cmsg);
>
> Error --->
> 			ipc->oif = info->ipi_ifindex;
> 			ipc->addr = info->ipi_spec_dst.s_addr;
> 			break;
> 		}
> ---------------------------------------------------------
> [BUG] buf is tainted. can print out arbitrary kernel data
> /home/junfeng/linux-tainted/drivers/usb/serial/empeg.c:225:empeg_write: ERROR:TAINTED:225:225: passing tainted ptr 'buf' to usb_serial_debug_data [Callstack: /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:empeg_write((tainted 2))]
>
> 	int bytes_sent = 0;
> 	int transfer_size;
>
> 	dbg("%s - port %d", __FUNCTION__, port->number);
>
>
> Error --->
> 	usb_serial_debug_data (__FILE__, __FUNCTION__, count, buf);
>
> 	while (count > 0) {
>
> ---------------------------------------------------------
> [BUG] can print out arbitrary kernel data
> /home/junfeng/linux-tainted/drivers/usb/serial/ipaq.c:371:ipaq_write: ERROR:TAINTED:371:371: passing tainted ptr 'buf' to usb_serial_debug_data [Callstack: /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:ipaq_write((tainted 2))]
>
> 	int			bytes_sent = 0;
> 	int			transfer_size;
>
> 	dbg("%s - port %d", __FUNCTION__, port->number);
>
>
> Error --->
> 	usb_serial_debug_data(__FILE__, __FUNCTION__, count, buf);
>
> 	while (count > 0) {
> 		transfer_size = min(count, PACKET_SIZE);
> ---------------------------------------------------------
> [BUG] can print out arbitrary kernel data
> /home/junfeng/linux-tainted/drivers/usb/serial/keyspan.c:328:keyspan_write: ERROR:TAINTED:328:328: dereferencing tainted ptr 'buf' [Callstack: ]
>
>
> 	p_priv = usb_get_serial_port_data(port);
> 	d_details = p_priv->device_details;
>
> 	dbg("%s - for port %d (%d chars [%x]), flip=%d",
>
> Error --->
> 	    __FUNCTION__, port->number, count, buf[0], p_priv->out_flip);
>
> 	for (left = count; left > 0; left -= todo) {
> 		todo = left;
> ---------------------------------------------------------
> [BUG] pointer (rdwr_arg.msgs[i]) points to user space
> /home/junfeng/linux-tainted/drivers/i2c/i2c-dev.c:230:i2cdev_ioctl: ERROR:TAINTED:230:230: dereferencing tainted ptr 'rdwr_arg.msgs + i * 12' [Callstack: ]
>
> 			if(rdwr_pa[i].buf == NULL)
> 			{
> 				res = -ENOMEM;
> 				break;
> 			}
>
> Error --->
> 			if(copy_from_user(rdwr_pa[i].buf,
> 				rdwr_arg.msgs[i].buf,
> 				rdwr_pa[i].len))
> 			{
> ---------------------------------------------------------
> [BUG] file_operations.read. even the parm itself has a name pUserBuffer
> /home/junfeng/linux-tainted/drivers/isdn/eicon/linchr.c:166:do_read: ERROR:TAINTED:166:166: passing tainted ptr 'pClientLogBuffer' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:do_read((tainted 1))]
>
>
> 	pHeadItem = (klog_t *) DivasLogFifoRead();
>
> 	if (pHeadItem)
> 	{
>
> Error --->
> 		memcpy(pClientLogBuffer, pHeadItem, sizeof(klog_t));
> 		kfree(pHeadItem);
> 		return sizeof(klog_t);
> 	}
> ---------------------------------------------------------
> [BUG] file_operations.write -> cm_write -> trans_ac3. write can take tainted. write can take tainted inputs.
> /home/junfeng/linux-tainted/sound/oss/cmpci.c:593:trans_ac3: ERROR:TAINTED:593:593: dereferencing tainted ptr 'src' [Callstack: /home/junfeng/linux-tainted/fs/read_write.c:307:vfs_write((tainted 1)(tainted 2)) -> /home/junfeng/linux-tainted/fs/read_write.c:241:cm_write((tainted 1)(tainted 2)) -> /home/junfeng/linux-tainted/sound/oss/cmpci.c:1662:trans_ac3((tainted 2))]
>
> 	unsigned long data;
> 	unsigned long *dst = (unsigned long *) dest;
> 	unsigned short *src = (unsigned short *)source;
>
> 	do {
>
> Error --->
> 		data = (unsigned long) *src++;
> 		data <<= 12;			// ok for 16-bit data
> 		if (s->spdif_counter == 2 || s->spdif_counter == 3)
> 			data |= 0x40000000;	// indicate AC-3 raw data
> ---------------------------------------------------------
> [BUG] in ioctl function. v (==arg) is not checked at all. can write arbitrary data to the kernel. on the same case branch, copy_to_user is called on arg
> /home/junfeng/linux-tainted/drivers/media/radio/radio-cadet.c:397:cadet_do_ioctl: ERROR:TAINTED:397:397: dereferencing tainted ptr 'v' [Callstack: ]
>
> 	{
> 		case VIDIOCGCAP:
> 		{
> 			struct video_capability *v = arg;
> 			memset(v,0,sizeof(*v));
>
> Error --->
> 			v->type=VID_TYPE_TUNER;
> 			v->channels=2;
> 			v->audios=1;
> 			strcpy(v->name, "ADS Cadet");
> ---------------------------------------------------------
> [BUG] no check in sis_main.c at all. can write arbitrary data to kernel
> /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2476:sisfb_ioctl: ERROR:TAINTED:2476:2476: dereferencing tainted ptr 'x' [Callstack: /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted 3))]
>
> 		}
> 	   case FBIOPUT_MODEINFO:
> 		{
> 			struct mode_info *x = (struct mode_info *)arg;
>
>
> Error --->
> 			ivideo.video_bpp      = x->bpp;
> 			ivideo.video_width    = x->xres;
> 			ivideo.video_height   = x->yres;
> 			ivideo.video_vwidth   = x->v_xres;
> ---------------------------------------------------------
> [BUG] on VIDIOCGCAPUTRE and VIDIOCSCAPUTRE branches copy_*_user functions are called. on other branches not
> /home/junfeng/linux-tainted/drivers/media/video/cpia.c:3432:cpia_do_ioctl: ERROR:TAINTED:3432:3432: dereferencing tainted ptr 'vp' [Callstack: ]
>
> 		DBG("VIDIOCSPICT\n");
>
> 		/* check validity */
> 		DBG("palette: %d\n", vp->palette);
> 		DBG("depth: %d\n", vp->depth);
>
> Error --->
> 		if (!valid_mode(vp->palette, vp->depth)) {
> 			retval = -EINVAL;
> 			break;
> 		}
> ---------------------------------------------------------
> [BUG] sisfb_ioctl->sis_dispinfo. drivers/video/sstfb_ioctl assumes "arg" is tainted. both of them are assigned to fb_ops.fb_ioctl
> /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:276:sis_dispinfo: ERROR:TAINTED:276:276: dereferencing tainted ptr 'rec' [Callstack: /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted 3)) -> /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2488:sis_dispinfo((tainted 0))]
>
> 	gly->ngmask = size;
> }
>
> void sis_dispinfo(struct ap_data *rec)
> {
>
> Error --->
> 	rec->minfo.bpp    = ivideo.video_bpp;
> 	rec->minfo.xres   = ivideo.video_width;
> 	rec->minfo.yres   = ivideo.video_height;
> 	rec->minfo.v_xres = ivideo.video_vwidth;
> ---------------------------------------------------------
> [BUG] mdc800_device_read is assigned to file_operations.read, which can take tainted inputs
> /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:750:mdc800_device_read: ERROR:TAINTED:750:750: passing tainted ptr 'ptr' to __memcpy [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:mdc800_device_read((tainted 1))]
>
> 			}
> 		}
> 		else
> 		{
> 			/* memcpy Bytes */
>
> Error --->
> 			memcpy (ptr, &mdc800->out [mdc800->out_ptr], sts);
> 			ptr+=sts;
> 			left-=sts;
> 			mdc800->out_ptr+=sts;
> ---------------------------------------------------------
> [BUG] call usb_serial_debug_data on data
> /home/junfeng/linux-tainted/drivers/usb/serial/io_edgeport.c:1381:edge_write: ERROR:TAINTED:1381:1381: passing tainted ptr 'data' to usb_serial_debug_data [Callstack: ]
>
> 		fifo->head  += secondhalf;
> 		// No need to check for wrap since we can not get to end of fifo in this part
> 	}
>
> 	if (copySize) {
>
> Error --->
> 		usb_serial_debug_data (__FILE__, __FUNCTION__, copySize, data);
> 	}
>
> 	send_more_port_data((struct edgeport_serial *)usb_get_serial_data(port->serial), edge_port);
> ---------------------------------------------------------
> [BUG] proc_dir_entry.read_proc can take tainted inputs
> /home/junfeng/linux-tainted/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain: ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:vicam_write_proc_gain((tainted 1))]
>
> static int vicam_write_proc_gain(struct file *file, const char *buffer,
> 				unsigned long count, void *data)
> {
> 	struct vicam_camera *cam = (struct vicam_camera *)data;
>
>
> Error --->
> 	cam->gain = simple_strtoul(buffer, NULL, 10);
>
> 	return count;
> }
> ---------------------------------------------------------
> [BUG] midi_synth.c:midi_synth_ioctl does __copy_to_user(not copy_to_user?). it could be that there are some assumptions about the devices.
> /home/junfeng/linux-tainted/sound/oss/mpu401.c:792:mpu_synth_ioctl: ERROR:TAINTED:792:792: passing tainted ptr 'arg' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:mpu_synth_ioctl((tainted 2))]
>
>
> 	switch (cmd)
> 	{
>
> 		case SNDCTL_SYNTH_INFO:
>
> Error --->
> 			memcpy((&((char *) arg)[0]), (char *) &mpu_synth_info[midi_dev], sizeof(struct synth_info));
> 			return 0;
>
> 		case SNDCTL_SYNTH_MEMAVL:
> ---------------------------------------------------------
> [BUG] similar errors
> /home/junfeng/linux-tainted/drivers/media/radio/radio-cadet.c:399:cadet_do_ioctl: ERROR:TAINTED:399:399: dereferencing tainted ptr 'v' [Callstack: ]
>
> 		{
> 			struct video_capability *v = arg;
> 			memset(v,0,sizeof(*v));
> 			v->type=VID_TYPE_TUNER;
> 			v->channels=2;
>
> Error --->
> 			v->audios=1;
> 			strcpy(v->name, "ADS Cadet");
> 			return 0;
> 		}
> ---------------------------------------------------------
> [BUG] ioctl
> /home/junfeng/linux-tainted/drivers/usb/media/vicam.c:617:vicam_ioctl: ERROR:TAINTED:617:617: dereferencing tainted ptr 'vp' [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:vicam_ioctl((tainted 3))]
>
> 			struct video_picture *vp = (struct video_picture *) arg;
>
> 			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
> 			    vp->palette);
>
>
> Error --->
> 			cam->gain = vp->brightness >> 8;
>
> 			if (vp->depth != 24
> 			    || vp->palette != VIDEO_PALETTE_RGB24)
> ---------------------------------------------------------
> [BUG] copy_to_user is called on case VIDIOCGCHAN
> /home/junfeng/linux-tainted/drivers/media/video/bw-qcam.c:763:qcam_do_ioctl: ERROR:TAINTED:763:763: dereferencing tainted ptr 'p' [Callstack: ]
>
> 			return 0;
> 		}
> 		case VIDIOCGPICT:
> 		{
> 			struct video_picture *p = arg;
>
> Error --->
> 			p->colour=0x8000;
> 			p->hue=0x8000;
> 			p->brightness=qcam->brightness<<8;
> 			p->contrast=qcam->contrast<<8;
> ---------------------------------------------------------
> [BUG]
> /home/junfeng/linux-tainted/drivers/media/video/bw-qcam.c:723:qcam_do_ioctl: ERROR:TAINTED:723:723: dereferencing tainted ptr 'v' [Callstack: ]
>
> 		case VIDIOCGCHAN:
> 		{
> 			struct video_channel *v = arg;
> 			if(v->channel!=0)
> 				return -EINVAL;
>
> Error --->
> 			v->flags=0;
> 			v->tuners=0;
> 			/* Good question.. its composite or SVHS so.. */
> 			v->type = VIDEO_TYPE_CAMERA;
> ---------------------------------------------------------
> [BUG] file_opetations.ioctl -> aac_cfg_ioctl -> aac_do_ioctl -> close_getadapter_fib -> aac_close_fib_context. also, check_revision calls copy_to_user on "arg"
> /home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:277:aac_close_fib_context: ERROR:TAINTED:277:277: dereferencing tainted ptr 'fibctx' [Callstack: /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:aac_cfg_ioctl((tainted 3)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/linit.c:671:aac_do_ioctl((tainted 2)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:421:close_getadapter_fib((tainted 1)) -> /home/junfeng/linux-tainted/drivers/scsi/aacraid/commctrl.c:348:aac_close_fib_context((tainted 1))]
>
> 	while (!list_empty(&fibctx->fibs)) {
> 		struct list_head * entry;
> 		/*
> 		 *	Pull the next fib from the fibs
> 		 */
>
> Error --->
> 		entry = fibctx->fibs.next;
> 		list_del(entry);
> 		fib = list_entry(entry, struct hw_fib, header.FibLinks);
> 		fibctx->count--;
> ---------------------------------------------------------
> [BUG] zoran_read and vbi_read are both assigned to video_device.read, while zoran_read assumes "buf" is tainted
> /home/junfeng/linux-tainted/drivers/media/video/zr36120.c:1698:vbi_read: ERROR:TAINTED:1698:1698: dereferencing tainted ptr 'optr' [Callstack: /home/junfeng/linux-tainted/drivers/media/video/zr36120.c:934:vbi_read((tainted 1))]
>
> 		{
> 			/* copy to doubled data to userland */
> 			for (x=0; optr+1<eptr && x<-done->w; x++)
> 			{
> 				unsigned char a = iptr[x*2];
>
> Error --->
> 				*optr++ = a;
> 				*optr++ = a;
> 			}
> 			/* and clear the rest of the line */
> ---------------------------------------------------------
> [BUG] video1394_ioctl's param arg is tainted -> initialize_dma_it_prg_var_packet_queue's param packet_sizes is also tainted
> /home/junfeng/linux-tainted/drivers/ieee1394/video1394.c:668:initialize_dma_it_prg_var_packet_queue: ERROR:TAINTED:668:668: dereferencing tainted ptr 'packet_sizes + i * 4' [Callstack: /home/junfeng/linux-tainted/drivers/ieee1394/video1394.c:1047:initialize_dma_it_prg_var_packet_queue((tainted 2))]
>
> 	for (i = 0; i < d->nb_cmd; i++) {
> 		unsigned int size;
> 		if (packet_sizes[i] > d->packet_size) {
> 			size = d->packet_size;
> 		} else {
>
> Error --->
> 			size = packet_sizes[i];
> 		}
> 		it_prg[i].data[1] = cpu_to_le32(size << 16);
> 		it_prg[i].end.control = cpu_to_le32(DMA_CTL_OUTPUT_LAST | DMA_CTL_BRANCH);
> ---------------------------------------------------------
> [BUG] sound/pci/rme9652/rme9652.c:snd_rme9652_capture_copy treats the parm "dst" as tainted and they are both assigned to snd_pcm_ops_t.copy
> /home/junfeng/linux-tainted/sound/pci/es1938.c:833:snd_es1938_capture_copy: ERROR:TAINTED:833:833: passing tainted ptr 'dst' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/pci/rme9652/rme9652.c:2010:snd_es1938_capture_copy((tainted 3))]
>
> 	es1938_t *chip = snd_pcm_substream_chip(substream);
> 	pos <<= chip->dma1_shift;
> 	count <<= chip->dma1_shift;
> 	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
> 	if (pos + count < chip->dma1_size)
>
> Error --->
> 		memcpy(dst, runtime->dma_area + pos + 1, count);
> 	else {
> 		memcpy(dst, runtime->dma_area + pos + 1, count - 1);
> 		((unsigned char *)dst)[count - 1] = runtime->dma_area[0];
> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc
> /home/junfeng/linux-tainted/drivers/media/video/zoran_procfs.c:122:zoran_write_proc: ERROR:TAINTED:122:122: passing tainted ptr 'buffer' to __memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:zoran_write_proc((tainted 1))]
>
> 	string = sp = vmalloc(count + 1);
> 	if (!string) {
> 		printk(KERN_ERR "%s: write_proc: can not allocate memory\n", zr->name);
> 		return -ENOMEM;
> 	}
>
> Error --->
> 	memcpy(string, buffer, count);
> 	string[count] = 0;
> 	DEBUG2(printk(KERN_INFO "%s: write_proc: name=%s count=%lu data=%x\n", zr->name, file->f_dentry->d_name.name, count, (int) data));
> 	ldelim = " \t\n";
> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc
> /home/junfeng/linux-tainted/drivers/pnp/pnpbios/proc.c:190:proc_write_node: ERROR:TAINTED:190:190: passing tainted ptr 'buf' to __memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:proc_write_node((tainted 1))]
>
> 	if (!node) return -ENOMEM;
> 	if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
> 		return -EIO;
> 	if (count != node->size - sizeof(struct pnp_bios_node))
> 		return -EINVAL;
>
> Error --->
> 	memcpy(node->data, buf, count);
> 	if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
> 	    return -EINVAL;
> 	kfree(node);
> ---------------------------------------------------------
> [BUG] file_operations.write
> /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:805:mdc800_device_write: ERROR:TAINTED:805:805: dereferencing tainted ptr 'buf + i' [Callstack: /home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110.c:3858:mdc800_device_write((tainted 1))]
>
> 		}
>
> 		/* save command byte */
> 		if (mdc800->in_count < 8)
> 		{
>
> Error --->
> 			mdc800->in[mdc800->in_count]=buf[i];
> 			mdc800->in_count++;
> 		}
> 		else
> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc can take tainted inputs
> /home/junfeng/linux-tainted/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter: ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted 1))]
>
> static int vicam_write_proc_shutter(struct file *file, const char *buffer,
> 				unsigned long count, void *data)
> {
> 	struct vicam_camera *cam = (struct vicam_camera *)data;
>
>
> Error --->
> 	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);
>
> 	return count;
> }
> ---------------------------------------------------------
> [BUG] av7110_ir_write_proc is assigned to proc_dir_entry.write_proc
> /home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc: ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/net/core/pktgen.c:991:av7110_ir_write_proc((tainted 1))]
>
> 	u32 ir_config;
>
> 	if (count < 4 + 256 * sizeof(u16))
> 		return -EINVAL;
>
>
> Error --->
> 	memcpy (&ir_config, buffer, 4);
> 	memcpy (&key_map, buffer + 4, 256 * sizeof(u16));
>
> 	av7110_setup_irc_config (NULL, ir_config);
> ---------------------------------------------------------
> [BUG] pointer (rdwr_arg.msgs[i]) points to user space
> /home/junfeng/linux-tainted/drivers/i2c/i2c-dev.c:249:i2cdev_ioctl: ERROR:TAINTED:249:249: dereferencing tainted ptr 'rdwr_arg.msgs + i * 12' [Callstack: ]
>
> 		}
> 		while(i-- > 0)
> 		{
> 			if( res>=0 && (rdwr_pa[i].flags & I2C_M_RD))
> 			{
>
> Error --->
> 				if(copy_to_user(
> 					rdwr_arg.msgs[i].buf,
> 					rdwr_pa[i].buf,
> 					rdwr_pa[i].len))
> ---------------------------------------------------------
> [BUG] ioctl
> /home/junfeng/linux-tainted/sound/oss/awe_wave.c:2049:awe_ioctl: ERROR:TAINTED:2049:2049: passing tainted ptr 'arg' to __constant_memcpy [Callstack: /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:awe_ioctl((tainted 2))]
>
> 	case SNDCTL_SYNTH_INFO:
> 		if (playing_mode == AWE_PLAY_DIRECT)
> 			awe_info.nr_voices = awe_max_voices;
> 		else
> 			awe_info.nr_voices = AWE_MAX_CHANNELS;
>
> Error --->
> 		memcpy((char*)arg, &awe_info, sizeof(awe_info));
> 		return 0;
> 		break;
>
>


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-27  9:26               ` James Morris
@ 2003-04-28  1:55                 ` Junfeng Yang
  0 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-28  1:55 UTC (permalink / raw)
  To: linux-kernel; +Cc: Chris Wright

hi

sorry the net/ipv4 warning is a false posiglve. i just checked this
warning again. CMSG_DATA(cmsg) is passed into ip_options_get with param
"user" == 0.  our checker didn't figure out that "user" == 0 implied that
the CMSG_DATA(cmsg)  was a kernel pointer.

-Junfeng


On Sun, 27 Apr 2003, James Morris wrote:

> On Fri, 25 Apr 2003, Junfeng Yang wrote:
>
> > Where bugs occur:
> >
> > net/ipv4/ip_sockglue.c
>
> Which kernel version is this for?
>
>
> - James
> --
> James Morris
> <jmorris@intercode.com.au>
>


^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
                                 ` (2 preceding siblings ...)
  2003-04-27 21:29               ` Junfeng Yang
@ 2003-04-28  6:43               ` Junfeng Yang
  2003-04-29  7:25                 ` Greg KH
  2003-04-28  6:50               ` [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel Junfeng Yang
  2003-04-29  7:26               ` [CHECKER] 30 potential dereference of user-pointer errors Greg KH
  5 siblings, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-04-28  6:43 UTC (permalink / raw)
  To: linux-kernel, Chris Wright; +Cc: mc


Hi,

I classfied the errors in my previous report. Here are 3 errors that allow
a malicious user to print out arbitrary kernel data. (the 4th error is at
least bad programming practice.)

All of them are in usb/serial.

Any replies will be appreciated.

---------------------------------------------------------
[BUG] buf is tainted. can print out arbitrary kernel data if debug is on
/home/junfeng/linux-tainted/drivers/usb/serial/empeg.c:225:empeg_write:
ERROR:TAINTED:225:225: passing tainted ptr 'buf' to usb_serial_debug_data
[Callstack:
/home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:empeg_write((tainted
2))]

	int bytes_sent = 0;
	int transfer_size;

	dbg("%s - port %d", __FUNCTION__, port->number);


Error --->
	usb_serial_debug_data (__FILE__, __FUNCTION__, count, buf);

	while (count > 0) {

---------------------------------------------------------
[BUG] can print out arbitrary kernel data if debug is on
/home/junfeng/linux-tainted/drivers/usb/serial/ipaq.c:371:ipaq_write:
ERROR:TAINTED:371:371: passing tainted ptr 'buf' to usb_serial_debug_data
[Callstack:
/home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:ipaq_write((tainted
2))]

	int			bytes_sent = 0;
	int			transfer_size;

	dbg("%s - port %d", __FUNCTION__, port->number);


Error --->
	usb_serial_debug_data(__FILE__, __FUNCTION__, count, buf);

	while (count > 0) {
		transfer_size = min(count, PACKET_SIZE);
---------------------------------------------------------
[BUG] can print out arbitrary kernel data if debug is on
/home/junfeng/linux-tainted/drivers/usb/serial/keyspan.c:328:keyspan_write:
ERROR:TAINTED:328:328: dereferencing tainted ptr 'buf' [Callstack: ]


	p_priv = usb_get_serial_port_data(port);
	d_details = p_priv->device_details;

	dbg("%s - for port %d (%d chars [%x]), flip=%d",

Error --->
	    __FUNCTION__, port->number, count, buf[0], p_priv->out_flip);

	for (left = count; left > 0; left -= todo) {
		todo = left;



---------------------------------------------------------
[BUG] at least bad programming practice. call usb_serial_debug_data on
tainted pointer data. it is verified by previous call to copy_*_user.

/home/junfeng/linux-tainted/drivers/usb/serial/io_edgeport.c:1381:edge_write:
ERROR:TAINTED:1381:1381: passing tainted ptr 'data' to
usb_serial_debug_data [Callstack: ]

		fifo->head  += secondhalf;
		// No need to check for wrap since we can not get to end
of fifo in this part
	}

	if (copySize) {

Error --->
		usb_serial_debug_data (__FILE__, __FUNCTION__, copySize,
data);
	}

	send_more_port_data((struct edgeport_serial
*)usb_get_serial_data(port->serial), edge_port);



^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
                                 ` (3 preceding siblings ...)
  2003-04-28  6:43               ` [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data Junfeng Yang
@ 2003-04-28  6:50               ` Junfeng Yang
  2003-04-28 12:49                 ` Alan Cox
  2003-04-29  7:26               ` [CHECKER] 30 potential dereference of user-pointer errors Greg KH
  5 siblings, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-04-28  6:50 UTC (permalink / raw)
  To: linux-kernel, Chris Wright; +Cc: mc


Hi,

Here are 8 bugs where user-pointers are dereferenced or passed into memcpy
without being checked. This allows a malicious user to write to anywhere
he wants.

Any confirmations or clarifications will be appreciated.

-Junfeng


---------------------------------------------------------
[BUG] Can write to anywhere in kernel. Should have used video_usercopy
(..., vicam_ioctl) and removed all the copy_*_user calls

/home/junfeng/linux-tainted/drivers/usb/media/vicam.c:617:vicam_ioctl:
ERROR:TAINTED:617:617: dereferencing tainted ptr 'vp' [Callstack:
/home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:vicam_ioctl((tainted
3))]

			struct video_picture *vp = (struct video_picture
*) arg;

			DBG("VIDIOCSPICT depth = %d, pal = %d\n",
vp->depth,
			    vp->palette);


Error --->
			cam->gain = vp->brightness >> 8;

			if (vp->depth != 24
			    || vp->palette != VIDEO_PALETTE_RGB24)
---------------------------------------------------------
[BUG] can write to anywhere in kernel. all the other *_ioctl functions
that are assigned to struct fb_ops.fb_ioctl assume "arg" is tainted.

/home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2476:sisfb_ioctl:
ERROR:TAINTED:2476:2476: dereferencing tainted ptr 'x' [Callstack:
/home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted
3))]

		}
	   case FBIOPUT_MODEINFO:
		{
			struct mode_info *x = (struct mode_info *)arg;


Error --->
			ivideo.video_bpp      = x->bpp;
			ivideo.video_width    = x->xres;
			ivideo.video_height   = x->yres;
			ivideo.video_vwidth   = x->v_xres;
---------------------------------------------------------
[BUG] can write to anywhere in kernel. do_read is assigned to
file_operations.read, which can take tainted inputs. even the parm itself
has a name pUserBuffer

/home/junfeng/linux-tainted/drivers/isdn/eicon/linchr.c:166:do_read:
ERROR:TAINTED:166:166: passing tainted ptr 'pClientLogBuffer' to
__constant_memcpy [Callstack:
/home/junfeng/linux-tainted/drivers/scsi/sg.c:362:do_read((tainted 1))]


	pHeadItem = (klog_t *) DivasLogFifoRead();

	if (pHeadItem)
	{

Error --->
		memcpy(pClientLogBuffer, pHeadItem, sizeof(klog_t));
		kfree(pHeadItem);
		return sizeof(klog_t);
	}
---------------------------------------------------------
[BUG] can write to anywhere in kernel. in the same subdir,
midi_synth.c:midi_synth_ioctl does __copy_to_user(not copy_to_user?). it
could be that there are some assumptions about the devices.

/home/junfeng/linux-tainted/sound/oss/mpu401.c:792:mpu_synth_ioctl:
ERROR:TAINTED:792:792: passing tainted ptr 'arg' to __constant_memcpy
[Callstack:
/home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:mpu_synth_ioctl((tainted
2))]


	switch (cmd)
	{

		case SNDCTL_SYNTH_INFO:

Error --->
			memcpy((&((char *) arg)[0]), (char *)
&mpu_synth_info[midi_dev], sizeof(struct synth_info));
			return 0;

		case SNDCTL_SYNTH_MEMAVL:
---------------------------------------------------------
[BUG] can write to anywhere in kernel. mdc800_device_read is assigned to
file_operations.read, which can take tainted inputs

/home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:750:mdc800_device_read:
ERROR:TAINTED:750:750: passing tainted ptr 'ptr' to __memcpy [Callstack:
/home/junfeng/linux-tainted/drivers/scsi/sg.c:362:mdc800_device_read((tainted
1))]

			}
		}
		else
		{
			/* memcpy Bytes */

Error --->
			memcpy (ptr, &mdc800->out [mdc800->out_ptr], sts);
			ptr+=sts;
			left-=sts;
			mdc800->out_ptr+=sts;
---------------------------------------------------------
[BUG] can write to anywhere in kernel. file_operations.write

/home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:805:mdc800_device_write:
ERROR:TAINTED:805:805: dereferencing tainted ptr 'buf + i' [Callstack:
/home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110.c:3858:mdc800_device_write((tainted
1))]

		}

		/* save command byte */
		if (mdc800->in_count < 8)
		{

Error --->
			mdc800->in[mdc800->in_count]=buf[i];
			mdc800->in_count++;
		}
		else
---------------------------------------------------------
[BUG] can write to anywhere in kernel.
sound/pci/rme9652/rme9652.c:snd_rme9652_capture_copy treats the parm "dst"
as tainted and they are both assigned to snd_pcm_ops_t.copy

/home/junfeng/linux-tainted/sound/pci/es1938.c:833:snd_es1938_capture_copy:
ERROR:TAINTED:833:833: passing tainted ptr 'dst' to __constant_memcpy
[Callstack:
/home/junfeng/linux-tainted/sound/pci/rme9652/rme9652.c:2010:snd_es1938_capture_copy((tainted
3))]

	es1938_t *chip = snd_pcm_substream_chip(substream);
	pos <<= chip->dma1_shift;
	count <<= chip->dma1_shift;
	snd_assert(pos + count <= chip->dma1_size, return -EINVAL);
	if (pos + count < chip->dma1_size)

Error --->
		memcpy(dst, runtime->dma_area + pos + 1, count);
	else {
		memcpy(dst, runtime->dma_area + pos + 1, count - 1);
		((unsigned char *)dst)[count - 1] = runtime->dma_area[0];
---------------------------------------------------------
[BUG] can write to anywhere in kernel. awe_ioctl is assigned to
file_operations.ioctl

/home/junfeng/linux-tainted/sound/oss/awe_wave.c:2049:awe_ioctl:
ERROR:TAINTED:2049:2049: passing tainted ptr 'arg' to __constant_memcpy
[Callstack:
/home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:awe_ioctl((tainted
2))]

	case SNDCTL_SYNTH_INFO:
		if (playing_mode == AWE_PLAY_DIRECT)
			awe_info.nr_voices = awe_max_voices;
		else
			awe_info.nr_voices = AWE_MAX_CHANNELS;

Error --->
		memcpy((char*)arg, &awe_info, sizeof(awe_info));
		return 0;
		break;




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel
  2003-04-28  6:50               ` [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel Junfeng Yang
@ 2003-04-28 12:49                 ` Alan Cox
  2003-04-28 19:11                   ` Junfeng Yang
  0 siblings, 1 reply; 43+ messages in thread
From: Alan Cox @ 2003-04-28 12:49 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: Linux Kernel Mailing List, Chris Wright, mc

On Llu, 2003-04-28 at 07:50, Junfeng Yang wrote:
> /home/junfeng/linux-tainted/drivers/usb/media/vicam.c:617:vicam_ioctl:
> ERROR:TAINTED:617:617: dereferencing tainted ptr 'vp' [Callstack:
> /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:vicam_ioctl((tainted
> 3))]
> 
> 			struct video_picture *vp = (struct video_picture
> *) arg;
> 
> 			DBG("VIDIOCSPICT depth = %d, pal = %d\n",
> vp->depth,
> 			    vp->palette);
> 
> 
> Error --->

Correct report. Fixed in 2.4.21-ac now

> /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2476:sisfb_ioctl:
> ERROR:TAINTED:2476:2476: dereferencing tainted ptr 'x' [Callstack:
> /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted
> 3))]
> 

Correct - already fixed in 2.4.21-ac

> ---------------------------------------------------------
> [BUG] can write to anywhere in kernel. do_read is assigned to
> file_operations.read, which can take tainted inputs. even the parm itself
> has a name pUserBuffer
> 
> /home/junfeng/linux-tainted/drivers/isdn/eicon/linchr.c:166:do_read:
> ERROR:TAINTED:166:166: passing tainted ptr 'pClientLogBuffer' to
> __constant_memcpy [Callstack:
> /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:do_read((tainted 1))]
> 
> 
> 	pHeadItem = (klog_t *) DivasLogFifoRead();
> 
Correct - fixed in 2.4.21-ac now

> [BUG] can write to anywhere in kernel. in the same subdir,
> midi_synth.c:midi_synth_ioctl does __copy_to_user(not copy_to_user?). it
> could be that there are some assumptions about the devices.
> 
> /home/junfeng/linux-tainted/sound/oss/mpu401.c:792:mpu_synth_ioctl:
> ERROR:TAINTED:792:792: passing tainted ptr 'arg' to __constant_memcpy
> [Callstack:
> /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:mpu_synth_ioctl((tainted
> 2))]

The verify is done in soundcard.c
Fixed mpu_synth_ioctl in 2.4.21-ac

> [BUG] can write to anywhere in kernel. mdc800_device_read is assigned to
> file_operations.read, which can take tainted inputs
> 
> /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:750:mdc800_device_read:
> ERROR:TAINTED:750:750: passing tainted ptr 'ptr' to __memcpy [Callstack:
> /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:mdc800_device_read((tainted
> 1))]

Correct. Fixed in 2.4.21-ac


> [BUG] can write to anywhere in kernel. file_operations.write
> 
> /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:805:mdc800_device_write:
> ERROR:TAINTED:805:805: dereferencing tainted ptr 'buf + i' [Callstack:
> /home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110.c:3858:mdc800_device_write((tainted
> 1))]

Correct, fixed in 2.4.21-ac


> [BUG] can write to anywhere in kernel. awe_ioctl is assigned to
> file_operations.ioctl
> 
> /home/junfeng/linux-tainted/sound/oss/awe_wave.c:2049:awe_ioctl:
> ERROR:TAINTED:2049:2049: passing tainted ptr 'arg' to __constant_memcpy
> [Callstack:
> /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:awe_ioctl((tainted
> 2))]
> 
> 	case SNDCTL_SYNTH_INFO:
> 		if (playing_mode == AWE_PLAY_DIRECT)
> 			awe_info.nr_voices = awe_max_voices;
> 		else
> 			awe_info.nr_voices = AWE_MAX_CHANNELS;
> 
> Error --->
> 		memcpy((char*)arg, &awe_info, sizeof(awe_info));

Already fixed in 2.4, but correct report


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel
  2003-04-28 12:49                 ` Alan Cox
@ 2003-04-28 19:11                   ` Junfeng Yang
  2003-04-29  0:02                     ` [CHECKER] 5 potential user-pointer errors in write_proc Junfeng Yang
  0 siblings, 1 reply; 43+ messages in thread
From: Junfeng Yang @ 2003-04-28 19:11 UTC (permalink / raw)
  To: Alan Cox; +Cc: Linux Kernel Mailing List, mc


Thanks a lot for the confirmations!

-Junfeng

On 28 Apr 2003, Alan Cox wrote:

> On Llu, 2003-04-28 at 07:50, Junfeng Yang wrote:
> > /home/junfeng/linux-tainted/drivers/usb/media/vicam.c:617:vicam_ioctl:
> > ERROR:TAINTED:617:617: dereferencing tainted ptr 'vp' [Callstack:
> > /home/junfeng/linux-tainted/drivers/scsi/sg.c:1002:vicam_ioctl((tainted
> > 3))]
> >
> > 			struct video_picture *vp = (struct video_picture
> > *) arg;
> >
> > 			DBG("VIDIOCSPICT depth = %d, pal = %d\n",
> > vp->depth,
> > 			    vp->palette);
> >
> >
> > Error --->
>
> Correct report. Fixed in 2.4.21-ac now
>
> > /home/junfeng/linux-tainted/drivers/video/sis/sis_main.c:2476:sisfb_ioctl:
> > ERROR:TAINTED:2476:2476: dereferencing tainted ptr 'x' [Callstack:
> > /home/junfeng/linux-tainted/drivers/video/sstfb.c:808:sisfb_ioctl((tainted
> > 3))]
> >
>
> Correct - already fixed in 2.4.21-ac
>
> > ---------------------------------------------------------
> > [BUG] can write to anywhere in kernel. do_read is assigned to
> > file_operations.read, which can take tainted inputs. even the parm itself
> > has a name pUserBuffer
> >
> > /home/junfeng/linux-tainted/drivers/isdn/eicon/linchr.c:166:do_read:
> > ERROR:TAINTED:166:166: passing tainted ptr 'pClientLogBuffer' to
> > __constant_memcpy [Callstack:
> > /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:do_read((tainted 1))]
> >
> >
> > 	pHeadItem = (klog_t *) DivasLogFifoRead();
> >
> Correct - fixed in 2.4.21-ac now
>
> > [BUG] can write to anywhere in kernel. in the same subdir,
> > midi_synth.c:midi_synth_ioctl does __copy_to_user(not copy_to_user?). it
> > could be that there are some assumptions about the devices.
> >
> > /home/junfeng/linux-tainted/sound/oss/mpu401.c:792:mpu_synth_ioctl:
> > ERROR:TAINTED:792:792: passing tainted ptr 'arg' to __constant_memcpy
> > [Callstack:
> > /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:mpu_synth_ioctl((tainted
> > 2))]
>
> The verify is done in soundcard.c
> Fixed mpu_synth_ioctl in 2.4.21-ac
>
> > [BUG] can write to anywhere in kernel. mdc800_device_read is assigned to
> > file_operations.read, which can take tainted inputs
> >
> > /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:750:mdc800_device_read:
> > ERROR:TAINTED:750:750: passing tainted ptr 'ptr' to __memcpy [Callstack:
> > /home/junfeng/linux-tainted/drivers/scsi/sg.c:362:mdc800_device_read((tainted
> > 1))]
>
> Correct. Fixed in 2.4.21-ac
>
>
> > [BUG] can write to anywhere in kernel. file_operations.write
> >
> > /home/junfeng/linux-tainted/drivers/usb/image/mdc800.c:805:mdc800_device_write:
> > ERROR:TAINTED:805:805: dereferencing tainted ptr 'buf + i' [Callstack:
> > /home/junfeng/linux-tainted/drivers/media/dvb/av7110/av7110.c:3858:mdc800_device_write((tainted
> > 1))]
>
> Correct, fixed in 2.4.21-ac
>
>
> > [BUG] can write to anywhere in kernel. awe_ioctl is assigned to
> > file_operations.ioctl
> >
> > /home/junfeng/linux-tainted/sound/oss/awe_wave.c:2049:awe_ioctl:
> > ERROR:TAINTED:2049:2049: passing tainted ptr 'arg' to __constant_memcpy
> > [Callstack:
> > /home/junfeng/linux-tainted/sound/oss/midi_synth.c:270:awe_ioctl((tainted
> > 2))]
> >
> > 	case SNDCTL_SYNTH_INFO:
> > 		if (playing_mode == AWE_PLAY_DIRECT)
> > 			awe_info.nr_voices = awe_max_voices;
> > 		else
> > 			awe_info.nr_voices = AWE_MAX_CHANNELS;
> >
> > Error --->
> > 		memcpy((char*)arg, &awe_info, sizeof(awe_info));
>
> Already fixed in 2.4, but correct report
>


^ permalink raw reply	[flat|nested] 43+ messages in thread

* [CHECKER] 5 potential user-pointer errors in write_proc
  2003-04-28 19:11                   ` Junfeng Yang
@ 2003-04-29  0:02                     ` Junfeng Yang
  0 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-29  0:02 UTC (permalink / raw)
  To: Alan Cox, Linux Kernel Mailing List; +Cc: mc


Hi,

Here are 5 bugs in 2.5.63 where the second argument of
proc_dir_entry.write_proc argument is passed into dereference-like
functions (memcpy, simple_strtoul).

Please confirm or clarify. Thanks!

-Junfeng

---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs

/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain:
ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_gain((tainted
1))]

static int vicam_write_proc_gain(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->gain = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs

/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter:
ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted
1))]

static int vicam_write_proc_shutter(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/media/video/zoran_procfs.c:122:zoran_write_proc:
ERROR:TAINTED:122:122: passing tainted ptr 'buffer' to __memcpy
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:zoran_write_proc((tainted
1))]

	string = sp = vmalloc(count + 1);
	if (!string) {
		printk(KERN_ERR "%s: write_proc: can not allocate
memory\n", zr->name);
		return -ENOMEM;
	}

Error --->
	memcpy(string, buffer, count);
	string[count] = 0;
	DEBUG2(printk(KERN_INFO "%s: write_proc: name=%s count=%lu
data=%x\n", zr->name, file->f_dentry->d_name.name, count, (int) data));
	ldelim = " \t\n";
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/pnp/pnpbios/proc.c:190:proc_write_node:
ERROR:TAINTED:190:190: passing tainted ptr 'buf' to __memcpy [Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:proc_write_node((tainted
1))]

	if (!node) return -ENOMEM;
	if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
		return -EIO;
	if (count != node->size - sizeof(struct pnp_bios_node))
		return -EINVAL;

Error --->
	memcpy(node->data, buf, count);
	if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
	    return -EINVAL;
	kfree(node);
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs.
av7110_ir_write_proc is assigned to proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc:
ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:av7110_ir_write_proc((tainted
1))]

	u32 ir_config;

	if (count < 4 + 256 * sizeof(u16))
		return -EINVAL;


Error --->
	memcpy (&ir_config, buffer, 4);
	memcpy (&key_map, buffer + 4, 256 * sizeof(u16));

	av7110_setup_irc_config (NULL, ir_config);



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data
  2003-04-28  6:43               ` [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data Junfeng Yang
@ 2003-04-29  7:25                 ` Greg KH
  2003-04-29  9:14                   ` Junfeng Yang
  0 siblings, 1 reply; 43+ messages in thread
From: Greg KH @ 2003-04-29  7:25 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, Chris Wright, mc

On Sun, Apr 27, 2003 at 11:43:56PM -0700, Junfeng Yang wrote:
> 
> ---------------------------------------------------------
> [BUG] buf is tainted. can print out arbitrary kernel data if debug is on
> /home/junfeng/linux-tainted/drivers/usb/serial/empeg.c:225:empeg_write:
> ERROR:TAINTED:225:225: passing tainted ptr 'buf' to usb_serial_debug_data
> [Callstack:
> /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:empeg_write((tainted
> 2))]
> 
> 	int bytes_sent = 0;
> 	int transfer_size;
> 
> 	dbg("%s - port %d", __FUNCTION__, port->number);
> 
> 
> Error --->
> 	usb_serial_debug_data (__FILE__, __FUNCTION__, count, buf);
> 
> 	while (count > 0) {

Real problem, I'll fix it.

> ---------------------------------------------------------
> [BUG] can print out arbitrary kernel data if debug is on
> /home/junfeng/linux-tainted/drivers/usb/serial/ipaq.c:371:ipaq_write:
> ERROR:TAINTED:371:371: passing tainted ptr 'buf' to usb_serial_debug_data
> [Callstack:
> /home/junfeng/linux-tainted/drivers/usb/serial/safe_serial.c:327:ipaq_write((tainted
> 2))]
> 
> 	int			bytes_sent = 0;
> 	int			transfer_size;
> 
> 	dbg("%s - port %d", __FUNCTION__, port->number);
> 
> 
> Error --->
> 	usb_serial_debug_data(__FILE__, __FUNCTION__, count, buf);

Real problem, I'll fix it.

> ---------------------------------------------------------
> [BUG] can print out arbitrary kernel data if debug is on
> /home/junfeng/linux-tainted/drivers/usb/serial/keyspan.c:328:keyspan_write:
> ERROR:TAINTED:328:328: dereferencing tainted ptr 'buf' [Callstack: ]
> 
> 
> 	p_priv = usb_get_serial_port_data(port);
> 	d_details = p_priv->device_details;
> 
> 	dbg("%s - for port %d (%d chars [%x]), flip=%d",
> 
> Error --->
> 	    __FUNCTION__, port->number, count, buf[0], p_priv->out_flip);

Real problem, I'll fix it.

> ---------------------------------------------------------
> [BUG] at least bad programming practice. call usb_serial_debug_data on
> tainted pointer data. it is verified by previous call to copy_*_user.
> 
> /home/junfeng/linux-tainted/drivers/usb/serial/io_edgeport.c:1381:edge_write:
> ERROR:TAINTED:1381:1381: passing tainted ptr 'data' to
> usb_serial_debug_data [Callstack: ]
> 
> 		fifo->head  += secondhalf;
> 		// No need to check for wrap since we can not get to end
> of fifo in this part
> 	}
> 
> 	if (copySize) {
> 
> Error --->
> 		usb_serial_debug_data (__FILE__, __FUNCTION__, copySize,
> data);

Again, a real problem, I'll fix it.

Thanks a lot for finding these, I think these problems are also in
2.4...


greg k-h

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 30 potential dereference of user-pointer errors
  2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
                                 ` (4 preceding siblings ...)
  2003-04-28  6:50               ` [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel Junfeng Yang
@ 2003-04-29  7:26               ` Greg KH
  5 siblings, 0 replies; 43+ messages in thread
From: Greg KH @ 2003-04-29  7:26 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, Chris Wright, mc

On Fri, Apr 25, 2003 at 07:18:02PM -0700, Junfeng Yang wrote:
> [BUG] pointer (rdwr_arg.msgs[i]) points to user space
> /home/junfeng/linux-tainted/drivers/i2c/i2c-dev.c:230:i2cdev_ioctl: ERROR:TAINTED:230:230: dereferencing tainted ptr 'rdwr_arg.msgs + i * 12' [Callstack: ]
> 
> 			if(rdwr_pa[i].buf == NULL)
> 			{
> 				res = -ENOMEM;
> 				break;
> 			}
> 
> Error --->
> 			if(copy_from_user(rdwr_pa[i].buf,
> 				rdwr_arg.msgs[i].buf,
> 				rdwr_pa[i].len))
> 			{

This one should be already be fixed in the 2.5.68 kernel.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data
  2003-04-29  7:25                 ` Greg KH
@ 2003-04-29  9:14                   ` Junfeng Yang
  0 siblings, 0 replies; 43+ messages in thread
From: Junfeng Yang @ 2003-04-29  9:14 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, mc


>
> Thanks a lot for finding these, I think these problems are also in
> 2.4...

Thanks a lot for the confirmations and fixes. We are going to run our
checkers on 2.4.X soon.

-Junfeng


^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2003-04-29  9:02 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-03-04 11:12 [CHECKER] potential races in kernel/*.c mm/*.c net/*ipv4*.c Dawson Engler
2003-03-04 12:24 ` Hugh Dickins
2003-03-04 13:23 ` Martin Josefsson
2003-03-21  6:33 ` [CHECKER] potential dereference of user pointer errors Junfeng Yang
2003-03-21 21:44   ` Chris Wright
2003-03-21 21:58     ` Junfeng Yang
2003-03-21 22:06       ` Chris Wright
2003-03-21 22:08     ` Junfeng Yang
2003-03-21 22:15   ` Chris Wright
2003-03-22 20:49     ` Alan Cox
2003-03-22 20:19       ` Chris Wright
2003-03-21 23:55   ` Chris Wright
2003-03-27  8:07     ` Jan Kasprzak
2003-03-27 17:10       ` Chris Wright
2003-04-21  7:49         ` [CHECKER] Help Needed! Junfeng Yang
2003-04-21 21:26           ` Chris Wright
2003-04-26  2:18             ` [CHECKER] 30 potential dereference of user-pointer errors Junfeng Yang
2003-04-27  9:26               ` James Morris
2003-04-28  1:55                 ` Junfeng Yang
2003-04-27 20:18               ` Nick Holloway
2003-04-27 21:14                 ` Junfeng Yang
2003-04-27 21:29               ` Junfeng Yang
2003-04-28  6:43               ` [CHECKER] 3 potential user-pointer errors in drivers/usb/serial that can print out arbitrary kernel data Junfeng Yang
2003-04-29  7:25                 ` Greg KH
2003-04-29  9:14                   ` Junfeng Yang
2003-04-28  6:50               ` [CHECKER] 8 potential user-pointer errors that allow arbitrary writes to kernel Junfeng Yang
2003-04-28 12:49                 ` Alan Cox
2003-04-28 19:11                   ` Junfeng Yang
2003-04-29  0:02                     ` [CHECKER] 5 potential user-pointer errors in write_proc Junfeng Yang
2003-04-29  7:26               ` [CHECKER] 30 potential dereference of user-pointer errors Greg KH
2003-03-22  0:15   ` [CHECKER] potential dereference of user pointer errors Chris Wright
2003-03-22  0:32     ` Greg KH
2003-03-22  0:47       ` Chris Wright
2003-03-22  1:00         ` Greg KH
2003-03-22  0:32   ` Chris Wright
2003-03-23 23:10   ` Junfeng Yang
2003-03-24  0:24     ` [CHECKER] 63 potential calling blocking functions with locks held errors Junfeng Yang
2003-03-24 12:35       ` [CHECKER] 8 potential calling blocking kmalloc(GFP_KERNEL) " Junfeng Yang
2003-03-24  0:29     ` [CHECKER] 1 potential double unlock error Junfeng Yang
2003-03-24  9:07     ` [CHECKER] potential dereference of user pointer errors Jaroslav Kysela
2003-03-24 22:28   ` Raja R Harinath
2003-03-25  0:44     ` David S. Miller
2003-03-25 18:52       ` Raja R Harinath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).