[libvirt] [PATCH v2 1/2] NSS: Add explicit check to not report expired lease

[Michal Privoznik]

On 03.10.2016 09:54, Nehal J Wani wrote:
> On Mon, Oct 3, 2016 at 1:19 PM, Michal Privoznik <mprivozn at redhat.com> wrote:
>> On 30.09.2016 17:11, Nehal J Wani wrote:
>>> The NSS module shouldn't rely on custom leases database to not have
>>> entries for leases which have expired.
>>>
>>> Change-Id: Ic3e043003d33ded0da74696a1d27ed4967ddbfb8
>>
>> Oh, is this a result of some git hook script?
> 
> Yes, indeed. I forgot to remove it. My bad. #GerritChronicles
>>
>>> ---
>>>  tools/nss/libvirt_nss.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
>>> index 54c4a2a..cb3edf5 100644
>>> --- a/tools/nss/libvirt_nss.c
>>> +++ b/tools/nss/libvirt_nss.c
>>> @@ -42,6 +42,7 @@
>>>  #include "virlease.h"
>>>  #include "viralloc.h"
>>>  #include "virfile.h"
>>> +#include "virtime.h"
>>>  #include "virerror.h"
>>>  #include "virstring.h"
>>>  #include "virsocketaddr.h"
>>> @@ -114,6 +115,8 @@ findLease(const char *name,
>>>      ssize_t i, nleases;
>>>      leaseAddress *tmpAddress = NULL;
>>>      size_t ntmpAddress = 0;
>>> +    time_t currtime;
>>> +    long long expirytime;
>>>
>>>      *address = NULL;
>>>      *naddress = 0;
>>> @@ -161,6 +164,11 @@ findLease(const char *name,
>>>      nleases = virJSONValueArraySize(leases_array);
>>>      DEBUG("Read %zd leases", nleases);
>>>
>>> +    if ((currtime = time(NULL)) == (time_t) - 1) {
>>> +        ERROR("Failed to get current system time");
>>> +        goto cleanup;
>>> +    }
>>> +
>>>      for (i = 0; i < nleases; i++) {
>>>          virJSONValuePtr lease;
>>>          const char *lease_name;
>>> @@ -181,6 +189,16 @@ findLease(const char *name,
>>>          if (STRNEQ_NULLABLE(name, lease_name))
>>>              continue;
>>>
>>> +        if (virJSONValueObjectGetNumberLong(lease, "expiry-time", &expirytime) < 0) {
>>> +            /* A lease cannot be present without expiry-time */
>>> +            ERROR("expiry-time field missing for %s", name);
>>> +            goto cleanup;
>>> +        }
>>> +
>>> +        /* Do not report expired lease */
>>> +        if (expirytime < (long long) currtime)
>>
>> I am putting here a debug message (which is no-op unless enabled) as it
>> might help me in debugging in the future.
> 
> Maybe we should at that debug message at other places too, where we do
> this, for example, in networkGetDHCPLeases()

Maybe, but nss module is kind of special. I mean sometimes it's just
very hard to debug (e.g. the nss plugins are loaded on-demand, glibc is
very optimized and thus a lot of symbols are missing, etc.) whereas I
found libvirt drivers to be slightly better in this respect.

Michal