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