Skip to content

Commit 92441ac

Browse files
committed
opal/info: fix recursive deadlock in opal_info_dup_mode()
use opal_info_{get,set}_nolock() instead of opal_info_{get,set}() since the former can be invoked when the info lock is being held. Signed-off-by: Gilles Gouaillardet <[email protected]>
1 parent c632784 commit 92441ac

File tree

1 file changed

+76
-60
lines changed

1 file changed

+76
-60
lines changed

opal/util/info.c

Lines changed: 76 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* Copyright (c) 2009 Sun Microsystems, Inc. All rights reserved.
1515
* Copyright (c) 2012-2017 Los Alamos National Security, LLC. All rights
1616
* reserved.
17-
* Copyright (c) 2015 Research Organization for Information Science
17+
* Copyright (c) 2015-2017 Research Organization for Information Science
1818
* and Technology (RIST). All rights reserved.
1919
* Copyright (c) 2016-2017 IBM Corporation. All rights reserved.
2020
* Copyright (c) 2017 Intel, Inc. All rights reserved.
@@ -93,6 +93,72 @@ int opal_info_dup (opal_info_t *info, opal_info_t **newinfo)
9393
return OPAL_SUCCESS;
9494
}
9595

96+
static int opal_info_get_nolock (opal_info_t *info, const char *key, int valuelen,
97+
char *value, int *flag)
98+
{
99+
opal_info_entry_t *search;
100+
int value_length;
101+
102+
search = info_find_key (info, key);
103+
if (NULL == search){
104+
*flag = 0;
105+
} else if (value && valuelen) {
106+
/*
107+
* We have found the element, so we can return the value
108+
* Set the flag, value_length and value
109+
*/
110+
*flag = 1;
111+
value_length = strlen(search->ie_value);
112+
/*
113+
* If the stored value is shorter than valuelen, then
114+
* we can copy the entire value out. Else, we have to
115+
* copy ONLY valuelen bytes out
116+
*/
117+
if (value_length < valuelen ) {
118+
strcpy(value, search->ie_value);
119+
} else {
120+
opal_strncpy(value, search->ie_value, valuelen);
121+
if (OPAL_MAX_INFO_VAL == valuelen) {
122+
value[valuelen-1] = 0;
123+
} else {
124+
value[valuelen] = 0;
125+
}
126+
}
127+
}
128+
return OPAL_SUCCESS;
129+
}
130+
131+
static int opal_info_set_nolock (opal_info_t *info, const char *key, const char *value)
132+
{
133+
char *new_value;
134+
opal_info_entry_t *new_info;
135+
opal_info_entry_t *old_info;
136+
137+
new_value = strdup(value);
138+
if (NULL == new_value) {
139+
return OPAL_ERR_OUT_OF_RESOURCE;
140+
}
141+
142+
old_info = info_find_key (info, key);
143+
if (NULL != old_info) {
144+
/*
145+
* key already exists. remove the value associated with it
146+
*/
147+
free(old_info->ie_value);
148+
old_info->ie_value = new_value;
149+
} else {
150+
new_info = OBJ_NEW(opal_info_entry_t);
151+
if (NULL == new_info) {
152+
free(new_value);
153+
OPAL_THREAD_UNLOCK(info->i_lock);
154+
return OPAL_ERR_OUT_OF_RESOURCE;
155+
}
156+
strncpy (new_info->ie_key, key, OPAL_MAX_INFO_KEY);
157+
new_info->ie_value = new_value;
158+
opal_list_append (&(info->super), (opal_list_item_t *) new_info);
159+
}
160+
return OPAL_SUCCESS;
161+
}
96162
/*
97163
* An object's info can be set, but those settings can be modified by
98164
* system callbacks. When those callbacks happen, we save a "__IN_<key>"/"val"
@@ -131,7 +197,7 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
131197

132198
is_IN_key = 1;
133199
exists_IN_key = 1;
134-
opal_info_get (info, pkey, 0, NULL, &flag);
200+
opal_info_get_nolock (info, pkey, 0, NULL, &flag);
135201
if (flag) {
136202
exists_reg_key = 1;
137203
}
@@ -142,7 +208,7 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
142208
// see if there is an __IN_<key> for the current <key>
143209
if (strlen(iterator->ie_key) + 5 < OPAL_MAX_INFO_KEY) {
144210
sprintf(savedkey, "__IN_%s", iterator->ie_key);
145-
err = opal_info_get (info, savedkey, OPAL_MAX_INFO_VAL,
211+
err = opal_info_get_nolock (info, savedkey, OPAL_MAX_INFO_VAL,
146212
savedval, &flag);
147213
} else {
148214
flag = 0;
@@ -161,7 +227,7 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
161227
// this would mean <key> was set by the user but ignored by the system
162228
// so base our behavior on the omit_ignored
163229
if (!omit_ignored) {
164-
err = opal_info_set(*newinfo, pkey, iterator->ie_value);
230+
err = opal_info_set_nolock(*newinfo, pkey, iterator->ie_value);
165231
if (OPAL_SUCCESS != err) {
166232
OPAL_THREAD_UNLOCK(info->i_lock);
167233
return err;
@@ -186,7 +252,7 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
186252
}
187253
}
188254
if (valptr) {
189-
err = opal_info_set(*newinfo, pkey, valptr);
255+
err = opal_info_set_nolock(*newinfo, pkey, valptr);
190256
if (OPAL_SUCCESS != err) {
191257
OPAL_THREAD_UNLOCK(info->i_lock);
192258
return err;
@@ -212,34 +278,10 @@ int opal_info_dup_mpistandard (opal_info_t *info, opal_info_t **newinfo)
212278
*/
213279
int opal_info_set (opal_info_t *info, const char *key, const char *value)
214280
{
215-
char *new_value;
216-
opal_info_entry_t *new_info;
217-
opal_info_entry_t *old_info;
218-
219-
new_value = strdup(value);
220-
if (NULL == new_value) {
221-
return OPAL_ERR_OUT_OF_RESOURCE;
222-
}
281+
int ret;
223282

224283
OPAL_THREAD_LOCK(info->i_lock);
225-
old_info = info_find_key (info, key);
226-
if (NULL != old_info) {
227-
/*
228-
* key already exists. remove the value associated with it
229-
*/
230-
free(old_info->ie_value);
231-
old_info->ie_value = new_value;
232-
} else {
233-
new_info = OBJ_NEW(opal_info_entry_t);
234-
if (NULL == new_info) {
235-
free(new_value);
236-
OPAL_THREAD_UNLOCK(info->i_lock);
237-
return OPAL_ERR_OUT_OF_RESOURCE;
238-
}
239-
strncpy (new_info->ie_key, key, OPAL_MAX_INFO_KEY);
240-
new_info->ie_value = new_value;
241-
opal_list_append (&(info->super), (opal_list_item_t *) new_info);
242-
}
284+
ret = opal_info_set_nolock(info, key, value);
243285
OPAL_THREAD_UNLOCK(info->i_lock);
244286
return OPAL_SUCCESS;
245287
}
@@ -266,38 +308,12 @@ int opal_info_set_value_enum (opal_info_t *info, const char *key, int value,
266308
int opal_info_get (opal_info_t *info, const char *key, int valuelen,
267309
char *value, int *flag)
268310
{
269-
opal_info_entry_t *search;
270-
int value_length;
311+
int ret;
271312

272313
OPAL_THREAD_LOCK(info->i_lock);
273-
search = info_find_key (info, key);
274-
if (NULL == search){
275-
*flag = 0;
276-
} else if (value && valuelen) {
277-
/*
278-
* We have found the element, so we can return the value
279-
* Set the flag, value_length and value
280-
*/
281-
*flag = 1;
282-
value_length = strlen(search->ie_value);
283-
/*
284-
* If the stored value is shorter than valuelen, then
285-
* we can copy the entire value out. Else, we have to
286-
* copy ONLY valuelen bytes out
287-
*/
288-
if (value_length < valuelen ) {
289-
strcpy(value, search->ie_value);
290-
} else {
291-
opal_strncpy(value, search->ie_value, valuelen);
292-
if (OPAL_MAX_INFO_VAL == valuelen) {
293-
value[valuelen-1] = 0;
294-
} else {
295-
value[valuelen] = 0;
296-
}
297-
}
298-
}
314+
ret = opal_info_get_nolock(info, key, valuelen, value, flag);
299315
OPAL_THREAD_UNLOCK(info->i_lock);
300-
return OPAL_SUCCESS;
316+
return ret;
301317
}
302318

303319
int opal_info_get_value_enum (opal_info_t *info, const char *key, int *value,

0 commit comments

Comments
 (0)