linux-kernel - Re: [PATCH] net: fix race in the receive/select

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: <20090626054230.GA5204@ff.dom.local>
Date:	Fri, 26 Jun 2009 05:42:30 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Davide Libenzi <davidel@...ilserver.org>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Oleg Nesterov <oleg@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
	netdev@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	fbl@...hat.com, nhorman@...hat.com, davem@...hat.com,
	Tejun Heo <htejun@...il.com>
Subject: Re: [PATCH] net: fix race in the receive/select

On 26-06-2009 05:14, Davide Libenzi wrote:
> On Fri, 26 Jun 2009, Eric Dumazet wrote:
> 
>> I wont argue with you David, just try to correct bugs.
>>
>> fs/ext4/ioctl.c line 182
>>
>> 	set_current_state(TASK_INTERRUPTIBLE);
>> 	add_wait_queue(&EXT4_SB(sb)->ro_wait_queue, &wait);
>> 	if (timer_pending(&EXT4_SB(sb)->turn_ro_timer)) {
>> 		schedule();
>>
>> Another example of missing barrier after add_wait_queue()
>>
>> Because add_wait_queue() misses a barrier, we have to add one after each call.
>>
>> Maybe it would be safer to add barrier in add_wait_queue() itself, not in _pollwait().
> 
> Not all the code that uses add_wait_queue() does need to have the MB,
> like code that does the most common pattern:
> 
> xxx_poll(...) {
> 	poll_wait(...);
> 	lock();
> 	flags = calc_flags(->status);
> 	unlock();
> 	return flags;
> }
> 
> xxx_update(...) {
> 	lock();
> 	->status = ...;
> 	unlock();
> 	if (waitqueue_active())
> 		wake_up();
> }
> 
> It's the code that does the lockless flags calculation in ->poll that 
> might need it.
> I dunno what the amount of changes are, but cross-matching MB across 
> subsystems does not look nice.
> IMHO that's a detail of the subsystem locking, and should be confined 
> inside the subsystem itself.
> No?

How about poll_wait_mb() and waitqueue_active_mb() (with mb and
additional check for NULL of wait_queue_head)?

Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/