Date:   Thu, 16 Feb 2023 11:59:10 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Kees Cook <keescook@chromium.org>
Cc:     Leon Romanovsky <leon@kernel.org>,
        Mark Zhang <markzhang@nvidia.com>,
        Patrisious Haddad <phaddad@nvidia.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        linux-rdma@vger.kernel.org, llvm@lists.linux.dev,
        Tom Rix <trix@redhat.com>,
        "Jason A. Donenfeld" <Jason@zx2c4.com>,
        =?utf-8?B?SMOla29u?= Bugge <haakon.bugge@oracle.com>,
        linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH] RDMA/cma: Distinguish between sockaddr_in and
 sockaddr_in6 by size
Message-ID: <Y+5SzsEkbXp6GBgN@nvidia.com>
References: <20230208232549.never.139-kees@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230208232549.never.139-kees@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ZM4FyiYz6w6UB/jcqRWVbvItN3ORT4OimESlZcHc62dYg4kaJpM8tTkCBelR?=
 =?us-ascii?Q?nSLUqrOlBRnIeEKJyvSN0et5tqO63jBoV0rqsstDlgugXlEX2fp7YH+jOVUJ?=
 =?us-ascii?Q?jr+jC7IXbJLKBbdUCf1FbwKsiHtcR1OR9Gju/NkVYicz+OWmvKdXn3fQuJ3L?=
 =?us-ascii?Q?ngBTuhWwJimYzykF8lZ6vxUpFAc1aiLZLcp4z982jXzO8+m9t4GJ47A2tUxw?=
 =?us-ascii?Q?NSwIHFIstOk5E/DWqfdkbvmsoF12P5I+6OwPWSG5/dDB9Ekvu+EHonuQf+Zn?=
 =?us-ascii?Q?bvanSg4mr+AKqKafrd12tv0H6EPS+sbQ49c0n22K7uXgPqwcNsvXfJr7h8qV?=
 =?us-ascii?Q?IR+vs73A5ao38H54ICD51GC61tYpV0rgN+078x7oZa9ZTraLj4idwdBdYy93?=
 =?us-ascii?Q?QlfbyeZX189rfa+p5E6UQjRMAtq1EpCKu2eXecc9q9PGbz14qy/VZTV6jS+i?=
 =?us-ascii?Q?sFQTJWT39bkj7L+2D/vJIf3yaiS4P4QEuoFTcwRZCXre4pVMzrvi5u98IY5t?=
 =?us-ascii?Q?FwGFseaHpr+KaHu0odFio4iZkKVRh32P9Bc7cCarIYJKTVSrI3MpNnCh0j2J?=
 =?us-ascii?Q?NdiwqQDemJQ+63scwwuEjXZHSki8nhvS4GFC5PFFiHf1vo5zg9Wp5B30u8AA?=
 =?us-ascii?Q?+D9vtTQ7/axlZ7Un6JhuAA2uzR0PXX3aafMc5ooJk4WzJdIvIjCtMQyncq0X?=
 =?us-ascii?Q?heG1I0tc8BxHU1gkBZP/j7X7sAYsylXQ+q7LX02JvjNQRyFXV0rjQC+D8bmh?=
 =?us-ascii?Q?2jjCtcfBZZS9v8xJU8h1qL5qKOoKZjVQcpLQTQCpD5oxChZEcNNZg84q5hMV?=
 =?us-ascii?Q?0dxem7DPVGVT12zQI+uhLylzqoLRrrH/FVKCMQ575EU7jLmuaAJf0JQEXJgF?=
 =?us-ascii?Q?ecN+DcRVzAcshbUjCYMR5R1UFe+Q+3dBoiH6vck3rWH/UqtBtiUBRxxYnsgH?=
 =?us-ascii?Q?UOSwTGDdrt35dt93Nx09KKdIhd4Z4PieDlfmMGVob/YJ8eEleKEAtnJKscWm?=
 =?us-ascii?Q?Fuy0QuSUWMSSaij+hUIerSnfnHuMQ6VZqp4uGcKCy6rMXsBuzF7sYDgbrEhm?=
 =?us-ascii?Q?9SCBUKE8kbLInlxEMl5aHZnCHiZs/WS+O+tn92b4eSM3HekGOKCej8FvEI3m?=
 =?us-ascii?Q?VBurt4G6zUMCfrCHHnjI01sr+6s3NN0v2q78C9z4E2PHCyFSidUAhfjxtUs7?=
 =?us-ascii?Q?Kj9DvwgAfEIQxhB4lVOqb2bR13MNOj9raT8Nw9LafgGaRcGTKcNxSBjt9d4G?=
 =?us-ascii?Q?ch4s9wqQqcZhf1kZzQZD1BXRtUYGY1c/QOOFVQx3PwaOGbZGckUfz57LYpWc?=
 =?us-ascii?Q?KbJhxHmncBvA6PTcq1Ef4Z6cmBVWDbnm1sIiJ/y1PvRkK3tixu/l6xTYLyks?=
 =?us-ascii?Q?/N/kWv9wpwYDFyjLi8YNpZTOUugBNvutoj5m2vVa83v7Pr+wHncRwY9DLTVs?=
 =?us-ascii?Q?IT0uKb6RELLT+MqmR/JZgNqPZmrbvx0lc/GuD5+u2vzysazh91ob9C9LZT8d?=
 =?us-ascii?Q?1BB4IQNtL0A80H3lY7qbJxpR0Kaf7D92f02tH/Aki0Gw300BfTjTihd3aM+8?=
 =?us-ascii?Q?Uhc444iDx85BjHogojIl23wXfehmtevDzu/7zhnw?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 57c1ebd2-2379-4464-5db4-08db1036bf09
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2023 15:59:12.4584
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MIM9kZn9WZ8Nn/U9HKNh9fEyh6FLs0/0oN1xg2yrGBKlaefgEPQBZKISCmBXypyG
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR12MB6074
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 08, 2023 at 03:25:53PM -0800, Kees Cook wrote:
> Clang can do some aggressive inlining, which provides it with greater
> visibility into the sizes of various objects that are passed into
> helpers. Specifically, compare_netdev_and_ip() can see through the type
> given to the "sa" argument, which means it can generate code for "struct
> sockaddr_in" that would have been passed to ipv6_addr_cmp() (that expects
> to operate on the larger "struct sockaddr_in6"), which would result in a
> compile-time buffer overflow condition detected by memcmp(). Logically,
> this state isn't reachable due to the sa_family assignment two callers
> above and the check in compare_netdev_and_ip(). Instead, provide a
> compile-time check on sizes so the size-mismatched code will be elided
> when inlining. Avoids the following warning from Clang:
> 
> ../include/linux/fortify-string.h:652:4: error: call to '__read_overflow' declared with 'error' attribute: detected read beyond size of object (1st parameter)
>                         __read_overflow();
>                         ^
> note: In function 'cma_netevent_callback'
> note:   which inlined function 'node_from_ndev_ip'
> 1 error generated.
> 
> When the underlying object size is not known (e.g. with GCC and older
> Clang), the result of __builtin_object_size() is SIZE_MAX, which
> will also compile away, leaving the code as it was originally.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1687
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Mark Zhang <markzhang@nvidia.com>
> Cc: Patrisious Haddad <phaddad@nvidia.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: linux-rdma@vger.kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org> # build
> ---
>  drivers/infiniband/core/cma.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)

This seems hacky, but I guess I can see why it is unreasonable for the
compiler to track the sa_family in this case.

Applied to for next

Jason