Received: by 2002:a05:6a10:a0d1:0:0:0:0 with SMTP id j17csp2693922pxa; Mon, 17 Aug 2020 16:55:54 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzSA/GekUhm2vnfgt2mPVKRMIWZ5A4vbQ1YcPW1wVy7E6nw21BgL4T7YBtNW/ZiuLQ4mBzf X-Received: by 2002:a17:906:5246:: with SMTP id y6mr17580497ejm.316.1597708554387; Mon, 17 Aug 2020 16:55:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1597708554; cv=none; d=google.com; s=arc-20160816; b=nsoM4KhF2Cu197FTCg9bA2IBekCqR5IESjorMDysaHZ9fiql6rSLFahP0Pg8QwN6A0 xZmIz2jLUSeV3zN8YTg79bLYbm6vmyg4IMsN/ONwVSbTF8bU5qimJOpPuAm70ji16bYA 78nlIsTCBQDPEwF5MWBwXaQwmdFg5WfS67/VoC5rCBcCg3WeXUoXkWdyf/2mM0KXTp4w OhlJPn5XGoQSQWoe7jz8fGXPjLz4rxO8zeqjGNbqZXJSSALrtp6izovZMo2yz7HrRBxw dxoh7Jagt2+LhPjP2O3ElIYXJfixUDand2cqjePQinRpj8qx+BEHAHSJWFZFefvY3hIu EsRg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=zRD9fxr2dmL3cWS6/haxe+/7+RVNAwBfQJLvwFE/2BM=; b=qBZ7IyL6TRv04gOyZNGlAhX5J/HQ2f8hJvl1QrAb3R1aZlLFwBJgWaNN/pqLjA//3l WbxnSc/hfyvM5HyOKat/WRVqx9B8jzfLwn9ODRWxBhYJlUSb+v+JUlbIdB+I/K56zJ2f vjtcl6+8bx5nlZI/sNgLunjYVncr37Duei95EHVYMvcfplEJFZFMIMWQaRpiIfT1J5xh fAj7GVxr7eMMNQPmqzkuSS9OaRcvTN/AWqPXfcPylf6s9w7fr8YRXKM9RmAFQ5zQJ7+T s9CAEoZQKDexfeMaETtZ7EuBniO/IlY3eqNiUyYWJu1va9rB3EYMKz9tFDBsEfg4zNoo MRSA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux-foundation.org header.s=google header.b=GttK3xwV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id i28si15053083edj.373.2020.08.17.16.55.29; Mon, 17 Aug 2020 16:55:54 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linux-foundation.org header.s=google header.b=GttK3xwV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726624AbgHQXy6 (ORCPT + 99 others); Mon, 17 Aug 2020 19:54:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726245AbgHQXyz (ORCPT ); Mon, 17 Aug 2020 19:54:55 -0400 Received: from mail-lj1-x242.google.com (mail-lj1-x242.google.com [IPv6:2a00:1450:4864:20::242]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0D84AC061389 for ; Mon, 17 Aug 2020 16:54:55 -0700 (PDT) Received: by mail-lj1-x242.google.com with SMTP id w14so19411893ljj.4 for ; Mon, 17 Aug 2020 16:54:54 -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=zRD9fxr2dmL3cWS6/haxe+/7+RVNAwBfQJLvwFE/2BM=; b=GttK3xwVXIEnQDmtYMvF2IJlJWEatfccGGsHzVm20TPi9ekAoaV3TrMSE8QIJupeaU o5VYt+7CPeShna6icVTEVqf0cqW5xz6D3gxuRcD4PasdgxEfOrygjNQEgJGTOM8C+RQ7 unaPe2LNmBbFRNpH7o6v91jPT9h97hM2C5bOw= 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=zRD9fxr2dmL3cWS6/haxe+/7+RVNAwBfQJLvwFE/2BM=; b=YQbcyfDQhpF89UEJUjqfet0gRIx/Eshtc7Gx+LN4nkOUnKL7cKH0btl8Bw3TRCa/C+ 1YNLzpRKkGUpbdHnpEionBgKY+xsv7jkG9nGpdKT2mwktQLuRM0wjvYZE66Ubz+jO5kS b7sQVhkGBx6vpGDJac/v1Wa/Z4Jl0t6VyPc3QljcC83UgBpCzLWovmEIFPUkEQaQ4PKR g2cIo7fqCsMXp/gDpHddqHJ4+T1OuNtdH6qlD2cbq3Yl/q7/vEBxtejrfy1hEDhtRHpZ tqHe5WUpLzaJXVh1zS9Di0BkihyUrOBxOXVjQCl3r27de6ACsDRFb+pXuM3yKVzYkCgo dGvw== X-Gm-Message-State: AOAM533CRjWdCDoB1GANJjS+syoOVmhJgIj78Pyp8L+V1/N7CM/3Qs7c Tee/+n3DhaQpjNVVzVBx4b2y7T+0q3Hxww== X-Received: by 2002:a2e:3802:: with SMTP id f2mr7760993lja.212.1597708493053; Mon, 17 Aug 2020 16:54:53 -0700 (PDT) Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com. [209.85.208.175]) by smtp.gmail.com with ESMTPSA id u9sm5310005ljh.20.2020.08.17.16.54.50 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Aug 2020 16:54:51 -0700 (PDT) Received: by mail-lj1-f175.google.com with SMTP id f26so19389924ljc.8 for ; Mon, 17 Aug 2020 16:54:50 -0700 (PDT) X-Received: by 2002:a2e:9a11:: with SMTP id o17mr7971652lji.314.1597708490610; Mon, 17 Aug 2020 16:54:50 -0700 (PDT) MIME-Version: 1.0 References: <87ft8l6ic3.fsf@x220.int.ebiederm.org> <20200817220425.9389-9-ebiederm@xmission.com> In-Reply-To: <20200817220425.9389-9-ebiederm@xmission.com> From: Linus Torvalds Date: Mon, 17 Aug 2020 16:54:34 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 09/17] file: Implement fnext_task To: "Eric W. Biederman" Cc: Linux Kernel Mailing List , linux-fsdevel , criu@openvz.org, bpf , Alexander Viro , Christian Brauner , Oleg Nesterov , Cyrill Gorcunov , Jann Horn , Kees Cook , =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= , Jeff Layton , Miklos Szeredi , Matthew Wilcox , "J. Bruce Fields" , Matthew Wilcox , Trond Myklebust , Chris Wright , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I like the series, but I have to say that the extension of the horrible "fcheck*()" interfaces raises my hackles.. That interface is very very questionable to begin with, and it's easy to get wrong. I don't see you getting it wrong - you do seem to hold the RCU read lock in the places I checked, but it worries me. I think my worry could be at least partially mitigated with more comments and docs: On Mon, Aug 17, 2020 at 3:10 PM Eric W. Biederman wrote: > > +struct file *fnext_task(struct task_struct *task, unsigned int *ret_fd) Please please *please* make it clear that because this does *not* increment any reference counts, you have to be very very careful using the "struct file" pointer this returns. I really dislike the naming. The old "fcheck()" naming came from the fact that at least one original user just mainly checked if the result was NULL or not. And then others had to be careful to either hold the file_lock spinlock, or at least the RCU read lock so that the result didn't go away. Here, you have "fnext_task()", and it doesn't even have that "check" warning mark, or any comment about how dangerous it can be to use.. So the rule is that the "struct file" pointer this returns can only be read under RCU, but the 'fd' it returns has a longer lifetime. I think your uses are ok, and I think this is an improvement in general, I just want a bigger "WARNING! Here be dragons!" sign (and naming could be nicer). Linus