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

On Mon, Oct 3, 2016 at 1:19 PM, Michal Privoznik <mprivozn(a)redhat.com&gt; 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()