[PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[Florian Weimer]

If a parse error is encountered, fail with NO_RECOVERY and
NSS_STATUS_UNAVAIL.

2019-03-08  Florian Weimer  <fweimer@redhat.com>

	* resolv/nss_dns/dns-network.c (getanswer_r): Handle unparseable
	messages more consistently.

diff --git a/resolv/nss_dns/dns-network.c b/resolv/nss_dns/dns-network.c
index 9c0082d270..68266d57c9 100644
--- a/resolv/nss_dns/dns-network.c
+++ b/resolv/nss_dns/dns-network.c
@@ -336,14 +336,11 @@ getanswer_r (const querybuf *answer, int anslen, struct netent *result,
 	bp[0] = '\0';
 
       if (n < 0)
-	break;
+	goto bad_message;
       cp += n;
 
       if (end_of_message - cp < 10)
-	{
-	  __set_h_errno (NO_RECOVERY);
-	  return NSS_STATUS_UNAVAIL;
-	}
+	goto bad_message;
 
       int type, class;
       GETSHORT (type, cp);
@@ -352,10 +349,7 @@ getanswer_r (const querybuf *answer, int anslen, struct netent *result,
       uint16_t rdatalen;
       GETSHORT (rdatalen, cp);
       if (end_of_message - cp < rdatalen)
-	{
-	  __set_h_errno (NO_RECOVERY);
-	  return NSS_STATUS_UNAVAIL;
-	}
+	goto bad_message;
 
       if (class == C_IN && type == T_PTR)
 	{
@@ -370,13 +364,7 @@ getanswer_r (const querybuf *answer, int anslen, struct netent *result,
 	    }
 
 	  if (n < 0)
-	    {
-	      /* XXX What does this mean?  The original form from bind
-		 returns NULL. Incrementing cp has no effect in any case.
-		 What should I return here. ??? */
-	      cp += n;
-	      return NSS_STATUS_UNAVAIL;
-	    }
+	    goto bad_message;
 	  cp += rdatalen;
          if (alias_pointer + 2 < &net_data->aliases[MAX_NR_ALIASES])
            {

Re: [PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> If a parse error is encountered, fail with NO_RECOVERY and
> NSS_STATUS_UNAVAIL.
>
> 2019-03-08  Florian Weimer  <fweimer@redhat.com>
>
> 	* resolv/nss_dns/dns-network.c (getanswer_r): Handle unparseable
> 	messages more consistently.

LGTM but depends on the errno question from the previous (dependency)
patch.

Re: [PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> If a parse error is encountered, fail with NO_RECOVERY and
>> NSS_STATUS_UNAVAIL.
>>
>> 2019-03-08  Florian Weimer  <fweimer@redhat.com>
>>
>> 	* resolv/nss_dns/dns-network.c (getanswer_r): Handle unparseable
>> 	messages more consistently.
>
> LGTM but depends on the errno question from the previous (dependency)
> patch.

Setting errno to any value is fine as long it's not 0 or ERANGE. 8-)

Thanks,
Florian

Re: [PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> Setting errno to any value is fine as long it's not 0 or ERANGE. 8-)

I was thinking back to a conversation (maybe here) about polluting errno
by setting it *at all* in functions that should only set h_errno.  So, I
flagged a change in whether or not errno was set.  I don't know if that
rule applies in this case, but we should at least know if it does or
not, and say so, so we don't unintentionally make things worse.

Re: [PATCH] nss_dns: More consistency in corrupt message handling in getnetby*

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> Setting errno to any value is fine as long it's not 0 or ERANGE. 8-)
>
> I was thinking back to a conversation (maybe here) about polluting errno
> by setting it *at all* in functions that should only set h_errno.

I'm not aware of such a discussion.  The h_errno == NETDB_INTERNAL
definitely needs to set errno, to name just one example.

What do you think about the nss_dns patch, given this aspect?

Thanks,
Florian