netdev - Re: [PATCH] Wrong locking code in udp seq

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

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

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

Message-ID: <49C799DD.8070504@cosmosbay.com>
Date:	Mon, 23 Mar 2009 15:17:01 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Vitaly Mayatskikh <v.mayatskih@...il.com>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] Wrong locking code in udp seq_file infrastructure

Vitaly Mayatskikh a écrit :
> Reading zero bytes from /proc/net/udp or other similar files which use
> the same seq_file udp infrastructure panics kernel in that way:
> 
> =====================================
> [ BUG: bad unlock balance detected! ]
> -------------------------------------
> read/1985 is trying to release lock (&table->hash[i].lock) at:
> [<ffffffff81321d83>] udp_seq_stop+0x27/0x29
> but there are no more locks to release!
> 
> other info that might help us debug this:
> 1 lock held by read/1985:
>  #0:  (&p->lock){--..}, at: [<ffffffff810eefb6>] seq_read+0x38/0x348
> 
> stack backtrace:
> Pid: 1985, comm: read Not tainted 2.6.29-rc8 #9
> Call Trace:
>  [<ffffffff81321d83>] ? udp_seq_stop+0x27/0x29
>  [<ffffffff8106dab9>] print_unlock_inbalance_bug+0xd6/0xe1
>  [<ffffffff8106db62>] lock_release_non_nested+0x9e/0x1c6
>  [<ffffffff810ef030>] ? seq_read+0xb2/0x348
>  [<ffffffff8106bdba>] ? mark_held_locks+0x68/0x86
>  [<ffffffff81321d83>] ? udp_seq_stop+0x27/0x29
>  [<ffffffff8106dde7>] lock_release+0x15d/0x189
>  [<ffffffff8137163c>] _spin_unlock_bh+0x1e/0x34
>  [<ffffffff81321d83>] udp_seq_stop+0x27/0x29
>  [<ffffffff810ef239>] seq_read+0x2bb/0x348
>  [<ffffffff810eef7e>] ? seq_read+0x0/0x348
>  [<ffffffff8111aedd>] proc_reg_read+0x90/0xaf
>  [<ffffffff810d878f>] vfs_read+0xa6/0x103
>  [<ffffffff8106bfac>] ? trace_hardirqs_on_caller+0x12f/0x153
>  [<ffffffff810d88a2>] sys_read+0x45/0x69
>  [<ffffffff8101123a>] system_call_fastpath+0x16/0x1b
> BUG: scheduling while atomic: read/1985/0xffffff00
> INFO: lockdep is turned off.
> Modules linked in: cpufreq_ondemand acpi_cpufreq freq_table dm_multipath kvm ppdev snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_hwdep snd_seq_dummy snd_seq_oss snd_seq_midi_event arc4 snd_s
> eq ecb thinkpad_acpi snd_seq_device iwl3945 hwmon sdhci_pci snd_pcm_oss sdhci rfkill mmc_core snd_mixer_oss i2c_i801 mac80211 yenta_socket ricoh_mmc i2c_core iTCO_wdt snd_pcm iTCO_vendor_support rs
> rc_nonstatic snd_timer snd lib80211 cfg80211 soundcore snd_page_alloc video parport_pc output parport e1000e [last unloaded: scsi_wait_scan]
> Pid: 1985, comm: read Not tainted 2.6.29-rc8 #9
> Call Trace:
>  [<ffffffff8106b456>] ? __debug_show_held_locks+0x1b/0x24
>  [<ffffffff81043660>] __schedule_bug+0x7e/0x83
>  [<ffffffff8136ede9>] schedule+0xce/0x838
>  [<ffffffff810d7972>] ? fsnotify_access+0x5f/0x67
>  [<ffffffff810112d0>] ? sysret_careful+0xb/0x37
>  [<ffffffff8106be9c>] ? trace_hardirqs_on_caller+0x1f/0x153
>  [<ffffffff8137127b>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff810112f6>] sysret_careful+0x31/0x37
> read[1985]: segfault at 7fffc479bfe8 ip 0000003e7420a180 sp 00007fffc479bfa0 error 6
> Kernel panic - not syncing: Aiee, killing interrupt handler!
> 
> udp_seq_stop() tries to unlock not yet locked spinlock. The lock was lost
> during splitting global udp_hash_lock to subsequent spinlocks.
> 
> Signed-off by: Vitaly Mayatskikh <v.mayatskih@...il.com>
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index c47c989..c8bee18 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1614,7 +1614,8 @@ static struct sock *udp_get_next(struct seq_file *seq, struct sock *sk)
>  	} while (sk && (!net_eq(sock_net(sk), net) || sk->sk_family != state->family));
>  
>  	if (!sk) {
> -		spin_unlock_bh(&state->udp_table->hash[state->bucket].lock);
> +		if (state->bucket < UDP_HTABLE_SIZE)
> +			spin_unlock_bh(&state->udp_table->hash[state->bucket].lock);
>  		return udp_get_first(seq, state->bucket + 1);
>  	}
>  	return sk;
> @@ -1632,6 +1633,9 @@ static struct sock *udp_get_idx(struct seq_file *seq, loff_t pos)
>  
>  static void *udp_seq_start(struct seq_file *seq, loff_t *pos)
>  {
> +	struct udp_iter_state *state = seq->private;
> +	state->bucket = UDP_HTABLE_SIZE;
> +
>  	return *pos ? udp_get_idx(seq, *pos-1) : SEQ_START_TOKEN;
>  }
>  
> 

Oops...

Nice catch Vitaly

Acked-by: Eric Dumazet <dada1@...mosbay.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html