On Tue, 2014-04-08 at 13:23 +0200, Lukas Slebodnik wrote:
On (08/04/14 13:00), Sumit Bose wrote:
>On Mon, Apr 07, 2014 at 06:13:53PM +0200, Sumit Bose wrote:
>> On Mon, Apr 07, 2014 at 02:22:04PM +0200, Pavel Reichl wrote:
>> > Hello,
>> >
>> > I noticed these two warnings in clang.
>> >
>> > It would be great if the 2nd patch could be checked by Sumit to make
>> > sure that the return value wasn't ignored on purpose.
>>
>> yes, I would prefer to ignore errors here. There might be various cases
>> were we are not able to resolve a single SID but still can proceed with
>> the others.
>>
>> The first patch looks good, I'll run some tests and will give my results
>> later.
>
>ACK to the first patch.
>
>bye,
>Sumit
>
Not all return point from function pac_lookup_sids_done were converted
using goto.
--
305- } else if (msg->count > 1) {
306- DEBUG(SSSDBG_CRIT_FAILURE, "More then one result returned "
\
307- "for SID [%s].\n",
308- entries[c].key.str);
309- talloc_free(msg);
310: pac_cmd_done(cctx, EINVAL);
311- return;
^^^^^^^
here
312- }
313-
314- id = ldb_msg_find_attr_as_uint64(msg->msgs[0],
315- SYSDB_UIDNUM, 0);
--
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thank you Lukas for your input. You complicated the situation one more
time. :-)
I had more careful look on the code and I found several places that
seems troublesome to me:
static void pac_lookup_sids_done(struct tevent_req *req)
{
struct pac_req_ctx *pr_ctx = tevent_req_callback_data(req, struct pac_req_ctx);
struct cli_ctx *cctx = pr_ctx->cctx;
[snip]
ret = pac_lookup_sids_recv(req);
talloc_zfree(req);
[snip]
ret = hash_entries(pr_ctx->sid_table, &count, &entries);
Is this use of pr_ctx->sid use-after-free bug as it was allocated in req which was
already freed?
talloc_free(pr_ctx);
pac_cmd_done(cctx, ret);
cctx points to pr_ctx->cctx, but pr_ctx is freed before call of pac_cmd_done(cctx, ret)
- use-after-free?
I don't feel confident enough to make any changes here without approval of somebody
more acquainted with this code.