[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Lukas Slebodnik]

On (17/03/16 09:23), Jakub Hrozek wrote:
>On Tue, Mar 15, 2016 at 11:22:42AM +0100, Pavel Březina wrote:
>> On 03/15/2016 10:36 AM, Pavel Březina wrote:
>> >On 03/11/2016 05:04 AM, Simo Sorce wrote:
>> >>On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:
>> >>>The attached patch fixes #2973,
>> >>>it's pretty straightforward.
>> >>
>> >>Same patch but fixed the typos in the commit message.
>> >>
>> >>Simo.
>> >
>> >Makes sense. Ack.
>> 
>> CI http://sssd-ci.duckdns.org/logs/job/39/53/summary.html
>
>* master: 600e0429c58081c080cc283a0d4619dff920296f
Can this fix be related to following crash
https://bugzilla.redhat.com/show_bug.cgi?id=1283769 ?

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Jakub Hrozek]

On Tue, Mar 15, 2016 at 11:22:42AM +0100, Pavel Březina wrote:
> On 03/15/2016 10:36 AM, Pavel Březina wrote:
> >On 03/11/2016 05:04 AM, Simo Sorce wrote:
> >>On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:
> >>>The attached patch fixes #2973,
> >>>it's pretty straightforward.
> >>
> >>Same patch but fixed the typos in the commit message.
> >>
> >>Simo.
> >
> >Makes sense. Ack.
> 
> CI http://sssd-ci.duckdns.org/logs/job/39/53/summary.html

* master: 600e0429c58081c080cc283a0d4619dff920296f
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Jakub Hrozek]

On Thu, Mar 17, 2016 at 09:49:20AM +0100, Lukas Slebodnik wrote:
> On (17/03/16 09:23), Jakub Hrozek wrote:
> >On Tue, Mar 15, 2016 at 11:22:42AM +0100, Pavel Březina wrote:
> >> On 03/15/2016 10:36 AM, Pavel Březina wrote:
> >> >On 03/11/2016 05:04 AM, Simo Sorce wrote:
> >> >>On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:
> >> >>>The attached patch fixes #2973,
> >> >>>it's pretty straightforward.
> >> >>
> >> >>Same patch but fixed the typos in the commit message.
> >> >>
> >> >>Simo.
> >> >
> >> >Makes sense. Ack.
> >> 
> >> CI http://sssd-ci.duckdns.org/logs/job/39/53/summary.html
> >
> >* master: 600e0429c58081c080cc283a0d4619dff920296f
> Can this fix be related to following crash
> https://bugzilla.redhat.com/show_bug.cgi?id=1283769 ?
> 
> LS

I wouldn't think so. The fix was about properly closing client code, the
crash happened when a provider went away but the responder kept sending
it sbus messages..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Pavel Březina]

On 03/15/2016 10:36 AM, Pavel Březina wrote:

On 03/11/2016 05:04 AM, Simo Sorce wrote:

On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:

The attached patch fixes #2973,
it's pretty straightforward.


Same patch but fixed the typos in the commit message.

Simo.


Makes sense. Ack.


CI http://sssd-ci.duckdns.org/logs/job/39/53/summary.html
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Pavel Březina]

On 03/11/2016 05:04 AM, Simo Sorce wrote:

On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:

The attached patch fixes #2973,
it's pretty straightforward.


Same patch but fixed the typos in the commit message.

Simo.


Makes sense. Ack.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix responders socket cleanup code

[Simo Sorce]

On Thu, 2016-03-10 at 21:04 -0500, Simo Sorce wrote:
> The attached patch fixes #2973,
> it's pretty straightforward.

Same patch but fixed the typos in the commit message.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From 93a636ca1283ca9b2bfbda55684eec43afff5c06 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Thu, 10 Mar 2016 20:52:43 -0500
Subject: [PATCH] Responders: Fix client destructor

To close a socket associated to an fd event we must set the close
function of the event and not associate a destructor to a parent context.

Otherwise the destructor will close() the socket before the fd event is
freed, and this may cause invalid calls on a closed file descriptor to
poll/epoll/etc.

Discovered by looking at strace output.

Resolves:
https://fedorahosted.org/sssd/ticket/2973
---
 src/responder/common/responder_common.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 6ac1ea984442792a21b53869a2911b431255..982318647ee9ee7d6795621c03ad8cf53fb78f43 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -66,9 +66,12 @@ static errno_t set_close_on_exec(int fd)
 return EOK;
 }
 
-static int client_destructor(struct cli_ctx *ctx)
+static void client_close_fn(struct tevent_context *ev,
+struct tevent_fd *fde, int fd,
+void *ptr)
 {
 errno_t ret;
+struct cli_ctx *ctx = talloc_get_type(ptr, struct cli_ctx);
 
 if ((ctx->cfd > 0) && close(ctx->cfd) < 0) {
 ret = errno;
@@ -80,7 +83,8 @@ static int client_destructor(struct cli_ctx *ctx)
 DEBUG(SSSDBG_TRACE_INTERNAL,
   "Terminated client [%p][%d]\n",
ctx, ctx->cfd);
-return 0;
+
+ctx->cfd = -1;
 }
 
 static errno_t get_client_cred(struct cli_ctx *cctx)
@@ -474,12 +478,11 @@ static void accept_fd_handler(struct tevent_context *ev,
accept_ctx->is_private ? " on privileged pipe" : "");
 return;
 }
+tevent_fd_set_close_fn(cctx->cfde, client_close_fn);
 
 cctx->ev = ev;
 cctx->rctx = rctx;
 
-talloc_set_destructor(cctx, client_destructor);
-
 /* Set up the idle timer */
 ret = reset_idle_timer(cctx);
 if (ret != EOK) {
-- 
2.5.0

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org