From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ka-Cheong Poon Subject: Re: [PATCH v3 net-next 0/3] rds: IPv6 support Date: Thu, 19 Jul 2018 11:10:03 +0800 Message-ID: References: <20180717112720.GA25197@oracle.com> <20180718103340.GA3893@oracle.com> <20180719.023124.1179208187513867868.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, santosh.shilimkar@oracle.com, rds-devel@oss.oracle.com To: David Miller , sowmini.varadhan@oracle.com Return-path: Received: from aserp2130.oracle.com ([141.146.126.79]:36808 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729215AbeGSDvL (ORCPT ); Wed, 18 Jul 2018 23:51:11 -0400 In-Reply-To: <20180719.023124.1179208187513867868.davem@davemloft.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 07/19/2018 01:31 AM, David Miller wrote: > From: Sowmini Varadhan > 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@oracle.com