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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C2EBC433EF for ; Mon, 4 Jul 2022 20:30:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233424AbiGDUas (ORCPT ); Mon, 4 Jul 2022 16:30:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41700 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229515AbiGDUaq (ORCPT ); Mon, 4 Jul 2022 16:30:46 -0400 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A09176306; Mon, 4 Jul 2022 13:30:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=0ahdG6vwtrla/GfArjvdjqG8Ieo6jXxVEyUk4N9P1DU=; b=ne0Uf/6y0BOWxJovHIrHEvr2xA xwOaT8SpGI4pQyxrDEsz0MXWEP5fU5BfmjkOS9uiXA2/uYafTL8z3mZG61t3pF8Yp5qDYFUM7dg0S 55ceSjFJBLdMCEPY8wiIS4wC5EWV4pUiB3jRlbbqEtE6dLqHsMTI3kvXXpvDrKj3EFnC7Jy9L9kyl iFqZTArbZn0Ub6BGDXrPH2SDexvyUvZH3gwVyWJGfZABlTfprEuLob7jWWY+L3v2nMp1bCnQ2+KGF G/mLSR88wuG9n29bHZMonvBJFlpsljyW7iZOnelikF61rHzsxzej0HSZFkj1WUecw35IcXl8USJUW /mXzlagg==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.95 #2 (Red Hat Linux)) id 1o8Sho-0088HH-1A; Mon, 04 Jul 2022 20:30:04 +0000 Date: Mon, 4 Jul 2022 21:30:03 +0100 From: Al Viro To: Matthew Wilcox Cc: Alexander Potapenko , Alexei Starovoitov , Andrew Morton , Andrey Konovalov , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Christoph Hellwig , Christoph Lameter , David Rientjes , Dmitry Vyukov , Eric Dumazet , Greg Kroah-Hartman , Herbert Xu , Ilya Leoshkevich , Ingo Molnar , Jens Axboe , Joonsoo Kim , Kees Cook , Marco Elver , Mark Rutland , "Michael S. Tsirkin" , Pekka Enberg , Peter Zijlstra , Petr Mladek , Steven Rostedt , Thomas Gleixner , Vasily Gorbik , Vegard Nossum , Vlastimil Babka , kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 44/45] mm: fs: initialize fsdata passed to write_begin/write_end interface Message-ID: References: <20220701142310.2188015-1-glider@google.com> <20220701142310.2188015-45-glider@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: Al Viro Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 04, 2022 at 09:07:43PM +0100, Matthew Wilcox wrote: > On Fri, Jul 01, 2022 at 04:23:09PM +0200, Alexander Potapenko wrote: > > Functions implementing the a_ops->write_end() interface accept the > > `void *fsdata` parameter that is supposed to be initialized by the > > corresponding a_ops->write_begin() (which accepts `void **fsdata`). > > > > However not all a_ops->write_begin() implementations initialize `fsdata` > > unconditionally, so it may get passed uninitialized to a_ops->write_end(), > > resulting in undefined behavior. > > ... wait, passing an uninitialised variable to a function *which doesn't > actually use it* is now UB? What genius came up with that rule? What > purpose does it serve? "The value we are passing might be utter bollocks, but that way it's obfuscated enough to confuse anyone, compiler included". Defensive progamming, don'cha know? I would suggest a different way to obfuscate it, though - pass const void ** and leave it for the callee to decide whether they want to dereferences it. It is still 100% dependent upon the ->write_end() being correctly matched with ->write_begin(), with zero assistance from the compiler, but it does look, er, safer. Or something. Of course, a clean way to handle that would be to have ->write_begin() return a partial application of foo_write_end to whatever it wants for fsdata, to be evaluated where we would currently call ->write_end(). _That_ could be usefully typechecked, but... we don't have usable partial application.