On Fri, Feb 26, 2016 at 09:01:33PM +0100, Lukas Slebodnik wrote:
> On (26/02/16 16:22), Lukas Slebodnik wrote:
> >On (23/02/16 14:28), Lukas Slebodnik wrote:
> >>On (23/02/16 13:37), Jakub Hrozek wrote:
> >>>On Thu, Feb 18, 2016 at 02:04:54PM +0100, Lukas Slebodnik wrote:
> >>>> ehlo,
> >>>>
> >>>> It took me some time to reproduce issue with cron.
> >>>> It occured very rarely in my case (twice in a week).
> >>>>
> >>>> Therefore I prepared different reproducer "mini_cron.c"
> >>>> attached in mail. It tries to query for data in the interval
> >>>> (9.990 sec .. 10.069 sec) when responder might destroy connection.
> >>>> mini_cron expect there is a user mof_user1 in LDAP and
> >>>> client_idle_timeout
> >>>> has minimal value 10 seconds. Default is 60 seconds. It's also good to
> >>>> decrease memory cache timeout to ensure connection to responder every
> >>>> time.
> >>>>
> >>>> e.g.
> >>>> [sssd]
> >>>> config_file_version = 2
> >>>> services = nss, pam
> >>>> domains = LDAP
> >>>> client_idle_timeout = 10
> >>>>
> >>>> [nss]
> >>>> filter_groups = root
> >>>> filter_users = root
> >>>> memcache_timeout = 0
> >>>> client_idle_timeout = 10
> >>>> debug_level =9
> >>>> debug_microseconds = true
> >>>>
> >>>> Detailed explanation is in commit message.
> >>>>
> >>>> Attached is also a debug patch which I used as part of
> >>>> analysis when it can fail.
> >>>
> >>>Thank you.
> >>>
> >>>>
> >>>> BTW I ran mini_cron reprodurer for a week and it didn't fail.
> >>>>
> >>>> LS
> >>>
> >>>The code looks good to me and sanity testing of the clients passed as
> >>>well -> ACK
> >>>
> >>>CI is still running.
> >>I realized that I didn't fix it in pam client.
> >>
> >Updated version is attached.
> >
> There were static analyzer warnings
>
> Error: UNUSED_VALUE (CWE-563): [#def2]
> sssd-1.13.90/src/sss_client/common.c:937: value_overwrite: Overwriting
> previous write to "ret" with value from "sss_cli_check_socket(errnop,
> "/var/lib/sss/pipes/autofs")".
> sssd-1.13.90/src/sss_client/common.c:935: assigned_value: Assigning value
> "SSS_STATUS_UNAVAIL" to "ret" here, but that stored value is overwritten
> before it can be used.
> # 933| int *errnop)
> # 934| {
> # 935|-> enum sss_status ret = SSS_STATUS_UNAVAIL;
> # 936|
> # 937| ret = sss_cli_check_socket(errnop, SSS_AUTOFS_SOCKET_NAME);
>
> Updated version is attached.
>
> LS
I'm sorry about the delay in review. I sanity-tested all responders to
make sure they work and included the patch in some downstream and IPA
QE tests I ran recently. I admit I didn't test the bug test case itself,
but seeing it was tested by users who hit the bug, I trust their
testing.
ACK
CI: http://sssd-ci.duckdns.org/logs/job/39/03/summary.html
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
