Re: Stats Collector Error 7.4beta1 and 7.4beta2 - Mailing list pgsql-hackers

From Jan Wieck
Subject Re: Stats Collector Error 7.4beta1 and 7.4beta2
Date
Msg-id 3F58D02A.5020300@Yahoo.com
Whole thread Raw
In response to Re: Stats Collector Error 7.4beta1 and 7.4beta2  (Jan Wieck <JanWieck@Yahoo.com>)
Responses Re: Stats Collector Error 7.4beta1 and 7.4beta2
List pgsql-hackers
On a second thought,

I still believe that this is just garbage in the padding bytes after the 
IPV4 address. The code currently bind()'s and connect()'s explicitly to 
an AF_INET address. So all we ever should see is something from and 
AF_INET address. Everything else in the sin_family has to be discarded. 
I do not think it is allowed to bind() and connect() to an IPV4 address 
and then get anything other than an IPV4 address back from the system. 
If that is the case, the whole idea is broken.

An AF_INET address now has only two relevant fields, the sin_port and 
sin_addr. If they are the same, everything is fine. So the correct check 
would be that 1. fromlen > sizeof(sin_family), 2. sin_family == AF_INET, 
3. sin_port and sin_addr identical.

After reading Kurt's quoting of the SUS manpage I have to agree with Tom 
in that we cannot skip the check entirely. It says it limits for recv() 
but we are using recvfrom() ... there might be a little difference on 
that platform ...


Jan Wieck wrote:

> 
> Tom Lane wrote:
> 
>> Jan Wieck <JanWieck@Yahoo.com> writes:
>>> Redhat 7.1 says
>>>         The file descriptor sockfd must refer to a socket.  If the
>>>         socket is of type SOCK_DGRAM then the serv_addr address is
>>>         the address to which datagrams are sent  by  default,  and
>>>         the  only  address  from which datagrams are received.  If
>> 
>>> Looks like the test is obsolete. Any objections to remove it? Do people 
>>> agree that it's a bugfix that can go into 7.4?
>> 
>> The test is not "obsolete"; we understood when we wrote it that it
>> should theoretically be redundant with the kernel-level restriction.
>> But there may be systems out there that fail to make the check promised
>> by the HPUX and Linux manpages.  So I'd prefer to leave it in there if
>> we can.  I don't want to rip it out simply because we don't understand
>> why it's failing on Adam's setup.
>> 
>> What I'm wondering about is whether we are comparing the right number of
>> bytes ... have both address structs been reported to have the same
>> length?  Maybe we need a min().
> 
> I disagree. If getsockname(), getpeername() or recvfrom() return 
> different address length's, it'd be more an indicator that the addresses 
> ARE different anyway. Just because an IPV4 address doesn't have that 
> feature is no reason to assume that addresses of different length are 
> the same if they match over min() bytes.
> 
> If you want to continue to check the addresses and feel that HPUX and 
> Linux manpage claims about kernel behaviour aren't enough, then we have 
> to make it a specific check per "supported" AF. For now, that'd just be 
> AF_INET and AF_INET6, and the default case bailing out with an error.
> 
> 
> Jan
> 

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #



pgsql-hackers by date:

Previous
From: Jeroen Ruigrok/asmodai
Date:
Subject: Re: 64-bit pgsql
Next
From: Andrew Dunstan
Date:
Subject: initdb in C