From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36D4DC433E0 for ; Tue, 28 Jul 2020 08:17:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1176122B43 for ; Tue, 28 Jul 2020 08:17:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=zx2c4.com header.i=@zx2c4.com header.b="nEjdWpcU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727941AbgG1IRr (ORCPT ); Tue, 28 Jul 2020 04:17:47 -0400 Received: from mail.zx2c4.com ([192.95.5.64]:36909 "EHLO mail.zx2c4.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727916AbgG1IRq (ORCPT ); Tue, 28 Jul 2020 04:17:46 -0400 Received: by mail.zx2c4.com (ZX2C4 Mail Server) with ESMTP id 1f508000; Tue, 28 Jul 2020 07:54:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=zx2c4.com; h=mime-version :references:in-reply-to:from:date:message-id:subject:to:cc :content-type; s=mail; bh=K4YAt33ilBR3hd1+7JojCPxtvSA=; b=nEjdWp cUQVj0KWRd87puFx0fk1HADPVt/5ckkL0vcSiVAoyePGSTncp+KafcroE0itJJAR hvCKOHIgNqwby+4I13HG5z9ibnxBmqHfVx+jKtrPfvHTnJrmPNg1WsagEKVyYT8s 0wLLBcv/NIyWIJ2xwHqa/F7Ekuot/H5qUld6hDSAXP06OIRI6q+V7cdk8Ye2E95N oE2iYHJELOZGqN2H10I58K4qjsUNSGzPFklfgHdPIOfnKUBe2eF7SiuhT2DW4gmx Zlh/4Lwi80eH4MYYZz1R0kvQtczs/XnszWGNGZW6geww/y9D6tsJ0FqJg7QkrUqh r9lHeJJo9aIu1+DA== Received: by mail.zx2c4.com (ZX2C4 Mail Server) with ESMTPSA id d27e76da (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 28 Jul 2020 07:54:16 +0000 (UTC) Received: by mail-io1-f54.google.com with SMTP id a5so4613572ioa.13; Tue, 28 Jul 2020 01:17:41 -0700 (PDT) X-Gm-Message-State: AOAM5326njdztPe3VkZjU3njFuLXGQOjJ86c1ItnwgBvIww26i5yBNmp mf7+bJPIr4+tDgdochtGaglyFVJWyqL1zhmrfeY= X-Google-Smtp-Source: ABdhPJwruWEMSl6lI3SS18Iq/8cGmq7nVWSaRamLRaM9jzUJiajMF+UBN6XkUtAb0UlYhZIkk+zlhZ92oa7Pvf4Iz8s= X-Received: by 2002:a05:6638:250f:: with SMTP id v15mr8210865jat.75.1595924260418; Tue, 28 Jul 2020 01:17:40 -0700 (PDT) MIME-Version: 1.0 References: <20200723060908.50081-1-hch@lst.de> <20200723060908.50081-13-hch@lst.de> <20200727150310.GA1632472@zx2c4.com> <20200727150601.GA3447@lst.de> <20200727162357.GA8022@lst.de> <908ed73081cc42d58a5b01e0c97dbe47@AcuMS.aculab.com> In-Reply-To: <908ed73081cc42d58a5b01e0c97dbe47@AcuMS.aculab.com> From: "Jason A. Donenfeld" Date: Tue, 28 Jul 2020 10:17:28 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 12/26] netfilter: switch nf_setsockopt to sockptr_t To: David Laight Cc: Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexei Starovoitov , Daniel Borkmann , Alexey Kuznetsov , Hideaki YOSHIFUJI , Eric Dumazet , Linux Crypto Mailing List , LKML , Netdev , "bpf@vger.kernel.org" , "netfilter-devel@vger.kernel.org" , "coreteam@netfilter.org" , "linux-sctp@vger.kernel.org" , "linux-hams@vger.kernel.org" , "linux-bluetooth@vger.kernel.org" , "bridge@lists.linux-foundation.org" , "linux-can@vger.kernel.org" , "dccp@vger.kernel.org" , "linux-decnet-user@lists.sourceforge.net" , "linux-wpan@vger.kernel.org" , "linux-s390@vger.kernel.org" , "mptcp@lists.01.org" , "lvs-devel@vger.kernel.org" , "rds-devel@oss.oracle.com" , "linux-afs@lists.infradead.org" , "tipc-discussion@lists.sourceforge.net" , "linux-x25@vger.kernel.org" , Kernel Hardening Content-Type: text/plain; charset="UTF-8" Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Tue, Jul 28, 2020 at 10:07 AM David Laight wrote: > > From: Christoph Hellwig > > Sent: 27 July 2020 17:24 > > > > On Mon, Jul 27, 2020 at 06:16:32PM +0200, Jason A. Donenfeld wrote: > > > Maybe sockptr_advance should have some safety checks and sometimes > > > return -EFAULT? Or you should always use the implementation where > > > being a kernel address is an explicit bit of sockptr_t, rather than > > > being implicit? > > > > I already have a patch to use access_ok to check the whole range in > > init_user_sockptr. > > That doesn't make (much) difference to the code paths that ignore > the user-supplied length. > OTOH doing the user/kernel check on the base address (not an > incremented one) means that the correct copy function is always > selected. Right, I had the same reaction in reading this, but actually, his code gets rid of the sockptr_advance stuff entirely and never mutates, so even though my point about attacking those pointers was missed, the code does the better thing now -- checking the base address and never mutating the pointer. So I think we're good. > > Perhaps the functions should all be passed a 'const sockptr_t'. > The typedef could be made 'const' - requiring non-const items > explicitly use the union/struct itself. I was thinking the same, but just by making the pointers inside the struct const. However, making the whole struct const via the typedef is a much better idea. That'd probably require changing the signature of init_user_sockptr a bit, which would be fine, but indeed I think this would be a very positive change. Jason