Skip to content

Commit

Permalink
CLIENT: fix thread unsafe acces to get*ent structs.
Browse files Browse the repository at this point in the history
All get*ent structs were protected with socket mutex. In case SSSD
is built with lock-free client support, `sss_nss_lock()` is a no-op,
thus resulting in thread unsafe access.

This patch changes those structs to have thread local storage.

This conradicts following note in the man page:
```
The function getgrent_r() is not really reentrant since it shares
the reading position in the stream with all other threads.
```
I'm not sure if 3rd party apps can legally assume this behaviour
based on a note in a man page. And in some cases, non-sharing reading
position between threads might make more sense.
But that way or another, this is better than thread unsafe access.

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
  • Loading branch information
alexey-tikhonov authored and pbrezina committed Sep 16, 2022
1 parent 3c99354 commit 69fd828
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 6 deletions.
14 changes: 12 additions & 2 deletions src/sss_client/nss_group.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

/* GROUP database NSS interface */

#include "config.h"

#include <nss.h>
#include <errno.h>
#include <sys/types.h>
Expand All @@ -31,7 +33,11 @@
#include "nss_mc.h"
#include "nss_common.h"

static struct sss_nss_getgrent_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_getgrent_data {
size_t len;
size_t ptr;
uint8_t *data;
Expand All @@ -53,7 +59,11 @@ enum sss_nss_gr_type {
GETGR_GID
};

static struct sss_nss_getgr_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_getgr_data {
enum sss_nss_gr_type type;
union {
char *grname;
Expand Down
8 changes: 7 additions & 1 deletion src/sss_client/nss_hosts.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "config.h"

#include <nss.h>
#include <netdb.h>
#include <resolv.h>
Expand All @@ -33,7 +35,11 @@
#include <string.h>
#include "sss_cli.h"

static struct sss_nss_gethostent_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_gethostent_data {
size_t len;
size_t ptr;
uint8_t *data;
Expand Down
8 changes: 7 additions & 1 deletion src/sss_client/nss_ipnetworks.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "config.h"

#include <nss.h>
#include <netdb.h>
#include <resolv.h>
Expand All @@ -33,7 +35,11 @@
#include <string.h>
#include "sss_cli.h"

static struct sss_nss_getnetent_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_getnetent_data {
size_t len;
size_t ptr;
uint8_t *data;
Expand Down
8 changes: 7 additions & 1 deletion src/sss_client/nss_passwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

/* PASSWD database NSS interface */

#include "config.h"

#include <nss.h>
#include <errno.h>
#include <sys/types.h>
Expand All @@ -30,7 +32,11 @@
#include "nss_mc.h"
#include "nss_common.h"

static struct sss_nss_getpwent_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_getpwent_data {
size_t len;
size_t ptr;
uint8_t *data;
Expand Down
8 changes: 7 additions & 1 deletion src/sss_client/nss_services.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "config.h"

#include <nss.h>
#include <netdb.h>
#include <errno.h>
Expand All @@ -31,7 +33,11 @@
#include <string.h>
#include "sss_cli.h"

static struct sss_nss_getservent_data {
static
#ifdef HAVE_PTHREAD_EXT
__thread
#endif
struct sss_nss_getservent_data {
size_t len;
size_t ptr;
uint8_t *data;
Expand Down

0 comments on commit 69fd828

Please sign in to comment.