On (15/06/16 19:08), Lukas Slebodnik wrote:
On (15/06/16 14:08), Pavel Březina wrote:
>On 06/15/2016 08:44 AM, Lukas Slebodnik wrote:
>> On (14/06/16 15:30), Pavel Březina wrote:
>> > On 05/16/2016 02:00 PM, Pavel Březina wrote:
>> > > Hi,
>> > > the patches are finally ready to be tested and reviewed. It is too
huge
>> > > to be sent to the list so please checkout my fedorapeople or github
repo:
>> > >
>> > >
https://fedorapeople.org/cgit/pbrezina/public_git/sssd.git/log/?h=backend
>> > >
https://github.com/pbrezina/sssd/tree/backend
>> > >
>> > > Subdomain handlers are not yet converted so subdomain support is
>> > > disabled, otherwise everything should work although I'm sure
you'll find
>> > > some bugs.
>> > >
>> > > I managed to do some simple tests (initgroups, authentication) with
ldap
>> > > provider so far and will continue testing and fixing so if you find a
>> > > bug make sure you run with the latest version before reporting it
please
>> > > :-)
>> > >
>> > > Since the changes touch almost all areas of SSSD I encourage everyone
to
>> > > run and try. Some handlers were converted quite easily, some took more
>> > > handy work. Areas that are most likely to contain some bugs are these
>> > > (please give it extra attention):
>> > >
>> > > - proxy provider
>> > > - if group membership changes during initgroups, nss memory cache
should
>> > > be clear through dbus call
>> > > - selinux support
>> > > - hbac support
>> > > - change password
>> > > - password migration (ipa)
>> > >
>> > > Don't be alarmed with the number of new lines -- there is not that
many
>> > > changes. I copied all touched files and suffixed them with _new so
sssd
>> > > can be compiled and kept working until the latest patches. I also
wanted
>> > > to keep the original code intact for comparison (it was easier for
>> > > development and it may be handy for bug chasing), you can simply use
>> > > some diff tool to see the changes. We can squash it in the end.
>> > >
>> > > When I will be confident that the patches are stable I will do some
>> > > clean up and remove content that is no longer needed.
>> >
>> > Just a quick update: all found bugs were fixed, CI pass, all downstream
tests
>> > which were tried (I think all but IPA) pass.
>>
>> 30 errno_t dp_host_handler(struct sbus_request *sbus_req,
>> 31 void *dp_cli,
>> 32 uint32_t dp_flags,
>> 33 const char *name,
>> 34 const char *alias)
>> 35 {
>> 36 struct dp_hostid_data *data;
>> 37 const char *key;
>> 38
>> 39 if (name == NULL) {
>> 40 return EINVAL;
>> 41 }
>> 42
>> 43 data = talloc_zero(sbus_req, struct dp_hostid_data);
>> 44 if (data == NULL) {
>> 45 return ENOMEM;
>> 46 }
>> 47
>> 48 data->name = name;
>> 49 data->alias = alias[0] == '\0' ? NULL : alias;
>> 50
>> 51 key = talloc_asprintf("%s:%s", name, (alias == '\0'
? "(null)" : alias));
>> ^^^
>> The 1st argument should be a talloc context
>> otherwise it will CRASH.
>>
>> 52 if (key == NULL) {
>> 53 talloc_free(data);
>> 54 return ENOMEM;
>> 55 }
>
>Thanks fixed.
>
>>
>> I can see a gcc warning on my fedora 24
>> src/providers/ad/ad_subdomains_new.c: In function
‘ad_subdomains_refresh_root_done’:
>> src/providers/ad/ad_subdomains_new.c:575:23: warning: ‘root_attrs’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
>> state->root_attrs = root_attrs;
>> ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
>> src/providers/ad/ad_subdomains_new.c:1161:25: note: ‘root_attrs’ was declared
here
>> struct sysdb_attrs *root_attrs;
>> ^~~~~~~~~~
>> src/providers/ad/ad_subdomains_new.c:1194:14: warning: ‘root_id_ctx’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
>> subreq = ad_get_slave_domain_send(state, state->ev, state->sd_ctx,
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> root_attrs, root_id_ctx->ldap_ctx);
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>I seen this in Coverity also reported with root_attrs maybe uninitialized. It
>seems to me as false positive but maybe I miss something?
>
It's verly likely some gcc-6 otimisation.
I will try to take a look.
I think that gcc want to tell us this
1156 static void ad_subdomains_refresh_root_done(struct tevent_req *subreq)
1157 {
1158 struct ad_subdomains_refresh_state *state;
1159 struct tevent_req *req;
1160 struct ad_id_ctx *root_id_ctx;
1161 struct sysdb_attrs *root_attrs;
1162 int dp_error;
1163 errno_t ret;
1164
1165 req = tevent_req_callback_data(subreq, struct tevent_req);
1166 state = tevent_req_data(req, struct ad_subdomains_refresh_state);
1167
1168 ret = ad_get_root_domain_recv(state, subreq, &root_attrs, &root_id_ctx);
1169 talloc_zfree(subreq);
1170 if (ret != EOK) {
1171 DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get forest root [%d]: %s\n",
1172 ret, sss_strerror(ret));
1173 /* We continue to finish sdap_id_op. */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
root_attrs and root_id_ctx needn't be initialized after this line.
1174 }
1175
LS