[SSSD] Warn to syslog if an unresponsive subprocess is terminated

Dmitri Pal dpal at redhat.com
Wed Dec 4 17:30:39 UTC 2013


On 12/04/2013 12:14 PM, Jakub Hrozek wrote:
> On Mon, Dec 02, 2013 at 07:41:19PM +0100, Pavel Reichl wrote:
>> Hello,
>>
>> first four patches address issues of freeing mt_svc structure. Last one
>> adds warns to syslog.
>>
>> Pavel Reichl
> The patches work well and the code looks good. I have a couple of very
> small nitpicks, see inline.
>
>> From 1d44284275e9f227d2f39085c6a6661a04e9b69c Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <pavel.reichl at redhat.com>
>> Date: Mon, 2 Dec 2013 14:51:59 +0000
>> Subject: [PATCH 1/5] monitor: use-after-free bugfix
>>
>> *monitor_kill_service* may create timed event which operates on *svc* and
>> therefore *svc* should not be freed right after call of *monitor_kill_sercice*.
>> *svc* is supposed to be freed by *mt_svc_restart*.
>> ---
>>  src/monitor/monitor.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
>> index 92bac422a53b450f29a2d5b23217b530030781c2..ce4b1adb12626517d5cfa0a932fb6c6198cd2858 100644
>> --- a/src/monitor/monitor.c
>> +++ b/src/monitor/monitor.c
>> @@ -727,7 +727,6 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal)
>>      if (!msg) {
>>          DEBUG(0,("Out of memory?!\n"));
> We're trying to move away from the old decimal debug levels in the code,
> can you change the number 0 to a SSSDBG_ macro while you're changing the
> code?
>
>>          monitor_kill_service(svc);
>> -        talloc_free(svc);
> This change is correct, nice catch btw.
>
>>          return ENOMEM;
>>      }
>>  
>> -- 
>> 1.8.3.1
>>
>> From 60033e161524a27587d409ae8f4807a9d70c8d02 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <pavel.reichl at redhat.com>
>> Date: Mon, 2 Dec 2013 15:20:01 +0000
>> Subject: [PATCH 2/5] monitor: use-after-free bugfix
> ACK
>
>
>> From b312127656465a30d7b5e93e661cacea592bdc99 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <pavel.reichl at redhat.com>
>> Date: Mon, 2 Dec 2013 15:37:57 +0000
>> Subject: [PATCH 3/5] monitor: check tevent_add_timer & remove sigkill_ev
>> @@ -629,8 +628,13 @@ static int monitor_kill_service (struct mt_svc *svc)
>>       * doesn't exit within sixty seconds
>>       */
>>      tv = tevent_timeval_current_ofs(svc->kill_time, 0);
>> -    svc->sigkill_ev = tevent_add_timer(svc->mt_ctx->ev, svc, tv,
>> -                                       mt_svc_sigkill, svc);
>> +    te = tevent_add_timer(svc->mt_ctx->ev, svc, tv, mt_svc_sigkill, svc);
>> +    if (te == NULL) {
>> +        /* Nothing much we can do */
>> +        ret = ENOMEM;
>> +        DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory?!\n"));
>> +        talloc_free(svc);
> I agree with removing the sigkill_ev, but I would prefer if the function
> either returned ENOMEM here directly or jumped to return. I think the
> fallthrough is potentially dangerous.


Can we also just in general be more explicit. May be use something like:
"Out of memory trying to allocate X to do Y"

>
>> +    }
>>  
>>      return ret;
>>  }
>> -- 
>> 1.8.3.1
>>
>> From 748aa120e07f20b022de29589c3e2179aa2c8840 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <pavel.reichl at redhat.com>
>> Date: Mon, 2 Dec 2013 15:49:04 +0000
>> Subject: [PATCH 4/5] monitor: memory-leak bug
> ACK
>
>
>> From b1bcf5cd7f73735ebe251a692ee989e63c0158bb Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <pavel.reichl at redhat.com>
>> Date: Mon, 2 Dec 2013 17:22:55 +0000
>> Subject: [PATCH 5/5] monitor: syslog when process killed by monitor
> ACK
>
> I tested service restarts (by sending SIGSEGV to sssd_be), service kills
> by stopping the sssd_be in gdb. The services are restarted as
> appropriate and the syslog contains the new message.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/






More information about the sssd-devel mailing list