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

Logged in as guest

Viewing Incoming/7444
Full headers

From: tixu@cs.ucsd.edu
Subject: Check the return value of ldap_pvt_thread_set_concurrency
Compose comment
Download message
State:
0 replies:
3 followups: 1 2 3

Major security issue: yes  no

Notes:

Notification:


Date: Sat, 17 Nov 2012 00:30:36 +0000
From: tixu@cs.ucsd.edu
To: openldap-its@OpenLDAP.org
Subject: Check the return value of ldap_pvt_thread_set_concurrency
Full_Name: Tianyin Xu
Version: 2.4.33
OS: Ubuntu 12.04 (actually doesn't matter)
URL: 
Submission from: (NULL) (2607:f720:1300:1241:590b:49c:889f:7b21)


Hi, all,

Still the verbosity idea. In the current version of slapd, when handling users'
"concurrency" setting, slapd does not check the return value of
"ldap_pvt_thread_set_concurrency" (essentially the return value of
"pthread_setconcurrency") to see whether the setting is successful or not.

So when the user setting is inappropriate (i.e., too big), slapd says nothing.
But users have no way to be aware of such configuration failure. I suggest to
check and notify users in this case.

Here's the patch: 

--- ../openldap-2.4.33/servers/slapd/bconfig.c      2012-10-10
05:18:49.000000000 -0700
+++ servers/slapd/bconfig.c     2012-11-16 14:52:37.211421828 -0800
@@ -1530,7 +1530,14 @@
                        break;
 
                case CFG_CONCUR:
-                       ldap_pvt_thread_set_concurrency(c->value_int);
+                       
+                        if(ldap_pvt_thread_set_concurrency(c->value_int) !=
0)
{
+                               snprintf( c->cr_msg, sizeof( c->cr_msg ),
+                                         "concurrency=%d is not valid; using
the default setting",
+                                         c->value_int );
+                                Debug(LDAP_DEBUG_ANY, "%s: %s.\n", c->log,
c->cr_msg, 0 );
+
+                       }
                        break;
 
                case CFG_THREADS:


Followup 1

Download message
Date: Sun, 25 Nov 2012 19:54:08 -0800
From: Howard Chu <hyc@symas.com>
To: tixu@cs.ucsd.edu
CC: openldap-its@openldap.org
Subject: Re: (ITS#7444) Check the return value of ldap_pvt_thread_set_concurrency
tixu@cs.ucsd.edu wrote:
> Full_Name: Tianyin Xu
> Version: 2.4.33
> OS: Ubuntu 12.04 (actually doesn't matter)
> URL:
> Submission from: (NULL) (2607:f720:1300:1241:590b:49c:889f:7b21)
>
>
> Hi, all,
>
> Still the verbosity idea. In the current version of slapd, when handling
users'
> "concurrency" setting, slapd does not check the return value of
> "ldap_pvt_thread_set_concurrency" (essentially the return value of
> "pthread_setconcurrency") to see whether the setting is successful or not.
>
> So when the user setting is inappropriate (i.e., too big), slapd says
nothing.
> But users have no way to be aware of such configuration failure. I suggest
to
> check and notify users in this case.

pthread_setconcurrency is a no-op on Linux systems. Even on systems where this 
setting might have an effect, it is generally unused. We have more important 
things to worry about than this.

> Here's the patch:
>
> --- ../openldap-2.4.33/servers/slapd/bconfig.c      2012-10-10
> 05:18:49.000000000 -0700
> +++ servers/slapd/bconfig.c     2012-11-16 14:52:37.211421828 -0800
> @@ -1530,7 +1530,14 @@
>                          break;
>
>                  case CFG_CONCUR:
> -                       ldap_pvt_thread_set_concurrency(c->value_int);
> +
> +                       
if(ldap_pvt_thread_set_concurrency(c->value_int) != 0)
> {
> +                               snprintf( c->cr_msg, sizeof(
c->cr_msg ),
> +                                         "concurrency=%d is not valid;
using
> the default setting",
> +                                         c->value_int );
> +                                Debug(LDAP_DEBUG_ANY, "%s: %s.\n",
c->log,
> c->cr_msg, 0 );
> +
> +                       }
>                          break;
>
>                  case CFG_THREADS:
>
>
>


-- 
   -- Howard Chu
   CTO, 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: Mon, 26 Nov 2012 01:26:59 -0800
Subject: Re: (ITS#7444) Check the return value of ldap_pvt_thread_set_concurrency
From: Tianyin Xu <tixu@cs.ucsd.edu>
To: Howard Chu <hyc@symas.com>
Cc: openldap-its@openldap.org
--f46d04428c6e0fb88b04cf62891a
Content-Type: text/plain; charset=ISO-8859-1

On Sun, Nov 25, 2012 at 7:54 PM, Howard Chu <hyc@symas.com> wrote:

> tixu@cs.ucsd.edu wrote:
>
>> Full_Name: Tianyin Xu
>> Version: 2.4.33
>> OS: Ubuntu 12.04 (actually doesn't matter)
>> URL:
>> Submission from: (NULL) (2607:f720:1300:1241:590b:49c:**889f:7b21)
>>
>>
>> Hi, all,
>>
>> Still the verbosity idea. In the current version of slapd, when
handling
>> users'
>> "concurrency" setting, slapd does not check the return value of
>> "ldap_pvt_thread_set_**concurrency" (essentially the return value of
>> "pthread_setconcurrency") to see whether the setting is successful or
not.
>>
>> So when the user setting is inappropriate (i.e., too big), slapd says
>> nothing.
>> But users have no way to be aware of such configuration failure. I
>> suggest to
>> check and notify users in this case.
>>
>
> pthread_setconcurrency is a no-op on Linux systems. Even on systems where
> this setting might have an effect, it is generally unused. We have more
> important things to worry about than this.
>
>
Yes, I understand that user configuration is never a high-priority issue.
That's why I attached my patch which simply checks the return value, and
prints out a log messge to notify users. The patch itself does not have any
side effect to other part of code.

I personally think the verbosity idea can improve the system reactions to
users' configurations including this one and its #7443. It is safe (no side
effect) and easy to do (simply adding some log messages), but can greatly
improve the usability and friendlyness to configurations. If you agree, I'm
happy to write patches and corresponding code.

Thanks,
Tianyin



>  Here's the patch:
>>
>> --- ../openldap-2.4.33/servers/**slapd/bconfig.c      2012-10-10
>> 05:18:49.000000000 -0700
>> +++ servers/slapd/bconfig.c     2012-11-16 14:52:37.211421828 -0800
>> @@ -1530,7 +1530,14 @@
>>                          break;
>>
>>                  case CFG_CONCUR:
>> -                      
ldap_pvt_thread_set_**concurrency(c->value_int);
>> +
>> +                       
if(ldap_pvt_thread_set_**concurrency(c->value_int)
>> != 0)
>> {
>> +                               snprintf( c->cr_msg, sizeof(
c->cr_msg ),
>> +                                         "concurrency=%d is not valid;
>> using
>> the default setting",
>> +                                         c->value_int );
>> +                                Debug(LDAP_DEBUG_ANY, "%s: %s.\n",
>> c->log,
>> c->cr_msg, 0 );
>> +
>> +                       }
>>                          break;
>>
>>                  case CFG_THREADS:
>>
>>
>>
>>
>
> --
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP 
http://www.openldap.org/**project/<http://www.openldap.org/project/>
>



-- 
Tianyin XU,
http://cseweb.ucsd.edu/~tixu/

--f46d04428c6e0fb88b04cf62891a
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div class=3D"gmail_extra"><br><div class=3D"gmail_quote">On
Sun, Nov 25, 2=
012 at 7:54 PM, Howard Chu <span dir=3D"ltr">&lt;<a
href=3D"mailto:hyc@syma=
s.com" target=3D"_blank">hyc@symas.com</a>&gt;</span>
wrote:<br><blockquote=
 class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc soli=
d;padding-left:1ex">
<div class=3D"im"><a href=3D"mailto:tixu@cs.ucsd.edu"
target=3D"_blank">tix=
u@cs.ucsd.edu</a> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
Full_Name: Tianyin Xu<br>
Version: 2.4.33<br>
OS: Ubuntu 12.04 (actually doesn&#39;t matter)<br>
URL:<br>
Submission from: (NULL)
(2607:f720:1300:1241:590b:49c:<u></u>889f:7b21)<br>
<br>
<br>
Hi, all,<br>
<br>
Still the verbosity idea. In the current version of slapd, when handling us=
ers&#39;<br>
&quot;concurrency&quot; setting, slapd does not check the return value
of<b=
r>
&quot;ldap_pvt_thread_set_<u></u>concurrency&quot;
(essentially the return =
value of<br>
&quot;pthread_setconcurrency&quot;) to see whether the setting is
successfu=
l or not.<br>
<br>
So when the user setting is inappropriate (i.e., too big), slapd says nothi=
ng.<br>
But users have no way to be aware of such configuration failure. I suggest =
to<br>
check and notify users in this case.<

Message of length 9349 truncated


Followup 3

Download message
Date: Fri, 30 Nov 2012 04:53:05 -0800
From: Howard Chu <hyc@symas.com>
To: Tianyin Xu <tixu@cs.ucsd.edu>
CC: openldap-its@openldap.org
Subject: Re: (ITS#7444) Check the return value of ldap_pvt_thread_set_concurrency
Tianyin Xu wrote:
>
> On Sun, Nov 25, 2012 at 7:54 PM, Howard Chu <hyc@symas.com
> <mailto:hyc@symas.com>> wrote:
>
>     tixu@cs.ucsd.edu <mailto:tixu@cs.ucsd.edu> wrote:
>
>         Full_Name: Tianyin Xu
>         Version: 2.4.33
>         OS: Ubuntu 12.04 (actually doesn't matter)
>         URL:
>         Submission from: (NULL) (2607:f720:1300:1241:590b:49c:__889f:7b21)
>
>
>         Hi, all,
>
>         Still the verbosity idea. In the current version of slapd, when
>         handling users'
>         "concurrency" setting, slapd does not check the return value of
>         "ldap_pvt_thread_set___concurrency" (essentially the return value
of
>         "pthread_setconcurrency") to see whether the setting is successful
or not.
>
>         So when the user setting is inappropriate (i.e., too big), slapd
says
>         nothing.
>         But users have no way to be aware of such configuration failure. I
>         suggest to
>         check and notify users in this case.
>
>
>     pthread_setconcurrency is a no-op on Linux systems. Even on systems
where
>     this setting might have an effect, it is generally unused. We have more
>     important things to worry about than this.
>
>
> Yes, I understand that user configuration is never a high-priority issue.
> That's why I attached my patch which simply checks the return value, and
> prints out a log messge to notify users. The patch itself does not have any
> side effect to other part of code.
>
> I personally think the verbosity idea can improve the system reactions to
> users' configurations including this one and its #7443. It is safe (no side
> effect) and easy to do (simply adding some log messages), but can greatly
> improve the usability and friendlyness to configurations. If you agree, I'm
> happy to write patches and corresponding code.

We appreciate your desire to help, but as I said before, these two directives 
are essentially unused in 99.999% of deployments. If you're looking for code 
to improve, I suggest you review some of the other open reports in the ITS.
>
> Thanks,
> Tianyin
>
>         Here's the patch:
>
>         --- ../openldap-2.4.33/servers/__slapd/bconfig.c      2012-10-10
>         05:18:49.000000000 -0700
>         +++ servers/slapd/bconfig.c     2012-11-16 14:52:37.211421828 -0800
>         @@ -1530,7 +1530,14 @@
>                                   break;
>
>                           case CFG_CONCUR:
>         -                      
ldap_pvt_thread_set___concurrency(c->value_int);
>         +
>         +
>           if(ldap_pvt_thread_set___concurrency(c->value_int) != 0)
>         {
>         +                               snprintf( c->cr_msg, sizeof(
c->cr_msg ),
>         +                                         "concurrency=%d is not
>         valid; using
>         the default setting",
>         +                                         c->value_int );
>         +                                Debug(LDAP_DEBUG_ANY, "%s: %s.\n",
>         c->log,
>         c->cr_msg, 0 );
>         +
>         +                       }
>                                   break;
>
>                           case CFG_THREADS:
>
>
>
>

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


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