OpenLDAP
Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest

Viewing Development/5370
Full headers

From: h.b.furuseth@usit.uio.no
Subject: race condition in slap_op_time()
Compose comment
Download message
State:
0 replies:
3 followups: 1 2 3

Major security issue: yes  no

Notes:

Notification:


Date: Mon, 11 Feb 2008 14:37:24 GMT
From: h.b.furuseth@usit.uio.no
To: openldap-its@OpenLDAP.org
Subject: race condition in slap_op_time()
Full_Name: Hallvard B Furuseth
Version: HEAD, RE23, RE24
OS: 
URL: 
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard


slapd/operation.c:slap_op_time() can decrease last_time:

thread 1:   *t = slap_get_time();
<clock:     increment time()>
thread 2:   *t = slap_get_time();
thread 2:   with mutex lock: increment last_time, set last_incr = 0;
thread 1:   with mutex lock: decrement last_time, set last_incr = 0;

The simplest fix is to move the slap_get_time() call inside the
(global) slap_op_mutex lock.

However as far as I can tell, only the accesslog overlay uses
op->o_tincr, the value which needs the mutex.  And accesslog
calls slap_op_time() itself when it needs that value.
So maybe we should remove the op->o_tincr field.  Other calls
to slap_op_time() can be replaced with slap_get_time().


Followup 1

Download message
Date: Mon, 11 Feb 2008 11:32:05 -0800
From: Howard Chu <hyc@symas.com>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#5370) race condition in slap_op_time()
h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD, RE23, RE24
> OS:
> URL:
> Submission from: (NULL) (129.240.6.233)
> Submitted by: hallvard
>
>
> slapd/operation.c:slap_op_time() can decrease last_time:
>
> thread 1:   *t = slap_get_time();
> <clock:     increment time()>
> thread 2:   *t = slap_get_time();
> thread 2:   with mutex lock: increment last_time, set last_incr = 0;
> thread 1:   with mutex lock: decrement last_time, set last_incr = 0;
>
> The simplest fix is to move the slap_get_time() call inside the
> (global) slap_op_mutex lock.
>
> However as far as I can tell, only the accesslog overlay uses
> op->o_tincr, the value which needs the mutex.  And accesslog
> calls slap_op_time() itself when it needs that value.
> So maybe we should remove the op->o_tincr field.  Other calls
> to slap_op_time() can be replaced with slap_get_time().

Sounds like a good idea. It was somewhat redundant with the CSN counter 
(except that this was for all operations, and CSN was only for write 
operations) and it never became generally useful. (Shifted to microsecond 
timestamps in 2.4, because tincr isn't useful across multiple servers.) I 
guess it makes more sense to make it specific to accesslog.

-- 
   -- Howard Chu
   Chief Architect, Symas Corp.  http://www.symas.com
   Director, Highland Sun        http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP     http://www.openldap.org/project/



Followup 2

Download message
Date: Tue, 12 Feb 2008 12:31:28 -0800
From: Howard Chu <hyc@symas.com>
To: openldap-its@openldap.org
Subject: Re: (ITS#5370) race condition in slap_op_time()
hyc@symas.com wrote:
> h.b.furuseth@usit.uio.no wrote:

>> However as far as I can tell, only the accesslog overlay uses
>> op->o_tincr, the value which needs the mutex.  And accesslog
>> calls slap_op_time() itself when it needs that value.
>> So maybe we should remove the op->o_tincr field.  Other calls
>> to slap_op_time() can be replaced with slap_get_time().
>
> Sounds like a good idea. It was somewhat redundant with the CSN counter
> (except that this was for all operations, and CSN was only for write
> operations) and it never became generally useful. (Shifted to microsecond
> timestamps in 2.4, because tincr isn't useful across multiple servers.) I
> guess it makes more sense to make it specific to accesslog.
>
Except that this will take a lot more restructuring in accesslog. Going with 
the quick fix for now.

-- 
   -- Howard Chu
   Chief Architect, Symas Corp.  http://www.symas.com
   Director, Highland Sun        http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP     http://www.openldap.org/project/



Followup 3

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 13 Feb 2008 10:17:01 +0100
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#5370) race condition in slap_op_time()
hyc@symas.com writes:
> Except that this will take a lot more restructuring in accesslog. Going
with 
> the quick fix for now.

Looks like I didn't notice the first use of o_tincr in accesslog:-(
I guess accesslog can set an "I need o_tincr" flag somewhere, which
slapd will then obey.

-- 
Hallvard


Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest


The OpenLDAP Issue Tracking System uses a hacked version of JitterBug

______________
© Copyright 2013, OpenLDAP Foundation, info@OpenLDAP.org