netdev - Re: [PATCH v3 net-next 0/3] rds: IPv6 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 19 Jul 2018 11:10:03 +0800
From:   Ka-Cheong Poon <ka-cheong.poon@...cle.com>
To:     David Miller <davem@...emloft.net>, sowmini.varadhan@...cle.com
Cc:     netdev@...r.kernel.org, santosh.shilimkar@...cle.com,
        rds-devel@....oracle.com
Subject: Re: [PATCH v3 net-next 0/3] rds: IPv6 support

On 07/19/2018 01:31 AM, David Miller wrote:
> From: Sowmini Varadhan <sowmini.varadhan@...cle.com>
> Date: Wed, 18 Jul 2018 03:33:40 -0700
> 
>> On (07/18/18 15:19), Ka-Cheong Poon wrote:
>>>> bind() and connect() are using the sa_family/ss_family to have
>>>> the application signal to the kernel about whether ipv4 or ipv6 is
>>>> desired. (and bind and connect are doing the right thing for
>>>> v4mapped, so that doesnt seem to be a problem there)
>>>>
>>>> In this case you want the application to signal that info via
>>>> the optlen.  (And the reason for this inconsistency is that you dont
>>>> want to deal with the user->kernel copy in the same way?)
>>>
>>>
>>> Because doing that can break existing RDS apps.  Existing code
>>> does not check the address family in processing this socket
>>> option.  It only cares about the address and port.  If the new
>>
>> I'll leave this up to DaveM. Existing code only handles IPv4,
>>
>> everywhere else, we always check the sa_family or ss_family
>> first and verify the length afterward. This was DaveM's original
>> point about bind/connect/sendmsg. I dont know why rds sockopts have
>> to be special.
> 
> Yes, but the above point is valid.
> 
> If the code never verified the sa_family value before, it is a very
> real possibility that code exists out that which is not initializing
> it or setting it incorrectly.
> 
> Those apps have worked for a long time, and suddenly will break.
> 
> We often have to deal with unfortunate mistakes like this.
> 
> But for now, I guess the check can be added but we have to look out
> for any regressions this causes and revert if necessary.


Is it OK not to do the check for this patch?  From a
customer's perspective, breaking working apps is a
really bad thing unless there is a very special
reason, such as security issue.  Do you see a very
important problem, such as security issue, for not
adding the check in this patch?

Thanks.


-- 
K. Poon
ka-cheong.poon@...cle.com