[SSSD] [PATCH] AD: Add initial gpo-smb implementation.

Lukas Slebodnik lslebodn at redhat.com
Fri Jun 27 18:39:09 UTC 2014


On (25/06/14 23:33), Yassir Elley wrote:
>
>I have attached a revised first patch which addresses the code review
>comments. I'm not sure if I needed to (since it has already been ACK'ed),
>but I have also attached an unchanged second patch.
>
>Thanks,
>Yassir.

I know that Sumit already ACKed these patches, but I used them to test
CI script and I found some minor issues
+ warnings from static analysers (as usual :-)

>From b58f39645872bc1dd878ac650861af89964101f9 Mon Sep 17 00:00:00 2001
>From: Yassir Elley <yelley at redhat.com>
>Date: Wed, 21 May 2014 15:11:36 -0400
>Subject: [PATCH 1/2] AD-GPO: Add gpo-smb implementation in gpo_child process
>
>---
> Makefile.am                     |  22 +
> contrib/sssd.spec.in            |   2 +-
> src/external/samba.m4           |   8 +
> src/providers/ad/ad_gpo.c       | 887 +++++++++++++++++++++++++++++++++++++++-
> src/providers/ad/ad_gpo_child.c | 637 +++++++++++++++++++++++++++++
> 5 files changed, 1543 insertions(+), 13 deletions(-)
> create mode 100644 src/providers/ad/ad_gpo_child.c
>
>diff --git a/Makefile.am b/Makefile.am
>index 83999f3584fe130320ebfa6ea459f5706992c6a4..6cf5d8cb0c7c68314dad49461e7cd1fb4452bc1b 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -121,6 +121,9 @@ endif
> if BUILD_IFP
> sssdlibexec_PROGRAMS += sssd_ifp
> endif
>+if BUILD_SAMBA
>+sssdlibexec_PROGRAMS += gpo_child
>+endif
> 
> 
> if BUILD_PAC_RESPONDER
>@@ -2298,6 +2301,25 @@ ldap_child_LDADD = \
>     $(DHASH_LIBS) \
>     $(KRB5_LIBS)
> 
>+gpo_child_SOURCES = \
>+    src/providers/ad/ad_gpo_child.c \
>+    src/util/atomic_io.c \
>+    src/util/util.c \
>+    src/util/signal.c
>+gpo_child_CFLAGS = \
>+    $(AM_CFLAGS) \
>+    $(POPT_CFLAGS) \
>+    $(KRB5_CFLAGS) \
>+    $(INI_CONFIG_CFLAGS) \
>+    $(SMBCLIENT_CFLAGS)
>+gpo_child_LDADD = \
>+    libsss_debug.la \
>+    $(TALLOC_LIBS) \
>+    $(POPT_LIBS) \
>+    $(DHASH_LIBS) \
>+    $(INI_CONFIG_LIBS) \
>+    $(SMBCLIENT_LIBS)
>+
> proxy_child_SOURCES = \
>     src/providers/proxy/proxy_child.c \
>     src/providers/data_provider_iface_generated.c \
>diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>index b5e7903713ccc6944fbed4f84c2f0413a257b760..10704ae48cf5636de2a5cbbb42fc00b99052144f 100644
>--- a/contrib/sssd.spec.in
>+++ b/contrib/sssd.spec.in
>@@ -97,7 +97,7 @@ BuildRequires: libtdb-devel
> BuildRequires: libldb-devel
> BuildRequires: libdhash-devel >= 0.4.2
> BuildRequires: libcollection-devel
>-BuildRequires: libini_config-devel
>+BuildRequires: libini_config-devel >= 1.1
> BuildRequires: dbus-devel
> BuildRequires: dbus-libs
> %if 0%{?rhel5_minor} >= 7
>diff --git a/src/external/samba.m4 b/src/external/samba.m4
>index 735cc5a187dba7514a9c7436a6d3ba948ad522b0..bf790365468da5ccdd40a260d74893490b7e7be8 100644
>--- a/src/external/samba.m4
>+++ b/src/external/samba.m4
>@@ -19,4 +19,12 @@ If you do not want to build these providers it is possible to build SSSD
> without them. In this case, you will need to execute configure script
> with argument --without-samba
>     ]]))
>+
>+    PKG_CHECK_MODULES(INI_CONFIG, ini_config >= 1.0.0, ,
                                                  ^^^^^
                                                  1.1.0
                          You used correct version in spec file.
configure will pass with old version of ding-libs and compilation fail in
gpo_child.


>+        AC_MSG_ERROR([[Please install libini_config development libraries.
>+libini_config libraries are necessary for building ad and ipa provider.
                                                      ^^^^^^^^^
                                        You touch this code. I might be good to
                                        slightly modify text. "ad with gpo"
                                        or better wording.
Sorry, for nitpicking. I would ignore it there was not problem with version.

>+If you do not want to build these providers it is possible to build SSSD
>+without them. In this case, you will need to execute configure script
>+with argument --without-samba
>+    ]]))
> fi

//snip

>+/* == ad_gpo_process_cse_send/recv helpers ================================= */
>+
>+static errno_t
>+create_cse_send_buffer(TALLOC_CTX *mem_ctx,
>+                       char *smb_uri,
>+                       struct io_buffer **io_buf)
>+{
>+    struct io_buffer *buf;
>+    size_t rp;
>+    int smb_uri_length;
>+
>+    smb_uri_length = strlen(smb_uri);
>+
>+    DEBUG(SSSDBG_TRACE_FUNC, "smb_uri: %s\n", smb_uri);
>+    DEBUG(SSSDBG_TRACE_FUNC, "strlen(smb_uri): %d\n", smb_uri_length);
>+
>+    buf = talloc(mem_ctx, struct io_buffer);
>+    if (buf == NULL) {
>+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc failed.\n");
>+        return ENOMEM;
>+    }
>+
>+    buf->size = 1 * sizeof(uint32_t);
>+    if (smb_uri) {
      I thing that this test is redundant, because strlen would crash anyway
      with NULL.

Error: REVERSE_INULL
sssd-1.11.92/src/providers/ad/ad_gpo.c:2792: deref_ptr_in_call: Dereferencing pointer "smb_uri".
sssd-1.11.92/src/providers/ad/ad_gpo.c:2804: check_after_deref: Null-checking "smb_uri" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

>+        buf->size += smb_uri_length;
>+    }
>+
>+    DEBUG(SSSDBG_TRACE_ALL, "buffer size: %zu\n", buf->size);
>+
>+    buf->data = talloc_size(buf, buf->size);
>+    if (buf->data == NULL) {
>+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_size failed.\n");
>+        talloc_free(buf);
>+        return ENOMEM;
>+    }
>+
>+    rp = 0;
>+    /* smb_uri */
>+    if (smb_uri) {
>+        SAFEALIGN_SET_UINT32(&buf->data[rp], smb_uri_length, &rp);
>+        safealign_memcpy(&buf->data[rp], smb_uri, smb_uri_length, &rp);
>+    } else {
>+        SAFEALIGN_SET_UINT32(&buf->data[rp], 0, &rp);
>+    }
>+
>+    *io_buf = buf;
>+    return EOK;
>+}

I am attaching another html report from clang.

I let final ACK for Sumit.

LS
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smb_path.html.gz
Type: application/gzip
Size: 59261 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140627/a80b2073/attachment.bin>


More information about the sssd-devel mailing list