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=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,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 075B7C433E2 for ; Wed, 13 May 2020 19:11:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B1BD5205ED for ; Wed, 13 May 2020 19:11:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589397110; bh=/GCNReSmCO9wet49yjeZWebjr4EvER7vzTxofeDO+nc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=KFU+SWc3yMp4wDJvxOhzpoe/8uIB+mjgeWw0usnyS64CtaszoewCFZasSkT+n4Iep 7YWkqfHEnAnMcPJn1gB6zvKKgIprTBH7w5dNezsj1qWMTCY4qN/yBP+gN2+rMT3Sak PAJKG8FRk3asp9OD8dFoKvWWUUxqe6r5/XKMMypI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390303AbgEMTLt (ORCPT ); Wed, 13 May 2020 15:11:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52382 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S2390391AbgEMTLt (ORCPT ); Wed, 13 May 2020 15:11:49 -0400 Received: from mail-lf1-x141.google.com (mail-lf1-x141.google.com [IPv6:2a00:1450:4864:20::141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAFA7C061A0F for ; Wed, 13 May 2020 12:11:48 -0700 (PDT) Received: by mail-lf1-x141.google.com with SMTP id 188so448114lfa.10 for ; Wed, 13 May 2020 12:11:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=DlDe1IobgVm68K0PtFybaGZyKFGHK1OfKry00qMtGtQ=; b=V1UkNiRNfL733X9vr+6V4+xkxNISM7utjyshsT36quvy8jA5IVqpf+k3EDQxjXOIYU JkvVkJQB05ajebE0nuwA1SP0j1/ZNVxXHdyZbBxtTdovrxuSXCzKJOTV20D7eG1vu7ZV OomJViLlEUU/v3q1g+qPNyPfCuDydticNEJPw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=DlDe1IobgVm68K0PtFybaGZyKFGHK1OfKry00qMtGtQ=; b=gmaKLGPI6wcc5V9T2YeD7uoALmyvNa5c9kCGwGTk5gdnH4OdDsYM4jTjufA+2hhrGb 8RHhX0Yt0OX9fJ6x97gMoiN7LbSQfbedqzI1ZWUyKCgACz4szjNP3sFJeJijLQJWo+Ax 4x/FyewvfniH3in3IfKT6gbwSbNDcEpgkMysCe6v4ZrCbCkzWip7NGHI0HxzW8wfw+bp IA9RbPdmg1+eYMjZYfUHD1CCHaOjJnWJAMME4lhR3xyMFsePYIx9UmAt8/a3FhtbMukJ zDhFHneHR1QeKXlbXx76U4WemUMrEycJBU6O9opZzTpljVy8a0V7yERvQOblMGZP4lFR 7t6A== X-Gm-Message-State: AOAM532o4SkuXvQjpIxgK0Q2JblwpSMw+r4/ooFkC1lb4aQHGBRG1TmT AJTJ/uzxL0W20gnjmAQvi7a847XOe90= X-Google-Smtp-Source: ABdhPJzdK3jLyHkGgHdd2nF+R6ZTnubsOUtopck+zgpnz4H/BwkIDsCaQJPatKAMdAcCTA7/tZnVIw== X-Received: by 2002:a19:4209:: with SMTP id p9mr74313lfa.175.1589397105206; Wed, 13 May 2020 12:11:45 -0700 (PDT) Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com. [209.85.167.46]) by smtp.gmail.com with ESMTPSA id w24sm278606lfk.47.2020.05.13.12.11.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 May 2020 12:11:44 -0700 (PDT) Received: by mail-lf1-f46.google.com with SMTP id c21so485105lfb.3 for ; Wed, 13 May 2020 12:11:43 -0700 (PDT) X-Received: by 2002:a19:ed07:: with SMTP id y7mr627896lfy.31.1589397103373; Wed, 13 May 2020 12:11:43 -0700 (PDT) MIME-Version: 1.0 References: <20200513160038.2482415-1-hch@lst.de> <20200513160038.2482415-12-hch@lst.de> In-Reply-To: <20200513160038.2482415-12-hch@lst.de> From: Linus Torvalds Date: Wed, 13 May 2020 12:11:27 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe To: Christoph Hellwig Cc: "the arch/x86 maintainers" , Alexei Starovoitov , Daniel Borkmann , Masami Hiramatsu , Andrew Morton , linux-parisc@vger.kernel.org, linux-um , Netdev , bpf@vger.kernel.org, Linux-MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > > +static void bpf_strncpy(char *buf, long unsafe_addr) > +{ > + buf[0] = 0; > + if (strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, > + BPF_STRNCPY_LEN)) > + strncpy_from_user_nofault(buf, (void __user *)unsafe_addr, > + BPF_STRNCPY_LEN); > +} This seems buggy when I look at it. It seems to think that strncpy_from_kernel_nofault() returns an error code. Not so, unless I missed where you changed the rules. It returns the length of the string for a successful copy. 0 is actually an error case (for count being <= 0). So the test for success seems entirely wrong. Also, I do wonder if we shouldn't gate this on TASK_SIZE, and do the user trial first. On architectures where this thing is valid in the first place (ie kernel and user addresses are separate), the test for address size would allow us to avoid a pointless fault due to an invalid kernel access to user space. So I think this function should look something like static void bpf_strncpy(char *buf, long unsafe_addr) { /* Try user address */ if (unsafe_addr < TASK_SIZE) { void __user *ptr = (void __user *)unsafe_addr; if (strncpy_from_user_nofault(buf, ptr, BPF_STRNCPY_LEN) >= 0) return; } /* .. fall back on trying kernel access */ buf[0] = 0; strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, BPF_STRNCPY_LEN); } or similar. No? Linus 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=-0.8 required=3.0 tests=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 ADC4AC433E3 for ; Wed, 13 May 2020 19:11:50 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 4B1382054F for ; Wed, 13 May 2020 19:11:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="V1UkNiRN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4B1382054F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id EF37A8002D; Wed, 13 May 2020 15:11:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EA4928000B; Wed, 13 May 2020 15:11:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D92338002D; Wed, 13 May 2020 15:11:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0064.hostedemail.com [216.40.44.64]) by kanga.kvack.org (Postfix) with ESMTP id BC0B88000B for ; Wed, 13 May 2020 15:11:49 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 830AB3A9B for ; Wed, 13 May 2020 19:11:49 +0000 (UTC) X-FDA: 76812640338.21.trip43_fe1169804c20 X-HE-Tag: trip43_fe1169804c20 X-Filterd-Recvd-Size: 5121 Received: from mail-lf1-f65.google.com (mail-lf1-f65.google.com [209.85.167.65]) by imf05.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 May 2020 19:11:48 +0000 (UTC) Received: by mail-lf1-f65.google.com with SMTP id u4so465808lfm.7 for ; Wed, 13 May 2020 12:11:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=DlDe1IobgVm68K0PtFybaGZyKFGHK1OfKry00qMtGtQ=; b=V1UkNiRNfL733X9vr+6V4+xkxNISM7utjyshsT36quvy8jA5IVqpf+k3EDQxjXOIYU JkvVkJQB05ajebE0nuwA1SP0j1/ZNVxXHdyZbBxtTdovrxuSXCzKJOTV20D7eG1vu7ZV OomJViLlEUU/v3q1g+qPNyPfCuDydticNEJPw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=DlDe1IobgVm68K0PtFybaGZyKFGHK1OfKry00qMtGtQ=; b=An1BLWZxSqhcwjDYxiQ0sRWnq5fHRhx9nhLUy+PGW4oWwJd2BnX4lJQCWU/cTG2gnM rbc++1iG2CFPZR/yGAXyCsunVnJaE57KMFknGicnMfoxfTCyKPvHUJYGdJgHxyAE1snr qGnfXb807sphkbj+JeqUcZH0J6PYXpmYo4jg6Xt8YXk3sAz2bRObrdeTW711FDO2dVO+ CbnACsagkjahCCN5UphL1XxPfS3ccb14+6lkXLmuiNJlIlNFOWQ7QSRw1gx7zlN1muaa +mQ+69/+ZGAD4Fp4Deyb8936FfjFKM9N+xL5mL1VHcV21URC2BEdwkLH3JgCFP/5jpKr wkzg== X-Gm-Message-State: AOAM531Z93vX8OG0rnX5C7OFWUsgf5yDm1JOzbrWZdfZL25/FX3sBMCY 2nuD79D1rjfjnwZoVEVgn/O0g1jZuoQ= X-Google-Smtp-Source: ABdhPJzSUNcK62KScXvK2hK7QBjJwIGJCNpRv2MVNG/B1xgl0YysSbpsElRMBDt4U/pOusv0h0mPOg== X-Received: by 2002:a19:e20b:: with SMTP id z11mr654390lfg.156.1589397105804; Wed, 13 May 2020 12:11:45 -0700 (PDT) Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com. [209.85.167.46]) by smtp.gmail.com with ESMTPSA id f27sm274635lfe.93.2020.05.13.12.11.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 May 2020 12:11:44 -0700 (PDT) Received: by mail-lf1-f46.google.com with SMTP id u4so465600lfm.7 for ; Wed, 13 May 2020 12:11:43 -0700 (PDT) X-Received: by 2002:a19:ed07:: with SMTP id y7mr627896lfy.31.1589397103373; Wed, 13 May 2020 12:11:43 -0700 (PDT) MIME-Version: 1.0 References: <20200513160038.2482415-1-hch@lst.de> <20200513160038.2482415-12-hch@lst.de> In-Reply-To: <20200513160038.2482415-12-hch@lst.de> From: Linus Torvalds Date: Wed, 13 May 2020 12:11:27 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe To: Christoph Hellwig Cc: "the arch/x86 maintainers" , Alexei Starovoitov , Daniel Borkmann , Masami Hiramatsu , Andrew Morton , linux-parisc@vger.kernel.org, linux-um , Netdev , bpf@vger.kernel.org, Linux-MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > > +static void bpf_strncpy(char *buf, long unsafe_addr) > +{ > + buf[0] = 0; > + if (strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, > + BPF_STRNCPY_LEN)) > + strncpy_from_user_nofault(buf, (void __user *)unsafe_addr, > + BPF_STRNCPY_LEN); > +} This seems buggy when I look at it. It seems to think that strncpy_from_kernel_nofault() returns an error code. Not so, unless I missed where you changed the rules. It returns the length of the string for a successful copy. 0 is actually an error case (for count being <= 0). So the test for success seems entirely wrong. Also, I do wonder if we shouldn't gate this on TASK_SIZE, and do the user trial first. On architectures where this thing is valid in the first place (ie kernel and user addresses are separate), the test for address size would allow us to avoid a pointless fault due to an invalid kernel access to user space. So I think this function should look something like static void bpf_strncpy(char *buf, long unsafe_addr) { /* Try user address */ if (unsafe_addr < TASK_SIZE) { void __user *ptr = (void __user *)unsafe_addr; if (strncpy_from_user_nofault(buf, ptr, BPF_STRNCPY_LEN) >= 0) return; } /* .. fall back on trying kernel access */ buf[0] = 0; strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, BPF_STRNCPY_LEN); } or similar. No? Linus From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f65.google.com ([209.85.167.65]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jYwnK-0001OL-NA for linux-um@lists.infradead.org; Wed, 13 May 2020 19:11:56 +0000 Received: by mail-lf1-f65.google.com with SMTP id z22so507547lfd.0 for ; Wed, 13 May 2020 12:11:53 -0700 (PDT) Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com. [209.85.167.53]) by smtp.gmail.com with ESMTPSA id n23sm198466ljj.48.2020.05.13.12.11.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 May 2020 12:11:44 -0700 (PDT) Received: by mail-lf1-f53.google.com with SMTP id x73so491619lfa.2 for ; Wed, 13 May 2020 12:11:43 -0700 (PDT) MIME-Version: 1.0 References: <20200513160038.2482415-1-hch@lst.de> <20200513160038.2482415-12-hch@lst.de> In-Reply-To: <20200513160038.2482415-12-hch@lst.de> From: Linus Torvalds Date: Wed, 13 May 2020 12:11:27 -0700 Message-ID: Subject: Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Christoph Hellwig Cc: Daniel Borkmann , linux-parisc@vger.kernel.org, Netdev , the arch/x86 maintainers , linux-um , Alexei Starovoitov , Linux Kernel Mailing List , Linux-MM , Masami Hiramatsu , Andrew Morton , bpf@vger.kernel.org On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > > +static void bpf_strncpy(char *buf, long unsafe_addr) > +{ > + buf[0] = 0; > + if (strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, > + BPF_STRNCPY_LEN)) > + strncpy_from_user_nofault(buf, (void __user *)unsafe_addr, > + BPF_STRNCPY_LEN); > +} This seems buggy when I look at it. It seems to think that strncpy_from_kernel_nofault() returns an error code. Not so, unless I missed where you changed the rules. It returns the length of the string for a successful copy. 0 is actually an error case (for count being <= 0). So the test for success seems entirely wrong. Also, I do wonder if we shouldn't gate this on TASK_SIZE, and do the user trial first. On architectures where this thing is valid in the first place (ie kernel and user addresses are separate), the test for address size would allow us to avoid a pointless fault due to an invalid kernel access to user space. So I think this function should look something like static void bpf_strncpy(char *buf, long unsafe_addr) { /* Try user address */ if (unsafe_addr < TASK_SIZE) { void __user *ptr = (void __user *)unsafe_addr; if (strncpy_from_user_nofault(buf, ptr, BPF_STRNCPY_LEN) >= 0) return; } /* .. fall back on trying kernel access */ buf[0] = 0; strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, BPF_STRNCPY_LEN); } or similar. No? Linus _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um